diff --git a/clang-tools-extra/clang-tidy/modernize/MacroToEnumCheck.cpp b/clang-tools-extra/clang-tidy/modernize/MacroToEnumCheck.cpp --- a/clang-tools-extra/clang-tidy/modernize/MacroToEnumCheck.cpp +++ b/clang-tools-extra/clang-tidy/modernize/MacroToEnumCheck.cpp @@ -222,6 +222,8 @@ void conditionStart(const SourceLocation &Loc); void checkCondition(SourceRange ConditionRange); void checkName(const Token &MacroNameTok); + void rememberExpressionName(const Token &MacroNameTok); + void invalidateExpressionNames(); void warnMacroEnum(const EnumMacro &Macro) const; void fixEnumMacro(const MacroList &MacroList) const; @@ -230,6 +232,7 @@ const SourceManager &SM; SmallVector Enums; SmallVector Files; + std::vector ExpressionNames; FileState *CurrentFile = nullptr; }; @@ -284,8 +287,9 @@ } void MacroToEnumCallbacks::checkName(const Token &MacroNameTok) { - StringRef Id = getTokenName(MacroNameTok); + rememberExpressionName(MacroNameTok); + StringRef Id = getTokenName(MacroNameTok); llvm::erase_if(Enums, [&Id](const MacroList &MacroList) { return llvm::any_of(MacroList, [&Id](const EnumMacro &Macro) { return getTokenName(Macro.Name) == Id; @@ -293,6 +297,14 @@ }); } +void MacroToEnumCallbacks::rememberExpressionName(const Token &MacroNameTok) { + std::string Id = getTokenName(MacroNameTok).str(); + auto Pos = llvm::lower_bound(ExpressionNames, Id); + if (Pos == ExpressionNames.end() || *Pos != Id) { + ExpressionNames.insert(Pos, Id); + } +} + void MacroToEnumCallbacks::FileChanged(SourceLocation Loc, FileChangeReason Reason, SrcMgr::CharacteristicKind FileType, @@ -384,6 +396,8 @@ void MacroToEnumCallbacks::MacroUndefined(const Token &MacroNameTok, const MacroDefinition &MD, const MacroDirective *Undef) { + rememberExpressionName(MacroNameTok); + auto MatchesToken = [&MacroNameTok](const EnumMacro &Macro) { return getTokenName(Macro.Name) == getTokenName(MacroNameTok); }; @@ -447,7 +461,19 @@ CurrentFile->GuardScanner = IncludeGuard::IfGuard; } +void MacroToEnumCallbacks::invalidateExpressionNames() { + for (const std::string &Id : ExpressionNames) { + llvm::erase_if(Enums, [Id](const MacroList &MacroList) { + return llvm::any_of(MacroList, [&Id](const EnumMacro &Macro) { + return getTokenName(Macro.Name) == Id; + }); + }); + } +} + void MacroToEnumCallbacks::EndOfMainFile() { + invalidateExpressionNames(); + for (const MacroList &MacroList : Enums) { if (MacroList.empty()) continue; 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 --- 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 @@ -181,6 +181,12 @@ #define USE_IFDEF 1 #define USE_IFNDEF 1 +// Undef'ing first and then defining later should still exclude this macro +#undef USE_UINT64 +#define USE_UINT64 0 +#undef USE_INT64 +#define USE_INT64 0 + #if defined(USE_FOO) && USE_FOO extern void foo(); #else @@ -243,6 +249,27 @@ #define IFNDEF3 3 #endif +// Macros used in conditions are invalidated, even if they look +// like enums after they are used in conditions. +#if DEFINED_LATER1 +#endif +#ifdef DEFINED_LATER2 +#endif +#ifndef DEFINED_LATER3 +#endif +#undef DEFINED_LATER4 +#if ((defined(DEFINED_LATER5) || DEFINED_LATER6) && DEFINED_LATER7) || (DEFINED_LATER8 > 10) +#endif + +#define DEFINED_LATER1 1 +#define DEFINED_LATER2 2 +#define DEFINED_LATER3 3 +#define DEFINED_LATER4 4 +#define DEFINED_LATER5 5 +#define DEFINED_LATER6 6 +#define DEFINED_LATER7 7 +#define DEFINED_LATER8 8 + // Sometimes an argument to ifdef can be classified as a keyword token. #ifdef __restrict #endif