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