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

Reply via email to