[PATCH] D154696: [Clang] Diagnose jumps into statement expressions

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin marked 2 inline comments as done.
cor3ntin added inline comments.


================
Comment at: clang/docs/ReleaseNotes.rst:571
   (`#61758 <https://github.com/llvm/llvm-project/issues/61758>`_)
+- Correcly diagnose jumps into statement expressions.
+  (`#63682 <https://github.com/llvm/llvm-project/issues/63682>`_)
----------------
shafik wrote:
> Maybe remark this conforms with gcc behavior.
I added something. should we link to GCC documentation?


================
Comment at: clang/lib/Sema/JumpDiagnostics.cpp:489
+    Scopes.push_back(GotoScope(ParentScope,
+                               diag::note_enters_statement_expression, 0,
+                               SE->getBeginLoc()));
----------------
shafik wrote:
> I guess we don't have a variable that stands or no-diagnostic.
nope, we use 0 all over the place in that file 


================
Comment at: clang/lib/Sema/SemaExpr.cpp:16468
   PushExpressionEvaluationContext(ExprEvalContexts.back().Context);
+  setFunctionHasBranchProtectedScope();
 }
----------------
shafik wrote:
> I see why we need this but I am not sure how someone would know that need to do this in addition to the change in `BuildScopeInformation(...)`
I added a comment.
I may have spent some time trying to figure out that i needed that line :)


================
Comment at: clang/test/Sema/scope-check.c:251
+  {
+    (void)sizeof (int){({ goto O; 1; })}; // ok (not evaluated)
+    O: ;
----------------
shafik wrote:
> I am sorry for this example:
> 
> ```
> int main() {
>    sizeof(({goto label;}), 0);
>    return 3;
>   ({label:1;});  
> }
> ```
> 
> see godbolt: https://godbolt.org/z/aeb6TbsoG
> 
> Note gcc's behavior Vs clangs.
I added it. It's gnarly :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154696