[PATCH] D123889: [clang-tidy] Improve macro handling in modernize-macro-to-enum

[Richard via Phabricator via cfe-commits]

LegalizeAdulthood created this revision.
LegalizeAdulthood added reviewers: aaron.ballman, klimek.
LegalizeAdulthood added a project: clang-tools-extra.
Herald added subscribers: carlosgalvezp, xazax.hun.
Herald added a project: All.
LegalizeAdulthood requested review of this revision.

When a macro is undef'ed or used in a preprocessor conditional
expression, we need to remember that macro should it later be
defined in the file to an integral value.  We need to exclude
such macro names from being turned into an enum.

Maintain a blacklist of identifiers that we've seen in an
undef or conditional preprocessor directive.  When the file is
done processing, remove all the blacklisted identifiers from
conversion to an enum.

Fixes #54842


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D123889

Files:
  clang-tools-extra/clang-tidy/modernize/MacroToEnumCheck.cpp
  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
@@ -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,19 @@
 #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
+
+#define DEFINED_LATER1 1
+#define DEFINED_LATER2 2
+#define DEFINED_LATER3 3
+
 // Sometimes an argument to ifdef can be classified as a keyword token.
 #ifdef __restrict
 #endif
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
@@ -193,6 +193,7 @@
   void Endif(SourceLocation Loc, SourceLocation IfLoc) override;
   void PragmaDirective(SourceLocation Loc,
                        PragmaIntroducerKind Introducer) override;
+  void invalidateExpressionNames();
 
   // After we've seen everything, issue warnings and fix-its.
   void EndOfMainFile() override;
@@ -222,6 +223,7 @@
   void conditionStart(const SourceLocation &Loc);
   void checkCondition(SourceRange ConditionRange);
   void checkName(const Token &MacroNameTok);
+  void rememberExpressionToken(const Token &MacroNameTok);
   void warnMacroEnum(const EnumMacro &Macro) const;
   void fixEnumMacro(const MacroList &MacroList) const;
 
@@ -230,6 +232,7 @@
   const SourceManager &SM;
   SmallVector<MacroList> Enums;
   SmallVector<FileState> Files;
+  std::vector<std::string> ExpressionNames;
   FileState *CurrentFile = nullptr;
 };
 
@@ -284,8 +287,9 @@
 }
 
 void MacroToEnumCallbacks::checkName(const Token &MacroNameTok) {
-  StringRef Id = getTokenName(MacroNameTok);
+  rememberExpressionToken(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::rememberExpressionToken(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) {
+  rememberExpressionToken(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;

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits