rjmccall added inline comments.

================
Comment at: lib/Sema/SemaCast.cpp:2288
+      SrcType->isPointerType()) {
+    const PointerType *DestPtr = DestType->getAs<PointerType>();
+    if (!DestPtr->isAddressSpaceOverlapping(*SrcType->getAs<PointerType>())) {
----------------
Anastasia wrote:
> Anastasia wrote:
> > rjmccall wrote:
> > > Please test the result of `getAs` instead of separately testing 
> > > `isPointerType`.
> > > 
> > > Why is this check OpenCL-specific?  Address spaces are a general language 
> > > feature.
> > I think mainly because I am factoring out from the existing OpenCL check. 
> > But it probably makes sense that the semantics of this is not different in 
> > other languages. I will update it! Thanks!
> After playing with this for a bit longer I discovered that I have to keep the 
> OpenCL check unfortunately.
> 
> I found this old commit (`d4c5f84`/`r129583`) that says:
>   C-style casts can add/remove/change address spaces through the 
> reinterpret_cast mechanism.
> That's not the same as in OpenCL because it seems for C++ you can cast any AS 
> to any other AS. Therefore, no checks are needed at all. I am not sure if we 
> can come up with a common function for the moment.
> 
> The following  tests are checking this:
>    CodeGenCXX/address-space-cast.cpp
>    SemaCXX/address-space-conversion.cpp
> 
> Do you think it would make sense to rename this method with OpenCL-something 
> or keep in case may be CUDA or some other languages might need similar 
> functionality...
> 
I think you can leave it alone for now, but please add a comment explaining the 
reasoning as best you see it, and feel free to express uncertainty about the 
right rule.

Don't take `QualType` by `const &`, by the way.  It's a cheap-to-copy value 
type and should always be passed by value.


https://reviews.llvm.org/D52598



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to