[PATCH] D53764: [OpenCL] Enable address spaces for references in C++
John McCall via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Fri Nov 9 09:42:17 PST 2018
rjmccall added inline comments.
Comment at: lib/AST/Expr.cpp:1609
- assert(getType()->isPointerType() || getType()->isBlockPointerType());
- assert(getSubExpr()->getType()->isPointerType() ||
- assert(getType()->getPointeeType().getAddressSpace() !=
+ assert(/*If pointer type then addr spaces for pointees must differ*/
+ (((getType()->isPointerType() &&
> rjmccall wrote:
> > Anastasia wrote:
> > > I don't like this assert now. Would adding extra variable be cleaner here?
> > Yeah, this assertion doesn't make any sense like this. It should be checking whether the cast is a gl-value and, if so, requiring the subexpression to also be a gl-value and then asserting the difference between the type. But you can certainly do an address-space conversion on l-values that just happen to be of pointer or block-pointer type.
> No, if this is a gl-value cast, the assertion must ignore whether there's a pointee type, or it will be messed up on gl-values of pointer types.
> That is, if I have a gl-value of type `char * __private`, I should be able to do an address-space promotion to get a gl-value of type `char * __generic`. It's okay that the pointers are into the same address space here — in fact, it's more than okay, it's necessary.
Thanks, that's right now. Although please assert that the base has the same value kind; I've seen bugs before where ICEs tried to implicitly materialize their arguments, and it's really frustrating to root out.
Comment at: lib/Sema/SemaExprCXX.cpp:4285
+ ? CK_AddressSpaceConversion
+ : CK_NoOp;
If `ToType` is a reference type, the address space will be on its pointee type.
More information about the cfe-commits