[clang-tools-extra] b985b6e - [clang-tidy] Ignore macros defined within declarations

via cfe-commits cfe-commits at lists.llvm.org
Fri Apr 22 16:47:04 PDT 2022


Author: Richard
Date: 2022-04-22T17:46:54-06:00
New Revision: b985b6e3c15a863112e1676d6211c80c7683f3eb

URL: https://github.com/llvm/llvm-project/commit/b985b6e3c15a863112e1676d6211c80c7683f3eb
DIFF: https://github.com/llvm/llvm-project/commit/b985b6e3c15a863112e1676d6211c80c7683f3eb.diff

LOG: [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

Added: 
    

Modified: 
    clang-tools-extra/clang-tidy/modernize/MacroToEnumCheck.cpp
    clang-tools-extra/clang-tidy/modernize/MacroToEnumCheck.h
    clang-tools-extra/test/clang-tidy/checkers/modernize-macro-to-enum.cpp

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/clang-tidy/modernize/MacroToEnumCheck.cpp b/clang-tools-extra/clang-tidy/modernize/MacroToEnumCheck.cpp
index 90d3fe0b99822..e916668f7cbb9 100644
--- 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 @@ class MacroToEnumCallbacks : public PPCallbacks {
   // 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 @@ class MacroToEnumCallbacks : public PPCallbacks {
   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 667ff49d671a1..09fdd5448ad4b 100644
--- 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 @@ class MacroToEnumCheck : public ClangTidyCheck {
       : 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 e065e83a0e1dc..6c2be4f1eac16 100644
--- 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;


        


More information about the cfe-commits mailing list