nickdesaulniers added inline comments.

================
Comment at: clang/lib/Sema/JumpDiagnostics.cpp:361
+      if (!GS->isAsmGoto())
+        break;
     // Remember both what scope a goto is in as well as the fact that we have
----------------
nickdesaulniers wrote:
> nickdesaulniers wrote:
> > rjmccall wrote:
> > > nickdesaulniers wrote:
> > > > rjmccall wrote:
> > > > > You can pull the `GCCAsmStmtClass` case right above this, make the 
> > > > > cast unconditional (`if (!cast<GCCAsmStmt>(S)->isAsmGoto()) break;`), 
> > > > > and then fall through into the GotoStmt case.
> > > > I could hoist + use `[[fallthrough]]` but note that the case above 
> > > > `Stmt::SwitchStmtClass` also currently uses `[[fallthrough]]` here as 
> > > > well; so I would still need the `dyn_cast`.
> > > > 
> > > > With that in mind, do you still prefer the hoisting? I don't have a 
> > > > preference, but wanted to triple check that with you.
> > > Ah, right.  Personally, I would probably put this common path in a helper 
> > > function, but you could also just duplicate it since it's so small.  This 
> > > also seems like an acceptable use-case for `goto` if you can stomach that 
> > > — with a good label, it should still be very readable.
> > Is https://reviews.llvm.org/D155522 what you had in mind? If so, then I'll 
> > squash it into this, otherwise can you state differently what you had in 
> > mind?
> oh, rereading what you wrote, I think I understand what you mean with 
> `goto`s. Let me post a diff of that, for clarification.
https://reviews.llvm.org/D155525 perhaps?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155342

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

Reply via email to