[PATCH] D103112: Reimplement __builtin_unique_stable_name-

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.


================
Comment at: clang/include/clang/Basic/Attr.td:329
+def SYCLDevice : LangOpt<"SYCLIsDevice">;
+def SYCLHost : LangOpt<"SYCLIsDevice">;
 def COnly : LangOpt<"", "!LangOpts.CPlusPlus">;
----------------
Pretty sure you meant this. :-)


================
Comment at: clang/include/clang/Basic/Attr.td:1184
   let Subjects = SubjectList<[FunctionTmpl]>;
-  let LangOpts = [SYCL];
+  let LangOpts = [SYCLDevice, SYCLHost];
   let Documentation = [SYCLKernelDocs];
----------------
Is this change (and the above) intentionally part of this commit? I don't see a different use of `sycl_kernel` in the test cases, so it's a bit unclear.


================
Comment at: clang/lib/AST/ASTContext.cpp:2461
     return getPreferredTypeAlign(getPointerDiffType().getTypePtr());
- 
   if (!Target->allowsLargerPreferedTypeAlignment())
----------------
Unintentional whitespace change?


================
Comment at: clang/lib/AST/ItaniumMangle.cpp:5057-5061
+    if (USN->isExpr()) {
+      mangleTemplateArgExpr(USN->getExpr());
+    } else {
+      mangleType(USN->getTypeSourceInfo()->getType());
+    }
----------------



================
Comment at: clang/lib/AST/StmtPrinter.cpp:1085
+void StmtPrinter::VisitUniqueStableNameExpr(UniqueStableNameExpr *Node) {
+  // FIXME: Is this the correct thing?
+  OS << "__builtin_unique_stable_name(";
----------------
I think this comment can be removed -- I think this is doing the correct thing, but adding a test case may help prove it.


================
Comment at: clang/lib/CodeGen/CGExprScalar.cpp:1591
+  llvm::Constant *GlobalConstStr = Builder.CreateGlobalStringPtr(
+      E->ComputeName(Context), "usn_str",
+      static_cast<unsigned>(GlobalAS.getValueOr(LangAS::Default)));
----------------
Do we need to give this a name with a reserved identifier to avoid linking conflicts?


================
Comment at: clang/lib/Sema/TreeTransform.h:10195
+  if (E->isExpr()) {
+    ExprResult ER = getDerived().TransformExpr(E->getExpr());
+
----------------
Does this transformation need to happen in an unevaluated context?


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

https://reviews.llvm.org/D103112