[PATCH] D46022: [OpenCL] Restrict various keywords in OpenCL C++ mode

John McCall via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Apr 26 08:14:32 PDT 2018


rjmccall added inline comments.


================
Comment at: include/clang/Basic/TokenKinds.def:255
+//   KEYNOOPENCL  - This is a keyword that is not supported in OpenCL C
+//                  nor in OpenCL C++.
 //   KEYALTIVEC - This is a keyword in AltiVec
----------------
svenvh wrote:
> rjmccall wrote:
> > `KEYNOOPENCL` is dead now, I think.
> There are still some other uses of `KEYNOOPENCL` (already there before this patch).
Oh, I see.


================
Comment at: lib/Parse/ParseStmtAsm.cpp:696
+    return StmtError();
+  }
+
----------------
You might consider parsing the statement normally and then just diagnosing after the fact, maybe in Sema.  You'd have to add the check in a couple different places, though.


================
Comment at: lib/Sema/DeclSpec.cpp:621
+    // OpenCL C++ 1.0 s2.9: the thread_local storage qualifier is not
+    // supported.
+    case TSCS_thread_local:
----------------
svenvh wrote:
> rjmccall wrote:
> > Do you really care about the spelling of the specifier?  Presumably `__thread` (the GNU extension) and `_Thread_local` (the C11 feature) are unsupported; where are those diagnosed?
> Fair enough, I have updated the patch to just reject any thread qualifier here.
I meant it as a question, but I take it from this response that we don't actually diagnose these.

It might make more sense to diagnose this in `Sema::ActOnVariableDeclarator`, where we're already doing some checking about whether e.g. the target supports the feature.


================
Comment at: lib/Sema/SemaStmt.cpp:2791
+                     << "goto");
+  }
+
----------------
Is this restriction really OpenCL C++-specific?  I mean, if that's what the language spec says, that's what it says, but I don't know what about OpenCL C++ would make `goto` unsupportable when it's supportable in ordinary OpenCL.

You should also add this check to ActOnIndirectGotoStmt.


https://reviews.llvm.org/D46022





More information about the cfe-commits mailing list