[clang-tools-extra] [clang-tidy] add 'IgnoreMarcos' option to 'avoid-goto' check (PR #143554)

2025-06-23 Thread Baranov Victor via cfe-commits

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)

2025-06-21 Thread Baranov Victor via cfe-commits

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)

2025-06-21 Thread Julian Schmidt via cfe-commits

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)

2025-06-21 Thread Julian Schmidt via cfe-commits

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)

2025-06-21 Thread Baranov Victor via cfe-commits


@@ -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)

2025-06-21 Thread Julian Schmidt via cfe-commits


@@ -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)

2025-06-21 Thread Julian Schmidt via cfe-commits


@@ -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)

2025-06-18 Thread Carlos Galvez via cfe-commits

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)

2025-06-15 Thread Baranov Victor via cfe-commits

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)

2025-06-15 Thread Baranov Victor via cfe-commits


@@ -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)

2025-06-15 Thread Baranov Victor via cfe-commits


@@ -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)

2025-06-15 Thread Carlos Galvez via cfe-commits


@@ -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)

2025-06-13 Thread Baranov Victor via cfe-commits


@@ -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)

2025-06-13 Thread Congcong Cai via cfe-commits


@@ -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)

2025-06-10 Thread Baranov Victor via cfe-commits

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)

2025-06-10 Thread via cfe-commits

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)

2025-06-10 Thread Baranov Victor via cfe-commits

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