From d563c2d0e52a738ab2038db02a76dc4c27ec7124 Mon Sep 17 00:00:00 2001 From: Richard Date: Sun, 10 Apr 2022 19:15:22 -0600 Subject: [PATCH] [clang-tidy] Support parenthesized literals in modernize-macro-to-enum When scanning a macro expansion to examine it as a candidate enum, first strip off arbitrary matching parentheses from the outside in, then examine what remains to see if it is Lit, +Lit, -Lit or ~Lit. If not, reject it as a possible enum candidate. Differential Revision: https://reviews.llvm.org/D123479 Fixes #54843 --- .../clang-tidy/modernize/MacroToEnumCheck.cpp | 49 ++++++++++++++++--- .../checks/modernize-macro-to-enum.rst | 12 +++-- .../checkers/modernize-macro-to-enum.cpp | 35 +++++++++++++ 3 files changed, 84 insertions(+), 12 deletions(-) diff --git a/clang-tools-extra/clang-tidy/modernize/MacroToEnumCheck.cpp b/clang-tools-extra/clang-tidy/modernize/MacroToEnumCheck.cpp index b2d59adc9a55..318b86be982b 100644 --- a/clang-tools-extra/clang-tidy/modernize/MacroToEnumCheck.cpp +++ b/clang-tools-extra/clang-tidy/modernize/MacroToEnumCheck.cpp @@ -324,18 +324,51 @@ void MacroToEnumCallbacks::MacroDefined(const Token &MacroNameTok, return; const MacroInfo *Info = MD->getMacroInfo(); - if (Info->isFunctionLike() || Info->isBuiltinMacro() || - Info->tokens().empty() || Info->tokens().size() > 2) + ArrayRef MacroTokens = Info->tokens(); + if (Info->isFunctionLike() || Info->isBuiltinMacro() || MacroTokens.empty()) return; - // It can be +Lit, -Lit or just Lit. - Token Tok = Info->tokens().front(); - if (Info->tokens().size() == 2) { - if (!Tok.isOneOf(tok::TokenKind::minus, tok::TokenKind::plus, - tok::TokenKind::tilde)) + // Return Lit when +Lit, -Lit or ~Lit; otherwise return Unknown. + Token Unknown; + Unknown.setKind(tok::TokenKind::unknown); + auto GetUnopArg = [Unknown](Token First, Token Second) { + return First.isOneOf(tok::TokenKind::minus, tok::TokenKind::plus, + tok::TokenKind::tilde) + ? Second + : Unknown; + }; + + // It could just be a single token. + Token Tok = MacroTokens.front(); + + // It can be any arbitrary nesting of matched parentheses around + // +Lit, -Lit, ~Lit or Lit. + if (MacroTokens.size() > 2) { + // Strip off matching '(', ..., ')' token pairs. + size_t Begin = 0; + size_t End = MacroTokens.size() - 1; + assert(End >= 2U); + for (; Begin < MacroTokens.size() / 2; ++Begin, --End) { + if (!MacroTokens[Begin].is(tok::TokenKind::l_paren) || + !MacroTokens[End].is(tok::TokenKind::r_paren)) + break; + } + size_t Size = End >= Begin ? (End - Begin + 1U) : 0U; + + // It was a single token inside matching parens. + if (Size == 1) + Tok = MacroTokens[Begin]; + else if (Size == 2) + // It can be +Lit, -Lit or ~Lit. + Tok = GetUnopArg(MacroTokens[Begin], MacroTokens[End]); + else + // Zero or too many tokens after we stripped matching parens. return; - Tok = Info->tokens().back(); + } 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)) return; diff --git a/clang-tools-extra/docs/clang-tidy/checks/modernize-macro-to-enum.rst b/clang-tools-extra/docs/clang-tidy/checks/modernize-macro-to-enum.rst index 6be406544ba8..d7784419faa1 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/modernize-macro-to-enum.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/modernize-macro-to-enum.rst @@ -14,10 +14,12 @@ within the constraints outlined below. Potential macros for replacement must meet the following constraints: -- Macros must expand only to integral literal tokens. The unary operators - plus, minus and tilde are recognized to allow for positive, negative - and bitwise negated integers. More complicated integral constant - expressions are not currently recognized by this check. +- Macros must expand only to integral literal tokens or simple expressions + of literal tokens. The unary operators plus, minus and tilde are + recognized to allow for positive, negative and bitwise negated integers. + The above expressions may also be surrounded by matching pairs of + parentheses. More complicated integral constant expressions are not + recognized by this check. - Macros must be defined on sequential source file lines, or with only comment lines in between macro definitions. - Macros must all be defined in the same source file. @@ -43,6 +45,7 @@ Examples: #define GREEN 0x00FF00 #define BLUE 0x0000FF + #define TM_NONE (-1) // No method selected. #define TM_ONE 1 // Use tailored method one. #define TM_TWO 2 // Use tailored method two. Method two // is preferable to method one. @@ -59,6 +62,7 @@ becomes }; enum { + TM_NONE = (-1), // No method selected. TM_ONE = 1, // Use tailored method one. TM_TWO = 2, // Use tailored method two. Method two // is preferable to method one. 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 37122cfc61b9..7727ce0e3a60 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 @@ -137,6 +137,41 @@ // CHECK-FIXES-NEXT: SUFFIX5 = +1ULL // CHECK-FIXES-NEXT: }; +// A limited form of constant expression is recognized: a parenthesized +// literal or a parenthesized literal with the unary operators +, - or ~. +#define PAREN1 (-1) +#define PAREN2 (1) +#define PAREN3 (+1) +#define PAREN4 (~1) +// CHECK-MESSAGES: :[[@LINE-4]]:1: warning: replace macro with enum +// CHECK-MESSAGES: :[[@LINE-5]]:9: warning: macro 'PAREN1' defines an integral constant; prefer an enum instead +// CHECK-MESSAGES: :[[@LINE-5]]:9: warning: macro 'PAREN2' defines an integral constant; prefer an enum instead +// CHECK-MESSAGES: :[[@LINE-5]]:9: warning: macro 'PAREN3' defines an integral constant; prefer an enum instead +// CHECK-MESSAGES: :[[@LINE-5]]:9: warning: macro 'PAREN4' defines an integral constant; prefer an enum instead +// CHECK-FIXES: enum { +// CHECK-FIXES-NEXT: PAREN1 = (-1), +// CHECK-FIXES-NEXT: PAREN2 = (1), +// CHECK-FIXES-NEXT: PAREN3 = (+1), +// CHECK-FIXES-NEXT: PAREN4 = (~1) +// CHECK-FIXES-NEXT: }; + +// More complicated parenthesized expressions are excluded. +// Expansions that are not surrounded by parentheses are excluded. +// Nested matching parentheses are stripped. +#define COMPLEX_PAREN1 (x+1) +#define COMPLEX_PAREN2 (x+1 +#define COMPLEX_PAREN3 (()) +#define COMPLEX_PAREN4 () +#define COMPLEX_PAREN5 (+1) +#define COMPLEX_PAREN6 ((+1)) +// CHECK-MESSAGES: :[[@LINE-2]]:1: warning: replace macro with enum +// CHECK-MESSAGES: :[[@LINE-3]]:9: warning: macro 'COMPLEX_PAREN5' defines an integral constant; prefer an enum instead +// CHECK-MESSAGES: :[[@LINE-3]]:9: warning: macro 'COMPLEX_PAREN6' defines an integral constant; prefer an enum instead +// CHECK-FIXES: enum { +// CHECK-FIXES-NEXT: COMPLEX_PAREN5 = (+1), +// CHECK-FIXES-NEXT: COMPLEX_PAREN6 = ((+1)) +// CHECK-FIXES-NEXT: }; + // Macros appearing in conditional expressions can't be replaced // by enums. #define USE_FOO 1