LegalizeAdulthood created this revision.
LegalizeAdulthood added reviewers: aaron.ballman, alexfh.
LegalizeAdulthood added a project: clang-tools-extra.
Herald added subscribers: carlosgalvezp, xazax.hun.
Herald added a project: All.
LegalizeAdulthood requested review of this revision.
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.
Fixes #54883
Repository:
rG LLVM Github Monorepo
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,75 @@
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
+};
+template <int N>
+#define INSIDE9 9
+bool fn()
+{
+ #define INSIDE10 10
+ return INSIDE9 > 1 || INSIDE10 < N;
+}
+extern int
+#define INSIDE11 11
+v;
+template <int N>
+class C2 {
+public:
+#define INSIDE12 12
+ char storage[N];
+ bool f() {
+ return N > INSIDE12;
+ }
+ bool g();
+};
+template <int N>
+#define INSIDE13 13
+bool C2<N>::g() {
+#define INSIDE14 14
+ return N < INSIDE12 || N > INSIDE13 || INSIDE14 > N;
+};
+template <typename T>
+class C3 {
+ T data;
+};
+template <typename T>
+#define INSIDE15 15
+using Data = C3<T[INSIDE15]>;
+using Data2 =
+#define INSIDE16 16
+ char[INSIDE16];
+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
