[PATCH] D70605: [OpenCL] Fix address space for implicit conversion (PR43145)

Tue Jul 7 10:07:45 PDT 2020

ebevhan added inline comments.

================
Comment at: clang/lib/Sema/SemaExprCXX.cpp:4106
+      NewToType = Context.getAddrSpaceQualType(NewToType,
+                                               FromPteeType.getAddressSpace());
+      if (ToType->isObjCObjectPointerType())
----------------
svenvh wrote:
> ebevhan wrote:
> > I don't think this will manage to properly repack the type if it's hidden behind enough sugar. When I enable C++ address space conversions in a non-OpenCL context, this breaks the derived-to-base example in CodeGenCXX/address-space-cast.cpp.
> > 
> > IsPointerConversion doesn't have an issue with this since it reconstructs the destination type with the appropriate qualifiers through BuildSimilarlyQualifiedPointerType.
> > 
> > Wouldn't it make more sense to have *PointerConversion only handle the derived-to-base and leave the address space conversion to *QualificationConversion?
> > Wouldn't it make more sense to have *PointerConversion only handle the derived-to-base and leave the address space conversion to *QualificationConversion?
> 
> I didn't have a thorough look to understand all implications of that, but maybe it's worth trying.  Not sure I'll be able to follow up on this any time soon, would you be able to give it a try?
I'll see if I can have a look, or at least make the existing implementation a bit more thorough.

To be entirely honest, I feel a bit stupid right now: I forgot that **I** added the derived-to-base test in the test case I mentioned in my local experimental branch, so of course you wouldn't be able to use that as a baseline. Oops!

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D70605/new/

https://reviews.llvm.org/D70605