Skip to content

Commit b985b6e

Browse files
[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 llvm#54883
1 parent 1af25a9 commit b985b6e

File tree

3 files changed

+146
-6
lines changed

3 files changed

+146
-6
lines changed

clang-tools-extra/clang-tidy/modernize/MacroToEnumCheck.cpp

Lines changed: 52 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,8 @@ struct FileState {
121121
SourceLocation LastMacroLocation;
122122
};
123123

124+
} // namespace
125+
124126
class MacroToEnumCallbacks : public PPCallbacks {
125127
public:
126128
MacroToEnumCallbacks(MacroToEnumCheck *Check, const LangOptions &LangOptions,
@@ -197,6 +199,8 @@ class MacroToEnumCallbacks : public PPCallbacks {
197199
// After we've seen everything, issue warnings and fix-its.
198200
void EndOfMainFile() override;
199201

202+
void invalidateRange(SourceRange Range);
203+
200204
private:
201205
void newEnum() {
202206
if (Enums.empty() || !Enums.back().empty())
@@ -224,6 +228,7 @@ class MacroToEnumCallbacks : public PPCallbacks {
224228
void checkName(const Token &MacroNameTok);
225229
void rememberExpressionName(const Token &MacroNameTok);
226230
void invalidateExpressionNames();
231+
void issueDiagnostics();
227232
void warnMacroEnum(const EnumMacro &Macro) const;
228233
void fixEnumMacro(const MacroList &MacroList) const;
229234

@@ -472,8 +477,20 @@ void MacroToEnumCallbacks::invalidateExpressionNames() {
472477
}
473478

474479
void MacroToEnumCallbacks::EndOfMainFile() {
475-
invalidateExpressionNames();
480+
invalidateExpressionNames();
481+
issueDiagnostics();
482+
}
476483

484+
void MacroToEnumCallbacks::invalidateRange(SourceRange Range) {
485+
llvm::erase_if(Enums, [Range](const MacroList &MacroList) {
486+
return llvm::any_of(MacroList, [Range](const EnumMacro &Macro) {
487+
return Macro.Directive->getLocation() >= Range.getBegin() &&
488+
Macro.Directive->getLocation() <= Range.getEnd();
489+
});
490+
});
491+
}
492+
493+
void MacroToEnumCallbacks::issueDiagnostics() {
477494
for (const MacroList &MacroList : Enums) {
478495
if (MacroList.empty())
479496
continue;
@@ -530,13 +547,43 @@ void MacroToEnumCallbacks::fixEnumMacro(const MacroList &MacroList) const {
530547
Diagnostic << FixItHint::CreateInsertion(End, "};\n");
531548
}
532549

533-
} // namespace
534-
535550
void MacroToEnumCheck::registerPPCallbacks(const SourceManager &SM,
536551
Preprocessor *PP,
537552
Preprocessor *ModuleExpanderPP) {
538-
PP->addPPCallbacks(
539-
std::make_unique<MacroToEnumCallbacks>(this, getLangOpts(), SM));
553+
auto Callback = std::make_unique<MacroToEnumCallbacks>(this, getLangOpts(), SM);
554+
PPCallback = Callback.get();
555+
PP->addPPCallbacks(std::move(Callback));
556+
}
557+
558+
void MacroToEnumCheck::registerMatchers(ast_matchers::MatchFinder *Finder) {
559+
using namespace ast_matchers;
560+
auto TopLevelDecl = hasParent(translationUnitDecl());
561+
Finder->addMatcher(decl(TopLevelDecl).bind("top"), this);
562+
}
563+
564+
static bool isValid(SourceRange Range) {
565+
return Range.getBegin().isValid() && Range.getEnd().isValid();
566+
}
567+
568+
static bool empty(SourceRange Range) {
569+
return Range.getBegin() == Range.getEnd();
570+
}
571+
572+
void MacroToEnumCheck::check(
573+
const ast_matchers::MatchFinder::MatchResult &Result) {
574+
auto *TLDecl = Result.Nodes.getNodeAs<Decl>("top");
575+
if (TLDecl == nullptr)
576+
return;
577+
578+
SourceRange Range = TLDecl->getSourceRange();
579+
if (auto *TemplateFn = Result.Nodes.getNodeAs<FunctionTemplateDecl>("top")) {
580+
if (TemplateFn->isThisDeclarationADefinition() && TemplateFn->hasBody())
581+
Range = SourceRange{TemplateFn->getBeginLoc(),
582+
TemplateFn->getUnderlyingDecl()->getBodyRBrace()};
583+
}
584+
585+
if (isValid(Range) && !empty(Range))
586+
PPCallback->invalidateRange(Range);
540587
}
541588

542589
} // namespace modernize

clang-tools-extra/clang-tidy/modernize/MacroToEnumCheck.h

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@ namespace clang {
1515
namespace tidy {
1616
namespace modernize {
1717

18+
class MacroToEnumCallbacks;
19+
1820
/// Replaces groups of related macros with an unscoped anonymous enum.
1921
///
2022
/// For the user-facing documentation see:
@@ -25,6 +27,11 @@ class MacroToEnumCheck : public ClangTidyCheck {
2527
: ClangTidyCheck(Name, Context) {}
2628
void registerPPCallbacks(const SourceManager &SM, Preprocessor *PP,
2729
Preprocessor *ModuleExpanderPP) override;
30+
void registerMatchers(ast_matchers::MatchFinder *Finder) override;
31+
void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
32+
33+
private:
34+
MacroToEnumCallbacks *PPCallback{};
2835
};
2936

3037
} // namespace modernize

clang-tools-extra/test/clang-tidy/checkers/modernize-macro-to-enum.cpp

Lines changed: 87 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// RUN: %check_clang_tidy -std=c++14-or-later %s modernize-macro-to-enum %t -- -- -I%S/Inputs/modernize-macro-to-enum
1+
// 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
22
// C++14 or later required for binary literals.
33

44
#if 1
@@ -303,3 +303,89 @@ void f()
303303
FN_GREEN(0);
304304
FN_BLUE(0);
305305
}
306+
307+
// Ignore macros defined inside a top-level function definition.
308+
void g(int x)
309+
{
310+
if (x != 0) {
311+
#define INSIDE1 1
312+
#define INSIDE2 2
313+
if (INSIDE1 > 1) {
314+
f();
315+
}
316+
} else {
317+
if (INSIDE2 == 1) {
318+
f();
319+
}
320+
}
321+
}
322+
323+
// Ignore macros defined inside a top-level function declaration.
324+
extern void g2(
325+
#define INSIDE3 3
326+
#define INSIDE4 4
327+
);
328+
329+
// Ignore macros defined inside a record (structure) declaration.
330+
struct S {
331+
#define INSIDE5 5
332+
#define INSIDE6 6
333+
char storage[INSIDE5];
334+
};
335+
class C {
336+
#define INSIDE7 7
337+
#define INSIDE8 8
338+
};
339+
340+
// Ignore macros defined inside a template function definition.
341+
template <int N>
342+
#define INSIDE9 9
343+
bool fn()
344+
{
345+
#define INSIDE10 10
346+
return INSIDE9 > 1 || INSIDE10 < N;
347+
}
348+
349+
// Ignore macros defined inside a variable declaration.
350+
extern int
351+
#define INSIDE11 11
352+
v;
353+
354+
// Ignore macros defined inside a template class definition.
355+
template <int N>
356+
class C2 {
357+
public:
358+
#define INSIDE12 12
359+
char storage[N];
360+
bool f() {
361+
return N > INSIDE12;
362+
}
363+
bool g();
364+
};
365+
366+
// Ignore macros defined inside a template member function definition.
367+
template <int N>
368+
#define INSIDE13 13
369+
bool C2<N>::g() {
370+
#define INSIDE14 14
371+
return N < INSIDE12 || N > INSIDE13 || INSIDE14 > N;
372+
};
373+
374+
// Ignore macros defined inside a template type alias.
375+
template <typename T>
376+
class C3 {
377+
T data;
378+
};
379+
template <typename T>
380+
#define INSIDE15 15
381+
using Data = C3<T[INSIDE15]>;
382+
383+
// Ignore macros defined inside a type alias.
384+
using Data2 =
385+
#define INSIDE16 16
386+
char[INSIDE16];
387+
388+
// Ignore macros defined inside a (constexpr) variable definition.
389+
constexpr int
390+
#define INSIDE17 17
391+
value = INSIDE17;

0 commit comments

Comments
 (0)