svenvh added inline comments.
================ Comment at: lib/Parse/ParseStmtAsm.cpp:696 + return StmtError(); + } + ---------------- rjmccall wrote: > 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. Precisely the reason why I did it in the parser, otherwise we'd have to duplicate the check in multiple places. ================ 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: ---------------- rjmccall wrote: > 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. We do actually diagnose the others in `Sema::ActOnVariableDeclarator` ("thread-local storage is not supported for the current target"). But your question made me realize there is no real need to differentiate here for the OpenCL C++ case. ================ Comment at: lib/Sema/SemaStmt.cpp:2791 + << "goto"); + } + ---------------- rjmccall wrote: > 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. Yes (oddly enough). The OpenCL C++ 1.0 spec explicitly mentions `goto` in the Restrictions section on page 51. The OpenCL C 1.1 / 2.0 specs ("ordinary OpenCL") do not restrict goto. I'll add the check for indirect goto statements. https://reviews.llvm.org/D46022 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits