[clang-tools-extra] [clang-tidy] add 'IgnoreMarcos' option to 'avoid-goto' check (PR #143554)
https://github.com/vbvictor closed https://github.com/llvm/llvm-project/pull/143554 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] add 'IgnoreMarcos' option to 'avoid-goto' check (PR #143554)
https://github.com/vbvictor updated https://github.com/llvm/llvm-project/pull/143554 >From 5c975c6b59c02b0464a9bfc1b424b89b4d7dd662 Mon Sep 17 00:00:00 2001 From: Victor Baranov Date: Tue, 10 Jun 2025 18:27:12 +0300 Subject: [PATCH 1/4] [clang-tidy] add 'IgnoreMarcos' option to 'avoid-goto' check --- .../cppcoreguidelines/AvoidGotoCheck.cpp | 17 ++- .../cppcoreguidelines/AvoidGotoCheck.h| 7 ++- .../checks/cppcoreguidelines/avoid-goto.rst | 9 .../checkers/cppcoreguidelines/avoid-goto.cpp | 50 --- 4 files changed, 72 insertions(+), 11 deletions(-) diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidGotoCheck.cpp b/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidGotoCheck.cpp index 8ffa44d41fa98..00864b1d69618 100644 --- a/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidGotoCheck.cpp +++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidGotoCheck.cpp @@ -17,8 +17,20 @@ namespace { AST_MATCHER(GotoStmt, isForwardJumping) { return Node.getBeginLoc() < Node.getLabel()->getBeginLoc(); } + +AST_MATCHER(GotoStmt, isInMacro) { + return Node.getBeginLoc().isMacroID() && Node.getEndLoc().isMacroID(); +} } // namespace +AvoidGotoCheck::AvoidGotoCheck(StringRef Name, ClangTidyContext *Context) +: ClangTidyCheck(Name, Context), + IgnoreMacros(Options.get("IgnoreMacros", true)) {} + +void AvoidGotoCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { + Options.store(Opts, "IgnoreMacros", IgnoreMacros); +} + void AvoidGotoCheck::registerMatchers(MatchFinder *Finder) { // TODO: This check does not recognize `IndirectGotoStmt` which is a // GNU extension. These must be matched separately and an AST matcher @@ -29,7 +41,10 @@ void AvoidGotoCheck::registerMatchers(MatchFinder *Finder) { auto Loop = mapAnyOf(forStmt, cxxForRangeStmt, whileStmt, doStmt); auto NestedLoop = Loop.with(hasAncestor(Loop)); - Finder->addMatcher(gotoStmt(anyOf(unless(hasAncestor(NestedLoop)), + const ast_matchers::internal::Matcher Anything = anything(); + + Finder->addMatcher(gotoStmt(IgnoreMacros ? unless(isInMacro()) : Anything, + anyOf(unless(hasAncestor(NestedLoop)), unless(isForwardJumping( .bind("goto"), this); diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidGotoCheck.h b/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidGotoCheck.h index 883ba78855e7c..8eae409462c91 100644 --- a/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidGotoCheck.h +++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidGotoCheck.h @@ -20,13 +20,16 @@ namespace clang::tidy::cppcoreguidelines { /// http://clang.llvm.org/extra/clang-tidy/checks/cppcoreguidelines/avoid-goto.html class AvoidGotoCheck : public ClangTidyCheck { public: - AvoidGotoCheck(StringRef Name, ClangTidyContext *Context) - : ClangTidyCheck(Name, Context) {} + AvoidGotoCheck(StringRef Name, ClangTidyContext *Context); bool isLanguageVersionSupported(const LangOptions &LangOpts) const override { return LangOpts.CPlusPlus; } + void storeOptions(ClangTidyOptions::OptionMap &Opts) override; void registerMatchers(ast_matchers::MatchFinder *Finder) override; void check(const ast_matchers::MatchFinder::MatchResult &Result) override; + +private: + const bool IgnoreMacros; }; } // namespace clang::tidy::cppcoreguidelines diff --git a/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/avoid-goto.rst b/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/avoid-goto.rst index 71b579a4ae99e..823b3c376e6d0 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/avoid-goto.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/avoid-goto.rst @@ -50,3 +50,12 @@ Modern C++ needs ``goto`` only to jump out of nested loops. some_operation(); All other uses of ``goto`` are diagnosed in `C++`. + + +Options +--- + +.. option:: IgnoreMacros + + If set to `true`, the check will not warn if both label and ``goto`` + statement are placed inside a macro. Default is `true`. diff --git a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-goto.cpp b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-goto.cpp index ee236bc44695a..9a95b21cf3411 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-goto.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-goto.cpp @@ -1,12 +1,13 @@ // RUN: %check_clang_tidy %s cppcoreguidelines-avoid-goto %t +// RUN: %check_clang_tidy -check-suffix=,MACRO %s cppcoreguidelines-avoid-goto %t -- -config="{CheckOptions: { cppcoreguidelines-avoid-goto.IgnoreMacros: false }}" void noop() {} int main() { noop(); goto jump_to_me; - // CHECK-NOTES: [[@LINE-1]]:3: warning: avoid using 'goto' for flow control - // CHECK-NOTES: [[@LINE+3]]:1: note: label defined he
[clang-tools-extra] [clang-tidy] add 'IgnoreMarcos' option to 'avoid-goto' check (PR #143554)
https://github.com/5chmidti edited https://github.com/llvm/llvm-project/pull/143554 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] add 'IgnoreMarcos' option to 'avoid-goto' check (PR #143554)
https://github.com/5chmidti approved this pull request. LGTM minus comments https://github.com/llvm/llvm-project/pull/143554 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] add 'IgnoreMarcos' option to 'avoid-goto' check (PR #143554)
@@ -1,27 +1,34 @@ // RUN: %check_clang_tidy %s cppcoreguidelines-avoid-goto %t +// RUN: %check_clang_tidy -check-suffix=MACRO %s cppcoreguidelines-avoid-goto %t -- -config="{CheckOptions: { cppcoreguidelines-avoid-goto.IgnoreMacros: true }}" vbvictor wrote: It'll be slightly harder, If I write `check-suffix=,MACRO` for `IgnoreMacros: true` it will try to match all default `chech-messages` that does not ignore macros. For your suggestion to work, I should write default `CHECK-MESSAGE` with `IgnoreMacros: true` and then add more cases provided by `IgnoreMacros: false`. I'll do that. https://github.com/llvm/llvm-project/pull/143554 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] add 'IgnoreMarcos' option to 'avoid-goto' check (PR #143554)
@@ -50,3 +50,12 @@ Modern C++ needs ``goto`` only to jump out of nested loops. some_operation(); All other uses of ``goto`` are diagnosed in `C++`. + + +Options +--- + +.. option:: IgnoreMacros + + If set to `true`, the check will not warn if ``goto`` statement is placed 5chmidti wrote: I think 'expanded from a macro ' sounds better than 'placed' --- 'if a go-to statement' https://github.com/llvm/llvm-project/pull/143554 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] add 'IgnoreMarcos' option to 'avoid-goto' check (PR #143554)
@@ -1,27 +1,34 @@ // RUN: %check_clang_tidy %s cppcoreguidelines-avoid-goto %t +// RUN: %check_clang_tidy -check-suffix=MACRO %s cppcoreguidelines-avoid-goto %t -- -config="{CheckOptions: { cppcoreguidelines-avoid-goto.IgnoreMacros: true }}" 5chmidti wrote: You don't have to duplicate all dogs for this. You can add a '-MACRO' suffix for the true case and have that match -check-suffix=,MACRO. The empty suffix before the comma will match the dishes that are already here https://github.com/llvm/llvm-project/pull/143554 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] add 'IgnoreMarcos' option to 'avoid-goto' check (PR #143554)
https://github.com/carlosgalvezp approved this pull request. LGTM! https://github.com/llvm/llvm-project/pull/143554 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] add 'IgnoreMarcos' option to 'avoid-goto' check (PR #143554)
https://github.com/vbvictor updated https://github.com/llvm/llvm-project/pull/143554 >From 5c975c6b59c02b0464a9bfc1b424b89b4d7dd662 Mon Sep 17 00:00:00 2001 From: Victor Baranov Date: Tue, 10 Jun 2025 18:27:12 +0300 Subject: [PATCH 1/3] [clang-tidy] add 'IgnoreMarcos' option to 'avoid-goto' check --- .../cppcoreguidelines/AvoidGotoCheck.cpp | 17 ++- .../cppcoreguidelines/AvoidGotoCheck.h| 7 ++- .../checks/cppcoreguidelines/avoid-goto.rst | 9 .../checkers/cppcoreguidelines/avoid-goto.cpp | 50 --- 4 files changed, 72 insertions(+), 11 deletions(-) diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidGotoCheck.cpp b/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidGotoCheck.cpp index 8ffa44d41fa98..00864b1d69618 100644 --- a/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidGotoCheck.cpp +++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidGotoCheck.cpp @@ -17,8 +17,20 @@ namespace { AST_MATCHER(GotoStmt, isForwardJumping) { return Node.getBeginLoc() < Node.getLabel()->getBeginLoc(); } + +AST_MATCHER(GotoStmt, isInMacro) { + return Node.getBeginLoc().isMacroID() && Node.getEndLoc().isMacroID(); +} } // namespace +AvoidGotoCheck::AvoidGotoCheck(StringRef Name, ClangTidyContext *Context) +: ClangTidyCheck(Name, Context), + IgnoreMacros(Options.get("IgnoreMacros", true)) {} + +void AvoidGotoCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { + Options.store(Opts, "IgnoreMacros", IgnoreMacros); +} + void AvoidGotoCheck::registerMatchers(MatchFinder *Finder) { // TODO: This check does not recognize `IndirectGotoStmt` which is a // GNU extension. These must be matched separately and an AST matcher @@ -29,7 +41,10 @@ void AvoidGotoCheck::registerMatchers(MatchFinder *Finder) { auto Loop = mapAnyOf(forStmt, cxxForRangeStmt, whileStmt, doStmt); auto NestedLoop = Loop.with(hasAncestor(Loop)); - Finder->addMatcher(gotoStmt(anyOf(unless(hasAncestor(NestedLoop)), + const ast_matchers::internal::Matcher Anything = anything(); + + Finder->addMatcher(gotoStmt(IgnoreMacros ? unless(isInMacro()) : Anything, + anyOf(unless(hasAncestor(NestedLoop)), unless(isForwardJumping( .bind("goto"), this); diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidGotoCheck.h b/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidGotoCheck.h index 883ba78855e7c..8eae409462c91 100644 --- a/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidGotoCheck.h +++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidGotoCheck.h @@ -20,13 +20,16 @@ namespace clang::tidy::cppcoreguidelines { /// http://clang.llvm.org/extra/clang-tidy/checks/cppcoreguidelines/avoid-goto.html class AvoidGotoCheck : public ClangTidyCheck { public: - AvoidGotoCheck(StringRef Name, ClangTidyContext *Context) - : ClangTidyCheck(Name, Context) {} + AvoidGotoCheck(StringRef Name, ClangTidyContext *Context); bool isLanguageVersionSupported(const LangOptions &LangOpts) const override { return LangOpts.CPlusPlus; } + void storeOptions(ClangTidyOptions::OptionMap &Opts) override; void registerMatchers(ast_matchers::MatchFinder *Finder) override; void check(const ast_matchers::MatchFinder::MatchResult &Result) override; + +private: + const bool IgnoreMacros; }; } // namespace clang::tidy::cppcoreguidelines diff --git a/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/avoid-goto.rst b/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/avoid-goto.rst index 71b579a4ae99e..823b3c376e6d0 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/avoid-goto.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/avoid-goto.rst @@ -50,3 +50,12 @@ Modern C++ needs ``goto`` only to jump out of nested loops. some_operation(); All other uses of ``goto`` are diagnosed in `C++`. + + +Options +--- + +.. option:: IgnoreMacros + + If set to `true`, the check will not warn if both label and ``goto`` + statement are placed inside a macro. Default is `true`. diff --git a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-goto.cpp b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-goto.cpp index ee236bc44695a..9a95b21cf3411 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-goto.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-goto.cpp @@ -1,12 +1,13 @@ // RUN: %check_clang_tidy %s cppcoreguidelines-avoid-goto %t +// RUN: %check_clang_tidy -check-suffix=,MACRO %s cppcoreguidelines-avoid-goto %t -- -config="{CheckOptions: { cppcoreguidelines-avoid-goto.IgnoreMacros: false }}" void noop() {} int main() { noop(); goto jump_to_me; - // CHECK-NOTES: [[@LINE-1]]:3: warning: avoid using 'goto' for flow control - // CHECK-NOTES: [[@LINE+3]]:1: note: label defined he
[clang-tools-extra] [clang-tidy] add 'IgnoreMarcos' option to 'avoid-goto' check (PR #143554)
@@ -132,8 +133,41 @@ void jump_out_backwards() { for (int j = 0; j < 10; ++j) { if (i * j > 80) goto before_the_loop; - // CHECK-NOTES: [[@LINE-1]]:9: warning: avoid using 'goto' for flow control - // CHECK-NOTES: [[@LINE-8]]:1: note: label defined here + // CHECK-MESSAGES: [[@LINE-1]]:9: warning: avoid using 'goto' for flow control + // CHECK-MESSAGES: [[@LINE-8]]:1: note: label defined here } } } + +#define macro_goto_code \ + noop(); \ + goto jump_to_me; \ + noop(); \ +jump_to_me:; \ + +#define macro_goto_label jump_to_me:; +#define macro_goto_jump goto jump_to_me; + +void inside_macro_all() { + macro_goto_code + // CHECK-MESSAGES-MACRO: [[@LINE-1]]:3: warning: avoid using 'goto' for flow control + // CHECK-MESSAGES-MACRO: [[@LINE-2]]:3: note: label defined here +} + +void inside_macro_label() { + noop(); + goto jump_to_me; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: avoid using 'goto' for flow control + // CHECK-MESSAGES: [[@LINE+2]]:3: note: label defined here + noop(); + macro_goto_label +} + +void inside_macro_goto() { + noop(); + macro_goto_jump + // CHECK-MESSAGES-MACRO: [[@LINE-1]]:3: warning: avoid using 'goto' for flow control + // CHECK-MESSAGES-MACRO: [[@LINE+2]]:3: note: label defined here vbvictor wrote: Changed docs to match current behavior. We warn even if labels is not in macro because `goto` is the main "evil" here. https://github.com/llvm/llvm-project/pull/143554 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] add 'IgnoreMarcos' option to 'avoid-goto' check (PR #143554)
@@ -50,3 +50,12 @@ Modern C++ needs ``goto`` only to jump out of nested loops. some_operation(); All other uses of ``goto`` are diagnosed in `C++`. + + +Options +--- + +.. option:: IgnoreMacros + + If set to `true`, the check will not warn if both label and ``goto`` + statement are placed inside a macro. Default is `true`. vbvictor wrote: Agreed, changed to `false` by default. https://github.com/llvm/llvm-project/pull/143554 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] add 'IgnoreMarcos' option to 'avoid-goto' check (PR #143554)
@@ -50,3 +50,12 @@ Modern C++ needs ``goto`` only to jump out of nested loops. some_operation(); All other uses of ``goto`` are diagnosed in `C++`. + + +Options +--- + +.. option:: IgnoreMacros + + If set to `true`, the check will not warn if both label and ``goto`` + statement are placed inside a macro. Default is `true`. carlosgalvezp wrote: Since `goto` is such a "bad" thing in C++, I'd be inclined to have this `false` by default (which also keeps the same functionality as on trunk). https://github.com/llvm/llvm-project/pull/143554 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] add 'IgnoreMarcos' option to 'avoid-goto' check (PR #143554)
@@ -132,8 +133,41 @@ void jump_out_backwards() { for (int j = 0; j < 10; ++j) { if (i * j > 80) goto before_the_loop; - // CHECK-NOTES: [[@LINE-1]]:9: warning: avoid using 'goto' for flow control - // CHECK-NOTES: [[@LINE-8]]:1: note: label defined here + // CHECK-MESSAGES: [[@LINE-1]]:9: warning: avoid using 'goto' for flow control + // CHECK-MESSAGES: [[@LINE-8]]:1: note: label defined here } } } + +#define macro_goto_code \ + noop(); \ + goto jump_to_me; \ + noop(); \ +jump_to_me:; \ + +#define macro_goto_label jump_to_me:; +#define macro_goto_jump goto jump_to_me; + +void inside_macro_all() { + macro_goto_code + // CHECK-MESSAGES-MACRO: [[@LINE-1]]:3: warning: avoid using 'goto' for flow control + // CHECK-MESSAGES-MACRO: [[@LINE-2]]:3: note: label defined here +} + +void inside_macro_label() { + noop(); + goto jump_to_me; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: avoid using 'goto' for flow control + // CHECK-MESSAGES: [[@LINE+2]]:3: note: label defined here + noop(); + macro_goto_label +} + +void inside_macro_goto() { + noop(); + macro_goto_jump + // CHECK-MESSAGES-MACRO: [[@LINE-1]]:3: warning: avoid using 'goto' for flow control + // CHECK-MESSAGES-MACRO: [[@LINE+2]]:3: note: label defined here vbvictor wrote: Thank you for the catch, my initial suggestion would be to ignore if both `goto` and `label` are in macro but later relaxed only to `goto` in macros. What do you think will be better, behavior like in doc or like in actual tests? https://github.com/llvm/llvm-project/pull/143554 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] add 'IgnoreMarcos' option to 'avoid-goto' check (PR #143554)
@@ -132,8 +133,41 @@ void jump_out_backwards() { for (int j = 0; j < 10; ++j) { if (i * j > 80) goto before_the_loop; - // CHECK-NOTES: [[@LINE-1]]:9: warning: avoid using 'goto' for flow control - // CHECK-NOTES: [[@LINE-8]]:1: note: label defined here + // CHECK-MESSAGES: [[@LINE-1]]:9: warning: avoid using 'goto' for flow control + // CHECK-MESSAGES: [[@LINE-8]]:1: note: label defined here } } } + +#define macro_goto_code \ + noop(); \ + goto jump_to_me; \ + noop(); \ +jump_to_me:; \ + +#define macro_goto_label jump_to_me:; +#define macro_goto_jump goto jump_to_me; + +void inside_macro_all() { + macro_goto_code + // CHECK-MESSAGES-MACRO: [[@LINE-1]]:3: warning: avoid using 'goto' for flow control + // CHECK-MESSAGES-MACRO: [[@LINE-2]]:3: note: label defined here +} + +void inside_macro_label() { + noop(); + goto jump_to_me; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: avoid using 'goto' for flow control + // CHECK-MESSAGES: [[@LINE+2]]:3: note: label defined here + noop(); + macro_goto_label +} + +void inside_macro_goto() { + noop(); + macro_goto_jump + // CHECK-MESSAGES-MACRO: [[@LINE-1]]:3: warning: avoid using 'goto' for flow control + // CHECK-MESSAGES-MACRO: [[@LINE+2]]:3: note: label defined here HerrCai0907 wrote: > If set to `true`, the check will not warn if both label and ``goto`` > statement are placed inside a macro. Default is `true`. According to doc, it should be warn also since label is not in macro. https://github.com/llvm/llvm-project/pull/143554 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] add 'IgnoreMarcos' option to 'avoid-goto' check (PR #143554)
https://github.com/vbvictor updated https://github.com/llvm/llvm-project/pull/143554 >From 5c975c6b59c02b0464a9bfc1b424b89b4d7dd662 Mon Sep 17 00:00:00 2001 From: Victor Baranov Date: Tue, 10 Jun 2025 18:27:12 +0300 Subject: [PATCH 1/2] [clang-tidy] add 'IgnoreMarcos' option to 'avoid-goto' check --- .../cppcoreguidelines/AvoidGotoCheck.cpp | 17 ++- .../cppcoreguidelines/AvoidGotoCheck.h| 7 ++- .../checks/cppcoreguidelines/avoid-goto.rst | 9 .../checkers/cppcoreguidelines/avoid-goto.cpp | 50 --- 4 files changed, 72 insertions(+), 11 deletions(-) diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidGotoCheck.cpp b/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidGotoCheck.cpp index 8ffa44d41fa98..00864b1d69618 100644 --- a/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidGotoCheck.cpp +++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidGotoCheck.cpp @@ -17,8 +17,20 @@ namespace { AST_MATCHER(GotoStmt, isForwardJumping) { return Node.getBeginLoc() < Node.getLabel()->getBeginLoc(); } + +AST_MATCHER(GotoStmt, isInMacro) { + return Node.getBeginLoc().isMacroID() && Node.getEndLoc().isMacroID(); +} } // namespace +AvoidGotoCheck::AvoidGotoCheck(StringRef Name, ClangTidyContext *Context) +: ClangTidyCheck(Name, Context), + IgnoreMacros(Options.get("IgnoreMacros", true)) {} + +void AvoidGotoCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { + Options.store(Opts, "IgnoreMacros", IgnoreMacros); +} + void AvoidGotoCheck::registerMatchers(MatchFinder *Finder) { // TODO: This check does not recognize `IndirectGotoStmt` which is a // GNU extension. These must be matched separately and an AST matcher @@ -29,7 +41,10 @@ void AvoidGotoCheck::registerMatchers(MatchFinder *Finder) { auto Loop = mapAnyOf(forStmt, cxxForRangeStmt, whileStmt, doStmt); auto NestedLoop = Loop.with(hasAncestor(Loop)); - Finder->addMatcher(gotoStmt(anyOf(unless(hasAncestor(NestedLoop)), + const ast_matchers::internal::Matcher Anything = anything(); + + Finder->addMatcher(gotoStmt(IgnoreMacros ? unless(isInMacro()) : Anything, + anyOf(unless(hasAncestor(NestedLoop)), unless(isForwardJumping( .bind("goto"), this); diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidGotoCheck.h b/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidGotoCheck.h index 883ba78855e7c..8eae409462c91 100644 --- a/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidGotoCheck.h +++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidGotoCheck.h @@ -20,13 +20,16 @@ namespace clang::tidy::cppcoreguidelines { /// http://clang.llvm.org/extra/clang-tidy/checks/cppcoreguidelines/avoid-goto.html class AvoidGotoCheck : public ClangTidyCheck { public: - AvoidGotoCheck(StringRef Name, ClangTidyContext *Context) - : ClangTidyCheck(Name, Context) {} + AvoidGotoCheck(StringRef Name, ClangTidyContext *Context); bool isLanguageVersionSupported(const LangOptions &LangOpts) const override { return LangOpts.CPlusPlus; } + void storeOptions(ClangTidyOptions::OptionMap &Opts) override; void registerMatchers(ast_matchers::MatchFinder *Finder) override; void check(const ast_matchers::MatchFinder::MatchResult &Result) override; + +private: + const bool IgnoreMacros; }; } // namespace clang::tidy::cppcoreguidelines diff --git a/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/avoid-goto.rst b/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/avoid-goto.rst index 71b579a4ae99e..823b3c376e6d0 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/avoid-goto.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/avoid-goto.rst @@ -50,3 +50,12 @@ Modern C++ needs ``goto`` only to jump out of nested loops. some_operation(); All other uses of ``goto`` are diagnosed in `C++`. + + +Options +--- + +.. option:: IgnoreMacros + + If set to `true`, the check will not warn if both label and ``goto`` + statement are placed inside a macro. Default is `true`. diff --git a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-goto.cpp b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-goto.cpp index ee236bc44695a..9a95b21cf3411 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-goto.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-goto.cpp @@ -1,12 +1,13 @@ // RUN: %check_clang_tidy %s cppcoreguidelines-avoid-goto %t +// RUN: %check_clang_tidy -check-suffix=,MACRO %s cppcoreguidelines-avoid-goto %t -- -config="{CheckOptions: { cppcoreguidelines-avoid-goto.IgnoreMacros: false }}" void noop() {} int main() { noop(); goto jump_to_me; - // CHECK-NOTES: [[@LINE-1]]:3: warning: avoid using 'goto' for flow control - // CHECK-NOTES: [[@LINE+3]]:1: note: label defined he
[clang-tools-extra] [clang-tidy] add 'IgnoreMarcos' option to 'avoid-goto' check (PR #143554)
llvmbot wrote: @llvm/pr-subscribers-clang-tidy Author: Baranov Victor (vbvictor) Changes --- Full diff: https://github.com/llvm/llvm-project/pull/143554.diff 4 Files Affected: - (modified) clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidGotoCheck.cpp (+16-1) - (modified) clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidGotoCheck.h (+5-2) - (modified) clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/avoid-goto.rst (+9) - (modified) clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-goto.cpp (+42-8) ``diff diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidGotoCheck.cpp b/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidGotoCheck.cpp index 8ffa44d41fa98..00864b1d69618 100644 --- a/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidGotoCheck.cpp +++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidGotoCheck.cpp @@ -17,8 +17,20 @@ namespace { AST_MATCHER(GotoStmt, isForwardJumping) { return Node.getBeginLoc() < Node.getLabel()->getBeginLoc(); } + +AST_MATCHER(GotoStmt, isInMacro) { + return Node.getBeginLoc().isMacroID() && Node.getEndLoc().isMacroID(); +} } // namespace +AvoidGotoCheck::AvoidGotoCheck(StringRef Name, ClangTidyContext *Context) +: ClangTidyCheck(Name, Context), + IgnoreMacros(Options.get("IgnoreMacros", true)) {} + +void AvoidGotoCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { + Options.store(Opts, "IgnoreMacros", IgnoreMacros); +} + void AvoidGotoCheck::registerMatchers(MatchFinder *Finder) { // TODO: This check does not recognize `IndirectGotoStmt` which is a // GNU extension. These must be matched separately and an AST matcher @@ -29,7 +41,10 @@ void AvoidGotoCheck::registerMatchers(MatchFinder *Finder) { auto Loop = mapAnyOf(forStmt, cxxForRangeStmt, whileStmt, doStmt); auto NestedLoop = Loop.with(hasAncestor(Loop)); - Finder->addMatcher(gotoStmt(anyOf(unless(hasAncestor(NestedLoop)), + const ast_matchers::internal::Matcher Anything = anything(); + + Finder->addMatcher(gotoStmt(IgnoreMacros ? unless(isInMacro()) : Anything, + anyOf(unless(hasAncestor(NestedLoop)), unless(isForwardJumping( .bind("goto"), this); diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidGotoCheck.h b/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidGotoCheck.h index 883ba78855e7c..8eae409462c91 100644 --- a/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidGotoCheck.h +++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidGotoCheck.h @@ -20,13 +20,16 @@ namespace clang::tidy::cppcoreguidelines { /// http://clang.llvm.org/extra/clang-tidy/checks/cppcoreguidelines/avoid-goto.html class AvoidGotoCheck : public ClangTidyCheck { public: - AvoidGotoCheck(StringRef Name, ClangTidyContext *Context) - : ClangTidyCheck(Name, Context) {} + AvoidGotoCheck(StringRef Name, ClangTidyContext *Context); bool isLanguageVersionSupported(const LangOptions &LangOpts) const override { return LangOpts.CPlusPlus; } + void storeOptions(ClangTidyOptions::OptionMap &Opts) override; void registerMatchers(ast_matchers::MatchFinder *Finder) override; void check(const ast_matchers::MatchFinder::MatchResult &Result) override; + +private: + const bool IgnoreMacros; }; } // namespace clang::tidy::cppcoreguidelines diff --git a/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/avoid-goto.rst b/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/avoid-goto.rst index 71b579a4ae99e..823b3c376e6d0 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/avoid-goto.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/avoid-goto.rst @@ -50,3 +50,12 @@ Modern C++ needs ``goto`` only to jump out of nested loops. some_operation(); All other uses of ``goto`` are diagnosed in `C++`. + + +Options +--- + +.. option:: IgnoreMacros + + If set to `true`, the check will not warn if both label and ``goto`` + statement are placed inside a macro. Default is `true`. diff --git a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-goto.cpp b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-goto.cpp index ee236bc44695a..9a95b21cf3411 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-goto.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-goto.cpp @@ -1,12 +1,13 @@ // RUN: %check_clang_tidy %s cppcoreguidelines-avoid-goto %t +// RUN: %check_clang_tidy -check-suffix=,MACRO %s cppcoreguidelines-avoid-goto %t -- -config="{CheckOptions: { cppcoreguidelines-avoid-goto.IgnoreMacros: false }}" void noop() {} int main() { noop(); goto jump_to_me; - // CHECK-NOTES: [[@LINE-1]]:3: warning: avoid using 'goto' for flow control - // CHECK-NOTES: [[@LINE+3]]:1: note: label defined here + // CHECK-MESSAGES: [[@LI
[clang-tools-extra] [clang-tidy] add 'IgnoreMarcos' option to 'avoid-goto' check (PR #143554)
https://github.com/vbvictor created https://github.com/llvm/llvm-project/pull/143554 None >From 5c975c6b59c02b0464a9bfc1b424b89b4d7dd662 Mon Sep 17 00:00:00 2001 From: Victor Baranov Date: Tue, 10 Jun 2025 18:27:12 +0300 Subject: [PATCH] [clang-tidy] add 'IgnoreMarcos' option to 'avoid-goto' check --- .../cppcoreguidelines/AvoidGotoCheck.cpp | 17 ++- .../cppcoreguidelines/AvoidGotoCheck.h| 7 ++- .../checks/cppcoreguidelines/avoid-goto.rst | 9 .../checkers/cppcoreguidelines/avoid-goto.cpp | 50 --- 4 files changed, 72 insertions(+), 11 deletions(-) diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidGotoCheck.cpp b/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidGotoCheck.cpp index 8ffa44d41fa98..00864b1d69618 100644 --- a/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidGotoCheck.cpp +++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidGotoCheck.cpp @@ -17,8 +17,20 @@ namespace { AST_MATCHER(GotoStmt, isForwardJumping) { return Node.getBeginLoc() < Node.getLabel()->getBeginLoc(); } + +AST_MATCHER(GotoStmt, isInMacro) { + return Node.getBeginLoc().isMacroID() && Node.getEndLoc().isMacroID(); +} } // namespace +AvoidGotoCheck::AvoidGotoCheck(StringRef Name, ClangTidyContext *Context) +: ClangTidyCheck(Name, Context), + IgnoreMacros(Options.get("IgnoreMacros", true)) {} + +void AvoidGotoCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { + Options.store(Opts, "IgnoreMacros", IgnoreMacros); +} + void AvoidGotoCheck::registerMatchers(MatchFinder *Finder) { // TODO: This check does not recognize `IndirectGotoStmt` which is a // GNU extension. These must be matched separately and an AST matcher @@ -29,7 +41,10 @@ void AvoidGotoCheck::registerMatchers(MatchFinder *Finder) { auto Loop = mapAnyOf(forStmt, cxxForRangeStmt, whileStmt, doStmt); auto NestedLoop = Loop.with(hasAncestor(Loop)); - Finder->addMatcher(gotoStmt(anyOf(unless(hasAncestor(NestedLoop)), + const ast_matchers::internal::Matcher Anything = anything(); + + Finder->addMatcher(gotoStmt(IgnoreMacros ? unless(isInMacro()) : Anything, + anyOf(unless(hasAncestor(NestedLoop)), unless(isForwardJumping( .bind("goto"), this); diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidGotoCheck.h b/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidGotoCheck.h index 883ba78855e7c..8eae409462c91 100644 --- a/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidGotoCheck.h +++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidGotoCheck.h @@ -20,13 +20,16 @@ namespace clang::tidy::cppcoreguidelines { /// http://clang.llvm.org/extra/clang-tidy/checks/cppcoreguidelines/avoid-goto.html class AvoidGotoCheck : public ClangTidyCheck { public: - AvoidGotoCheck(StringRef Name, ClangTidyContext *Context) - : ClangTidyCheck(Name, Context) {} + AvoidGotoCheck(StringRef Name, ClangTidyContext *Context); bool isLanguageVersionSupported(const LangOptions &LangOpts) const override { return LangOpts.CPlusPlus; } + void storeOptions(ClangTidyOptions::OptionMap &Opts) override; void registerMatchers(ast_matchers::MatchFinder *Finder) override; void check(const ast_matchers::MatchFinder::MatchResult &Result) override; + +private: + const bool IgnoreMacros; }; } // namespace clang::tidy::cppcoreguidelines diff --git a/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/avoid-goto.rst b/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/avoid-goto.rst index 71b579a4ae99e..823b3c376e6d0 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/avoid-goto.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/avoid-goto.rst @@ -50,3 +50,12 @@ Modern C++ needs ``goto`` only to jump out of nested loops. some_operation(); All other uses of ``goto`` are diagnosed in `C++`. + + +Options +--- + +.. option:: IgnoreMacros + + If set to `true`, the check will not warn if both label and ``goto`` + statement are placed inside a macro. Default is `true`. diff --git a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-goto.cpp b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-goto.cpp index ee236bc44695a..9a95b21cf3411 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-goto.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-goto.cpp @@ -1,12 +1,13 @@ // RUN: %check_clang_tidy %s cppcoreguidelines-avoid-goto %t +// RUN: %check_clang_tidy -check-suffix=,MACRO %s cppcoreguidelines-avoid-goto %t -- -config="{CheckOptions: { cppcoreguidelines-avoid-goto.IgnoreMacros: false }}" void noop() {} int main() { noop(); goto jump_to_me; - // CHECK-NOTES: [[@LINE-1]]:3: warning: avoid using 'goto' for flow control - // CHECK-NOTES: [[@LINE+3]]:1: note: label defined h