[PATCH] D155525: WIP

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers abandoned this revision.
nickdesaulniers added inline comments.



Comment at: clang/lib/Sema/JumpDiagnostics.cpp:362
 // it.  This makes the second scan not have to walk the AST again.
+RecordJumpScope:
 LabelAndGotoScopes[S] = ParentScope;

rjmccall wrote:
> I would indent this at the same level as the `case` labels, and group it with 
> them, too, but yeah, this is what I was thinking.  But the helper function 
> approach looks pretty clean, too.  Your choice.
I don't feel strongly either way. I'll fix up the indentation then squash this 
down into D155342; this is fewer lines changed than D155522.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155525

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D155525: WIP

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: clang/lib/Sema/JumpDiagnostics.cpp:362
 // it.  This makes the second scan not have to walk the AST again.
+RecordJumpScope:
 LabelAndGotoScopes[S] = ParentScope;

I would indent this at the same level as the `case` labels, and group it with 
them, too, but yeah, this is what I was thinking.  But the helper function 
approach looks pretty clean, too.  Your choice.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155525

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D155525: WIP

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers created this revision.
Herald added a project: All.
nickdesaulniers requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Alternate to D155522  based on
https://reviews.llvm.org/D155342#4507805


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D155525

Files:
  clang/lib/Sema/JumpDiagnostics.cpp


Index: clang/lib/Sema/JumpDiagnostics.cpp
===
--- clang/lib/Sema/JumpDiagnostics.cpp
+++ clang/lib/Sema/JumpDiagnostics.cpp
@@ -331,11 +331,8 @@
 // operand (to avoid recording the address-of-label use), which
 // works only because of the restricted set of expressions which
 // we detect as constant targets.
-if (cast(S)->getConstantTarget()) {
-  LabelAndGotoScopes[S] = ParentScope;
-  Jumps.push_back(S);
-  return;
-}
+if (cast(S)->getConstantTarget())
+  goto RecordJumpScope;
 
 LabelAndGotoScopes[S] = ParentScope;
 IndirectJumps.push_back(S);
@@ -352,15 +349,17 @@
   BuildScopeInformation(Var, ParentScope);
   ++StmtsToSkip;
 }
-[[fallthrough]];
+goto RecordJumpScope;
 
-  case Stmt::GotoStmtClass:
   case Stmt::GCCAsmStmtClass:
-if (auto *GS = dyn_cast(S))
-  if (!GS->isAsmGoto())
-break;
+if (!cast(S)->isAsmGoto())
+  break;
+goto RecordJumpScope;
+
+  case Stmt::GotoStmtClass:
 // Remember both what scope a goto is in as well as the fact that we have
 // it.  This makes the second scan not have to walk the AST again.
+RecordJumpScope:
 LabelAndGotoScopes[S] = ParentScope;
 Jumps.push_back(S);
 break;


Index: clang/lib/Sema/JumpDiagnostics.cpp
===
--- clang/lib/Sema/JumpDiagnostics.cpp
+++ clang/lib/Sema/JumpDiagnostics.cpp
@@ -331,11 +331,8 @@
 // operand (to avoid recording the address-of-label use), which
 // works only because of the restricted set of expressions which
 // we detect as constant targets.
-if (cast(S)->getConstantTarget()) {
-  LabelAndGotoScopes[S] = ParentScope;
-  Jumps.push_back(S);
-  return;
-}
+if (cast(S)->getConstantTarget())
+  goto RecordJumpScope;
 
 LabelAndGotoScopes[S] = ParentScope;
 IndirectJumps.push_back(S);
@@ -352,15 +349,17 @@
   BuildScopeInformation(Var, ParentScope);
   ++StmtsToSkip;
 }
-[[fallthrough]];
+goto RecordJumpScope;
 
-  case Stmt::GotoStmtClass:
   case Stmt::GCCAsmStmtClass:
-if (auto *GS = dyn_cast(S))
-  if (!GS->isAsmGoto())
-break;
+if (!cast(S)->isAsmGoto())
+  break;
+goto RecordJumpScope;
+
+  case Stmt::GotoStmtClass:
 // Remember both what scope a goto is in as well as the fact that we have
 // it.  This makes the second scan not have to walk the AST again.
+RecordJumpScope:
 LabelAndGotoScopes[S] = ParentScope;
 Jumps.push_back(S);
 break;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits