[clang-tidy] Ignore macros defined within declarations

Modernize-macro-to-enum shouldn't try to convert macros to enums
when they are defined inside a declaration or definition, only
when the macros are defined at the top level.  Since preprocessing
is disconnected from AST traversal, match nodes in the AST and then
invalidate source ranges spanning AST nodes before issuing diagnostics.

ClangTidyCheck::onEndOfTranslationUnit is called before
PPCallbacks::EndOfMainFile, so defer final diagnostics to the
PPCallbacks implementation.

Differential Revision: https://reviews.llvm.org/D124066

Fixes #54883

diff --git a/clang-tools-extra/clang-tidy/modernize/MacroToEnumCheck.cpp b/clang-tools-extra/clang-tidy/modernize/MacroToEnumCheck.cpp
index 90d3fe0..e916668 100644 (file)
--- a/clang-tools-extra/clang-tidy/modernize/MacroToEnumCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/MacroToEnumCheck.cpp
@@ -121,6 +121,8 @@ struct FileState {
   SourceLocation LastMacroLocation;
 };
 
+} // namespace
+
 class MacroToEnumCallbacks : public PPCallbacks {
 public:
   MacroToEnumCallbacks(MacroToEnumCheck *Check, const LangOptions &LangOptions,
@@ -197,6 +199,8 @@ public:
   // After we've seen everything, issue warnings and fix-its.
   void EndOfMainFile() override;
 
+  void invalidateRange(SourceRange Range);
+
 private:
   void newEnum() {
     if (Enums.empty() || !Enums.back().empty())
@@ -224,6 +228,7 @@ private:
   void checkName(const Token &MacroNameTok);
   void rememberExpressionName(const Token &MacroNameTok);
   void invalidateExpressionNames();
+  void issueDiagnostics();
   void warnMacroEnum(const EnumMacro &Macro) const;
   void fixEnumMacro(const MacroList &MacroList) const;
 
@@ -472,8 +477,20 @@ void MacroToEnumCallbacks::invalidateExpressionNames() {
 }
 
 void MacroToEnumCallbacks::EndOfMainFile() {
-  invalidateExpressionNames();
+    invalidateExpressionNames();
+    issueDiagnostics();
+}
 
+void MacroToEnumCallbacks::invalidateRange(SourceRange Range) {
+  llvm::erase_if(Enums, [Range](const MacroList &MacroList) {
+    return llvm::any_of(MacroList, [Range](const EnumMacro &Macro) {
+      return Macro.Directive->getLocation() >= Range.getBegin() &&
+             Macro.Directive->getLocation() <= Range.getEnd();
+    });
+  });
+}
+
+void MacroToEnumCallbacks::issueDiagnostics() {
   for (const MacroList &MacroList : Enums) {
     if (MacroList.empty())
       continue;
@@ -530,13 +547,43 @@ void MacroToEnumCallbacks::fixEnumMacro(const MacroList &MacroList) const {
   Diagnostic << FixItHint::CreateInsertion(End, "};\n");
 }
 
-} // namespace
-
 void MacroToEnumCheck::registerPPCallbacks(const SourceManager &SM,
                                            Preprocessor *PP,
                                            Preprocessor *ModuleExpanderPP) {
-  PP->addPPCallbacks(
-      std::make_unique<MacroToEnumCallbacks>(this, getLangOpts(), SM));
+  auto Callback = std::make_unique<MacroToEnumCallbacks>(this, getLangOpts(), SM);
+  PPCallback = Callback.get();
+  PP->addPPCallbacks(std::move(Callback));
+}
+
+void MacroToEnumCheck::registerMatchers(ast_matchers::MatchFinder *Finder) {
+  using namespace ast_matchers;
+  auto TopLevelDecl = hasParent(translationUnitDecl());
+  Finder->addMatcher(decl(TopLevelDecl).bind("top"), this);
+}
+
+static bool isValid(SourceRange Range) {
+  return Range.getBegin().isValid() && Range.getEnd().isValid();
+}
+
+static bool empty(SourceRange Range) {
+  return Range.getBegin() == Range.getEnd();
+}
+
+void MacroToEnumCheck::check(
+    const ast_matchers::MatchFinder::MatchResult &Result) {
+  auto *TLDecl = Result.Nodes.getNodeAs<Decl>("top");
+  if (TLDecl == nullptr)
+      return;
+
+  SourceRange Range = TLDecl->getSourceRange();
+  if (auto *TemplateFn = Result.Nodes.getNodeAs<FunctionTemplateDecl>("top")) {
+    if (TemplateFn->isThisDeclarationADefinition() && TemplateFn->hasBody())
+      Range = SourceRange{TemplateFn->getBeginLoc(),
+                          TemplateFn->getUnderlyingDecl()->getBodyRBrace()};
+  }
+
+  if (isValid(Range) && !empty(Range))
+    PPCallback->invalidateRange(Range);
 }
 
 } // namespace modernize

diff --git a/clang-tools-extra/clang-tidy/modernize/MacroToEnumCheck.h b/clang-tools-extra/clang-tidy/modernize/MacroToEnumCheck.h
index 667ff49..09fdd54 100644 (file)
--- a/clang-tools-extra/clang-tidy/modernize/MacroToEnumCheck.h
+++ b/clang-tools-extra/clang-tidy/modernize/MacroToEnumCheck.h
@@ -15,6 +15,8 @@ namespace clang {
 namespace tidy {
 namespace modernize {
 
+class MacroToEnumCallbacks;
+
 /// Replaces groups of related macros with an unscoped anonymous enum.
 ///
 /// For the user-facing documentation see:
@@ -25,6 +27,11 @@ public:
       : ClangTidyCheck(Name, Context) {}
   void registerPPCallbacks(const SourceManager &SM, Preprocessor *PP,
                            Preprocessor *ModuleExpanderPP) override;
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+
+private:
+  MacroToEnumCallbacks *PPCallback{};
 };
 
 } // namespace modernize

diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize-macro-to-enum.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize-macro-to-enum.cpp
index e065e83..6c2be4f 100644 (file)
--- a/clang-tools-extra/test/clang-tidy/checkers/modernize-macro-to-enum.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/modernize-macro-to-enum.cpp
@@ -1,4 +1,4 @@
-// RUN: %check_clang_tidy -std=c++14-or-later %s modernize-macro-to-enum %t -- -- -I%S/Inputs/modernize-macro-to-enum
+// RUN: %check_clang_tidy -std=c++14-or-later %s modernize-macro-to-enum %t -- -- -I%S/Inputs/modernize-macro-to-enum -fno-delayed-template-parsing
 // C++14 or later required for binary literals.
 
 #if 1
@@ -303,3 +303,89 @@ void f()
   FN_GREEN(0);
   FN_BLUE(0);
 }
+
+// Ignore macros defined inside a top-level function definition.
+void g(int x)
+{
+  if (x != 0) {
+#define INSIDE1 1
+#define INSIDE2 2
+    if (INSIDE1 > 1) {
+      f();
+    }
+  } else {
+    if (INSIDE2 == 1) {
+      f();
+    }
+  }
+}
+
+// Ignore macros defined inside a top-level function declaration.
+extern void g2(
+#define INSIDE3 3
+#define INSIDE4 4
+);
+
+// Ignore macros defined inside a record (structure) declaration.
+struct S {
+#define INSIDE5 5
+#define INSIDE6 6
+  char storage[INSIDE5];
+};
+class C {
+#define INSIDE7 7
+#define INSIDE8 8
+};
+
+// Ignore macros defined inside a template function definition.
+template <int N>
+#define INSIDE9 9
+bool fn()
+{
+  #define INSIDE10 10
+  return INSIDE9 > 1 || INSIDE10 < N;
+}
+
+// Ignore macros defined inside a variable declaration.
+extern int
+#define INSIDE11 11
+v;
+
+// Ignore macros defined inside a template class definition.
+template <int N>
+class C2 {
+public:
+#define INSIDE12 12
+    char storage[N];
+  bool f() {
+    return N > INSIDE12;
+  }
+  bool g();
+};
+
+// Ignore macros defined inside a template member function definition.
+template <int N>
+#define INSIDE13 13
+bool C2<N>::g() {
+#define INSIDE14 14
+  return N < INSIDE12 || N > INSIDE13 || INSIDE14 > N;
+};
+
+// Ignore macros defined inside a template type alias.
+template <typename T>
+class C3 {
+  T data;
+};
+template <typename T>
+#define INSIDE15 15
+using Data = C3<T[INSIDE15]>;
+
+// Ignore macros defined inside a type alias.
+using Data2 =
+#define INSIDE16 16
+    char[INSIDE16];
+
+// Ignore macros defined inside a (constexpr) variable definition.
+constexpr int
+#define INSIDE17 17
+value = INSIDE17;