yaxunl added a comment. Can you add a codegen test for 1.2, 2.0?
Also, I am wondering whether this is valid for OpenCL C++. Do we need special handling for string literal? ================ Comment at: lib/AST/Expr.cpp:870 + if (C.getLangOpts().OpenCL && Ty.getAddressSpace() == LangAS::Default) + Ty = C.getAddrSpaceQualType(Ty, LangAS::opencl_constant); + ---------------- Anastasia wrote: > Anastasia wrote: > > bader wrote: > > > Anastasia wrote: > > > > bader wrote: > > > > > As `Ty` is passed by value, shouldn't we accept only data located in > > > > > constant address space? > > > > Do you mean to assert? Currently it should be passed with `constant` AS > > > > but I thought the idea is to modify this function so we can accept > > > > `Default` AS too but then replace by `constant`. > > > > > > > Yes, but according to your other comment this idea doesn't work. > > > > > > > I have added the address space to the creation of StringLiteral, but > > > > unfortunately it doesn't seems like we can remove similar code in other > > > > places because **QualType created for StringLiteral is also used > > > > elsewhere and has to match (contain right address space).** I.e. here > > > > is it used further down to create PredefinedExpr. > > > > > > > > > > > > > > Indeed for the current cases it doesn't... there might be some uncaught > > ones though where it can be useful. But we can add it later as well. So I > > am thinking to undo this? > I quite liked the idea initially, but unfortunately it seems like it's not > applicable to the current use cases. Can you extract this as ``` QualType ASTContext::getStringLiteralType(QualType); ``` then use it here and below? ================ Comment at: test/SemaOpenCL/predefined-expr.cl:1 +// RUN: %clang_cc1 %s -verify + ---------------- can you also check for cl 2.0? https://reviews.llvm.org/D46049 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
