JonasToth added inline comments.
================
Comment at: clang-tidy/cppcoreguidelines/AvoidGotoCheck.cpp:20
+
+AST_MATCHER(GotoStmt, isForwardJumping) {
+
----------------
lebedev.ri wrote:
> It would be nice to have it in standard ASTMatchers, i believe it will be
> useful for `else-after-return` check.
> Though the ASTMatchers are stuck due to the doc-dumping script being 'broken'
> (see D41455)
Yes. The GNU extension goto does not have ASTMatcher yet, so i will pack these
together in a review. What do you think how long the ASTMatcher issue will be
there? Maybe it could be done after that check lands?
================
Comment at: clang-tidy/cppcoreguidelines/AvoidGotoCheck.cpp:41-53
+ if (const auto *BackGoto =
+ Result.Nodes.getNodeAs<GotoStmt>("backward-goto")) {
+ diag(BackGoto->getGotoLoc(), "do not jump backwards with 'goto'")
+ << BackGoto->getSourceRange();
+ diag(BackGoto->getLabel()->getLocStart(), "label defined here",
+ DiagnosticIDs::Note);
+ }
----------------
aaron.ballman wrote:
> Is there a reason to have two separate diagnostics? It seems like these both
> would be covered by "this use of 'goto' for flow control is prohibited".
Yes, the new wording better. The note about the backward jump could be added in
the `label defined here` note. I think distignuishing between backward jumps
and forward jumps is still a good thing.
The forward jumps could come from a C code part where forward jumps are done
for resource cleaning. So having a strong `prohibited` and a suggesting `avoid`
diagnostic makes sense to me.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D41815
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
