LegalizeAdulthood updated this revision to Diff 423815. LegalizeAdulthood added a comment.
Add comments describing test cases CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124066/new/ https://reviews.llvm.org/D124066 Files: 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
Index: clang-tools-extra/test/clang-tidy/checkers/modernize-macro-to-enum.cpp =================================================================== --- clang-tools-extra/test/clang-tidy/checkers/modernize-macro-to-enum.cpp +++ 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 @@ 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; Index: clang-tools-extra/clang-tidy/modernize/MacroToEnumCheck.h =================================================================== --- clang-tools-extra/clang-tidy/modernize/MacroToEnumCheck.h +++ clang-tools-extra/clang-tidy/modernize/MacroToEnumCheck.h @@ -15,6 +15,8 @@ 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 @@ : 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 Index: clang-tools-extra/clang-tidy/modernize/MacroToEnumCheck.cpp =================================================================== --- clang-tools-extra/clang-tidy/modernize/MacroToEnumCheck.cpp +++ clang-tools-extra/clang-tidy/modernize/MacroToEnumCheck.cpp @@ -121,6 +121,8 @@ SourceLocation LastMacroLocation; }; +} // namespace + class MacroToEnumCallbacks : public PPCallbacks { public: MacroToEnumCallbacks(MacroToEnumCheck *Check, const LangOptions &LangOptions, @@ -197,6 +199,8 @@ // 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 @@ 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::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,57 @@ 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(varDecl(TopLevelDecl).bind("top"), this); + Finder->addMatcher(functionDecl(TopLevelDecl).bind("top"), this); + Finder->addMatcher(recordDecl(TopLevelDecl).bind("top"), this); + Finder->addMatcher(typeAliasDecl(TopLevelDecl).bind("top"), this); + Finder->addMatcher(functionTemplateDecl(TopLevelDecl).bind("top"), this); + Finder->addMatcher(classTemplateDecl(TopLevelDecl).bind("top"), this); + Finder->addMatcher(typeAliasTemplateDecl(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)) { + CharSourceRange CharRange = + Lexer::makeFileCharRange(CharSourceRange::getTokenRange(Range), + *Result.SourceManager, getLangOpts()); + std::string Text = + Lexer::getSourceText(CharRange, *Result.SourceManager, getLangOpts()) + .str(); + + PPCallback->invalidateRange(Range); + } } } // namespace modernize
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits