From b985b6e3c15a863112e1676d6211c80c7683f3eb Mon Sep 17 00:00:00 2001 From: Richard Date: Sat, 16 Apr 2022 18:26:01 -0600 Subject: [PATCH] [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 --- .../clang-tidy/modernize/MacroToEnumCheck.cpp | 57 ++++++++++++-- .../clang-tidy/modernize/MacroToEnumCheck.h | 7 ++ .../checkers/modernize-macro-to-enum.cpp | 88 +++++++++++++++++++++- 3 files changed, 146 insertions(+), 6 deletions(-) diff --git a/clang-tools-extra/clang-tidy/modernize/MacroToEnumCheck.cpp b/clang-tools-extra/clang-tidy/modernize/MacroToEnumCheck.cpp index 90d3fe0..e916668 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 @@ 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(this, getLangOpts(), SM)); + auto Callback = std::make_unique(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("top"); + if (TLDecl == nullptr) + return; + + SourceRange Range = TLDecl->getSourceRange(); + if (auto *TemplateFn = Result.Nodes.getNodeAs("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 --- 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..6c2be4f1 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 +#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 +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 +#define INSIDE13 13 +bool C2::g() { +#define INSIDE14 14 + return N < INSIDE12 || N > INSIDE13 || INSIDE14 > N; +}; + +// Ignore macros defined inside a template type alias. +template +class C3 { + T data; +}; +template +#define INSIDE15 15 +using Data = C3; + +// 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; -- 2.7.4