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

Author: vbvictor

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", false)) {}
+
+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<GotoStmt> Anything = anything();
+
+  Finder->addMatcher(gotoStmt(IgnoreMacros ? unless(isInMacro()) : Anything,
+                              anyOf(unless(hasAncestor(NestedLoop)),
                                     unless(isForwardJumping())))
                          .bind("goto"),
                      this);

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

clang-tools-extra/docs/ReleaseNotes.rst

@@ -203,11 +203,19 @@ Changes in existing checks
   <clang-tidy/checks/concurrency/mt-unsafe>` check by fixing a false positive
   where ``strerror`` was flagged as MT-unsafe.
 
+- Improved :doc:`cppcoreguidelines-avoid-goto
+  <clang-tidy/checks/cppcoreguidelines/avoid-goto>` check by adding the option
+  `IgnoreMacros` to ignore ``goto`` labels defined in macros.
+
 - Improved :doc:`google-readability-namespace-comments
   <clang-tidy/checks/google/readability-namespace-comments>` check by adding
   the option `AllowOmittingNamespaceComments` to accept if a namespace comment
   is omitted entirely.
 
+- Improved :doc:`hicpp-avoid-goto
+  <clang-tidy/checks/hicpp/avoid-goto>` check by adding the option
+  `IgnoreMacros` to ignore ``goto`` labels defined in macros.
+
 - Improved :doc:`llvm-namespace-comment
   <clang-tidy/checks/llvm/namespace-comment>` check by adding the option
   `AllowOmittingNamespaceComments` to accept if a namespace comment is omitted

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 a ``goto`` statement is
+   expanded from a macro. Default is `false`.

clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-goto.cpp

@@ -1,27 +1,28 @@
-// RUN: %check_clang_tidy %s cppcoreguidelines-avoid-goto %t
+// RUN: %check_clang_tidy -check-suffix=,MACRO %s cppcoreguidelines-avoid-goto %t
+// RUN: %check_clang_tidy %s cppcoreguidelines-avoid-goto %t -- -config="{CheckOptions: { cppcoreguidelines-avoid-goto.IgnoreMacros: true }}"
 
 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: [[@LINE-1]]:3: warning: avoid using 'goto' for flow control
+  // CHECK-MESSAGES: [[@LINE+3]]:1: note: label defined here
   noop();
 
 jump_to_me:;
 
 jump_backwards:;
   noop();
   goto jump_backwards;
-  // CHECK-NOTES: [[@LINE-1]]:3: warning: avoid using 'goto' for flow control
-  // CHECK-NOTES: [[@LINE-4]]:1: note: label defined here
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: avoid using 'goto' for flow control
+  // CHECK-MESSAGES: [[@LINE-4]]:1: note: label defined here
 
   goto jump_in_line;
   ;
 jump_in_line:;
-  // CHECK-NOTES: [[@LINE-3]]:3: warning: avoid using 'goto' for flow control
-  // CHECK-NOTES: [[@LINE-2]]:1: note: label defined here
+  // CHECK-MESSAGES: [[@LINE-3]]:3: warning: avoid using 'goto' for flow control
+  // CHECK-MESSAGES: [[@LINE-2]]:1: note: label defined here
 
   // Test the GNU extension https://gcc.gnu.org/onlinedocs/gcc/Labels-as-Values.html
 some_label:;
@@ -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
+  noop();
+  jump_to_me:;
+}