[PATCH] D59988: [PR41276] Generate address space cast of 'this' for objects attributed by an address space in C++

Mon Apr 1 10:05:43 PDT 2019

rjmccall added inline comments.

================
Comment at: lib/CodeGen/CGClass.cpp:2025
+    ThisPtr =
+        Builder.CreatePointerBitCastOrAddrSpaceCast(This.getPointer(), NewType);
   }
----------------
brunodf wrote:
> rjmccall wrote:
> > Anastasia wrote:
> > > I am a bit unsure if `performAddrSpaceCast` should be used, but considering that we know that we are not casting a constant it should be fine?
> > > 
> > > If not any suggestions how to propagate `LangAS` of 'this' here. Some thoughts I have are:
> > > - Try to list the conversion up in the call stack
> > > - Pass `LangAS` all the way to here
> > I feel like `This` should just be in the right address space for the constructor at the point `EmitCXXConstructorCall` is called.  We don't expect this function to do any other semantic conversions.  Or is this necessary to handle special-case use of things like trivial default / copy constructors?
> Where could the conversion of `this` be listed in the clang AST? `this` seems implicit there.
> 
> Passing along `LangAS` seems to have some precedent. `EmitCXXConstructExpr` (which calls `EmitCXXConstructorCall`) works on `AggValueSlot` which carries the original qualifiers. Currently not yet used for address space (but this seems similar to me):
> 
> ```
>   /// \param quals - The qualifiers that dictate how the slot should
>   /// be initialied. Only 'volatile' and the Objective-C lifetime
>   /// qualifiers matter.
> ```
> 
Passing down the address space also works, whether in an `AggValueSlot` or an `LValue` or whatever, and I suppose it's better for the special cases for trivial constructors.

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

https://reviews.llvm.org/D59988