[clang-tools-extra] 693246e - [clang-tidy] Modernize-macro-to-enum should skip macros used in other macros

[via cfe-commits]

Author: Richard
Date: 2022-04-26T21:09:13-06:00
New Revision: 693246e03f28eaa72e0959fe6a969cea655b1fdc

URL: https://github.com/llvm/llvm-project/commit/693246e03f28eaa72e0959fe6a969cea655b1fdc
DIFF: https://github.com/llvm/llvm-project/commit/693246e03f28eaa72e0959fe6a969cea655b1fdc.diff

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

If a macro is used in the expansion of another macro, that can cause
a compile error if the macro is replaced with an enum.  Token-pasting is
an example where converting a macro defined as an integral constant can
cause code to no longer compile.

This change causes such macros to be skipped from the conversion
process in order to prevent fixits from creating code that no longer
compiles.

A subsequent enhancement will examine macro usage in more detail to
allow more cases to be handled without breaking code.

Differential Revision: https://reviews.llvm.org/D124316

Fixes #54948

Added: 
    

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

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/clang-tidy/modernize/MacroToEnumCheck.cpp b/clang-tools-extra/clang-tidy/modernize/MacroToEnumCheck.cpp
index e916668f7cbb9..0d813a14233ca 100644
--- a/clang-tools-extra/clang-tidy/modernize/MacroToEnumCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/MacroToEnumCheck.cpp
@@ -226,7 +226,8 @@ class MacroToEnumCallbacks : public PPCallbacks {
   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::checkName(const Token &MacroNameTok) {
   });
 }
 
-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 @@ void MacroToEnumCallbacks::FileChanged(SourceLocation Loc,
   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 @@ void MacroToEnumCallbacks::MacroDefined(const Token &MacroNameTok,
 
   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 @@ void MacroToEnumCallbacks::MacroDefined(const Token &MacroNameTok,
     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();

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
index 6c2be4f1eac16..616e49012c612 100644
--- 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
@@ -281,27 +281,14 @@ inline void used_ifndef() {}
 #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 @@ using Data2 =
 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();
+}