[PATCH] D124316: [clang-tidy] Modernize-macro-to-enum should skip macros used in other macros

[Richard via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG693246e03f28: [clang-tidy] Modernize-macro-to-enum should 
skip macros used in other macros (authored by LegalizeAdulthood).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D124316/new/

https://reviews.llvm.org/D124316

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
@@ -281,27 +281,14 @@
 #define EPS2 1e5
 #define EPS3 1.
 
-#define DO_RED draw(RED)
-#define DO_GREEN draw(GREEN)
-#define DO_BLUE draw(BLUE)
-
-#define FN_RED(x) draw(RED | x)
-#define FN_GREEN(x) draw(GREEN | x)
-#define FN_BLUE(x) draw(BLUE | x)
-
 extern void draw(unsigned int Color);
 
 void f()
 {
+  // Usage of macros converted to enums should still compile.
   draw(RED);
-  draw(GREEN);
-  draw(BLUE);
-  DO_RED;
-  DO_GREEN;
-  DO_BLUE;
-  FN_RED(0);
-  FN_GREEN(0);
-  FN_BLUE(0);
+  draw(GREEN | RED);
+  draw(BLUE + RED);
 }
 
 // Ignore macros defined inside a top-level function definition.
@@ -389,3 +376,17 @@
 constexpr int
 #define INSIDE17 17
 value = INSIDE17;
+
+// Ignore macros used in the expansion of other macros
+#define INSIDE18 18
+#define INSIDE19 19
+
+#define CONCAT(n_, s_) n_##s_
+#define FN_NAME(n_, s_) CONCAT(n_, s_)
+
+extern void FN_NAME(g, INSIDE18)();
+
+void gg()
+{
+    g18();
+}
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
@@ -226,7 +226,8 @@
   void conditionStart(const SourceLocation &Loc);
   void checkCondition(SourceRange ConditionRange);
   void checkName(const Token &MacroNameTok);
-  void rememberExpressionName(const Token &MacroNameTok);
+  void rememberExpressionName(const Token &Tok);
+  void rememberExpressionTokens(ArrayRef<Token> MacroTokens);
   void invalidateExpressionNames();
   void issueDiagnostics();
   void warnMacroEnum(const EnumMacro &Macro) const;
@@ -302,14 +303,22 @@
   });
 }
 
-void MacroToEnumCallbacks::rememberExpressionName(const Token &MacroNameTok) {
-  std::string Id = getTokenName(MacroNameTok).str();
+void MacroToEnumCallbacks::rememberExpressionName(const Token &Tok) {
+  std::string Id = getTokenName(Tok).str();
   auto Pos = llvm::lower_bound(ExpressionNames, Id);
   if (Pos == ExpressionNames.end() || *Pos != Id) {
     ExpressionNames.insert(Pos, Id);
   }
 }
 
+void MacroToEnumCallbacks::rememberExpressionTokens(
+    ArrayRef<Token> MacroTokens) {
+  for (Token Tok : MacroTokens) {
+    if (Tok.isAnyIdentifier())
+      rememberExpressionName(Tok);
+  }
+}
+
 void MacroToEnumCallbacks::FileChanged(SourceLocation Loc,
                                        FileChangeReason Reason,
                                        SrcMgr::CharacteristicKind FileType,
@@ -326,6 +335,8 @@
   CurrentFile = &Files.back();
 }
 
+// Any defined but rejected macro is scanned for identifiers that
+// are to be excluded as enums.
 void MacroToEnumCallbacks::MacroDefined(const Token &MacroNameTok,
                                         const MacroDirective *MD) {
   // Include guards are never candidates for becoming an enum.
@@ -342,8 +353,12 @@
 
   const MacroInfo *Info = MD->getMacroInfo();
   ArrayRef<Token> MacroTokens = Info->tokens();
-  if (Info->isFunctionLike() || Info->isBuiltinMacro() || MacroTokens.empty())
+  if (Info->isBuiltinMacro() || MacroTokens.empty())
+    return;
+  if (Info->isFunctionLike()) {
+    rememberExpressionTokens(MacroTokens);
     return;
+  }
 
   // Return Lit when +Lit, -Lit or ~Lit; otherwise return Unknown.
   Token Unknown;
@@ -378,17 +393,22 @@
     else if (Size == 2)
       // It can be +Lit, -Lit or ~Lit.
       Tok = GetUnopArg(MacroTokens[Begin], MacroTokens[End]);
-    else
+    else {
       // Zero or too many tokens after we stripped matching parens.
+      rememberExpressionTokens(MacroTokens);
       return;
+    }
   } else if (MacroTokens.size() == 2) {
     // It can be +Lit, -Lit, or ~Lit.
     Tok = GetUnopArg(MacroTokens.front(), MacroTokens.back());
   }
 
   if (!Tok.isLiteral() || isStringLiteral(Tok.getKind()) ||
-      !isIntegralConstant(Tok))
+      !isIntegralConstant(Tok)) {
+    if (Tok.isAnyIdentifier())
+      rememberExpressionName(Tok);
     return;
+  }
 
   if (!isConsecutiveMacro(MD))
     newEnum();

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