rjmccall added inline comments.
================
Comment at: lib/AST/Expr.cpp:1609
case CK_AddressSpaceConversion:
- assert(getType()->isPointerType() || getType()->isBlockPointerType());
- assert(getSubExpr()->getType()->isPointerType() ||
- getSubExpr()->getType()->isBlockPointerType());
- assert(getType()->getPointeeType().getAddressSpace() !=
- getSubExpr()->getType()->getPointeeType().getAddressSpace());
- LLVM_FALLTHROUGH;
+ assert(/*If pointer type then addr spaces for pointees must differ*/
+ (((getType()->isPointerType() &&
----------------
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.
================
Comment at: lib/CodeGen/CGExpr.cpp:4252
+ Address V =
+ Builder.CreateAddrSpaceCast(LV.getAddress(), ConvertType(DestTy));
+
----------------
Please use the `performAddrSpaceCast` target hook instead of directly
constructing an LLVM `addrspacecast`.
================
Comment at: lib/Sema/DeclSpec.cpp:576
+ if (S.getLangOpts().OpenCLVersion < 120 &&
+ !S.getLangOpts().OpenCLCPlusPlus) {
+ DiagID = diag::err_opencl_unknown_type_specifier;
----------------
Please update the comment above this.
================
Comment at: lib/Sema/SemaDecl.cpp:7366
+ (getLangOpts().OpenCLVersion == 200 ||
+ getLangOpts().OpenCLCPlusPlus)))) {
int Scope = NewVD->isStaticLocal() | NewVD->hasExternalStorage() << 1;
----------------
Please update the comment above this.
================
Comment at: lib/Sema/SemaInit.cpp:7614
+ : CK_NoOp;
+ CurInit = S.ImpCastExprToType(CurInit.get(), Step->Type, CK, VK);
break;
----------------
Please extract a function to do an l-value qualification conversion just in
case we add more non-trivial conversions that we need to represent.
https://reviews.llvm.org/D53764
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
