Skip to content

Commit d563c2d

Browse files
[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
1 parent cbcb3bc commit d563c2d

File tree

3 files changed

+84
-12
lines changed

3 files changed

+84
-12
lines changed

clang-tools-extra/clang-tidy/modernize/MacroToEnumCheck.cpp

Lines changed: 41 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -324,18 +324,51 @@ void MacroToEnumCallbacks::MacroDefined(const Token &MacroNameTok,
324324
return;
325325

326326
const MacroInfo *Info = MD->getMacroInfo();
327-
if (Info->isFunctionLike() || Info->isBuiltinMacro() ||
328-
Info->tokens().empty() || Info->tokens().size() > 2)
327+
ArrayRef<Token> MacroTokens = Info->tokens();
328+
if (Info->isFunctionLike() || Info->isBuiltinMacro() || MacroTokens.empty())
329329
return;
330330

331-
// It can be +Lit, -Lit or just Lit.
332-
Token Tok = Info->tokens().front();
333-
if (Info->tokens().size() == 2) {
334-
if (!Tok.isOneOf(tok::TokenKind::minus, tok::TokenKind::plus,
335-
tok::TokenKind::tilde))
331+
// Return Lit when +Lit, -Lit or ~Lit; otherwise return Unknown.
332+
Token Unknown;
333+
Unknown.setKind(tok::TokenKind::unknown);
334+
auto GetUnopArg = [Unknown](Token First, Token Second) {
335+
return First.isOneOf(tok::TokenKind::minus, tok::TokenKind::plus,
336+
tok::TokenKind::tilde)
337+
? Second
338+
: Unknown;
339+
};
340+
341+
// It could just be a single token.
342+
Token Tok = MacroTokens.front();
343+
344+
// It can be any arbitrary nesting of matched parentheses around
345+
// +Lit, -Lit, ~Lit or Lit.
346+
if (MacroTokens.size() > 2) {
347+
// Strip off matching '(', ..., ')' token pairs.
348+
size_t Begin = 0;
349+
size_t End = MacroTokens.size() - 1;
350+
assert(End >= 2U);
351+
for (; Begin < MacroTokens.size() / 2; ++Begin, --End) {
352+
if (!MacroTokens[Begin].is(tok::TokenKind::l_paren) ||
353+
!MacroTokens[End].is(tok::TokenKind::r_paren))
354+
break;
355+
}
356+
size_t Size = End >= Begin ? (End - Begin + 1U) : 0U;
357+
358+
// It was a single token inside matching parens.
359+
if (Size == 1)
360+
Tok = MacroTokens[Begin];
361+
else if (Size == 2)
362+
// It can be +Lit, -Lit or ~Lit.
363+
Tok = GetUnopArg(MacroTokens[Begin], MacroTokens[End]);
364+
else
365+
// Zero or too many tokens after we stripped matching parens.
336366
return;
337-
Tok = Info->tokens().back();
367+
} else if (MacroTokens.size() == 2) {
368+
// It can be +Lit, -Lit, or ~Lit.
369+
Tok = GetUnopArg(MacroTokens.front(), MacroTokens.back());
338370
}
371+
339372
if (!Tok.isLiteral() || isStringLiteral(Tok.getKind()) ||
340373
!isIntegralConstant(Tok))
341374
return;

clang-tools-extra/docs/clang-tidy/checks/modernize-macro-to-enum.rst

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,10 +14,12 @@ within the constraints outlined below.
1414

1515
Potential macros for replacement must meet the following constraints:
1616

17-
- Macros must expand only to integral literal tokens. The unary operators
18-
plus, minus and tilde are recognized to allow for positive, negative
19-
and bitwise negated integers. More complicated integral constant
20-
expressions are not currently recognized by this check.
17+
- Macros must expand only to integral literal tokens or simple expressions
18+
of literal tokens. The unary operators plus, minus and tilde are
19+
recognized to allow for positive, negative and bitwise negated integers.
20+
The above expressions may also be surrounded by matching pairs of
21+
parentheses. More complicated integral constant expressions are not
22+
recognized by this check.
2123
- Macros must be defined on sequential source file lines, or with
2224
only comment lines in between macro definitions.
2325
- Macros must all be defined in the same source file.
@@ -43,6 +45,7 @@ Examples:
4345
#define GREEN 0x00FF00
4446
#define BLUE 0x0000FF
4547

48+
#define TM_NONE (-1) // No method selected.
4649
#define TM_ONE 1 // Use tailored method one.
4750
#define TM_TWO 2 // Use tailored method two. Method two
4851
// is preferable to method one.
@@ -59,6 +62,7 @@ becomes
5962
};
6063

6164
enum {
65+
TM_NONE = (-1), // No method selected.
6266
TM_ONE = 1, // Use tailored method one.
6367
TM_TWO = 2, // Use tailored method two. Method two
6468
// is preferable to method one.

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

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -137,6 +137,41 @@
137137
// CHECK-FIXES-NEXT: SUFFIX5 = +1ULL
138138
// CHECK-FIXES-NEXT: };
139139

140+
// A limited form of constant expression is recognized: a parenthesized
141+
// literal or a parenthesized literal with the unary operators +, - or ~.
142+
#define PAREN1 (-1)
143+
#define PAREN2 (1)
144+
#define PAREN3 (+1)
145+
#define PAREN4 (~1)
146+
// CHECK-MESSAGES: :[[@LINE-4]]:1: warning: replace macro with enum
147+
// CHECK-MESSAGES: :[[@LINE-5]]:9: warning: macro 'PAREN1' defines an integral constant; prefer an enum instead
148+
// CHECK-MESSAGES: :[[@LINE-5]]:9: warning: macro 'PAREN2' defines an integral constant; prefer an enum instead
149+
// CHECK-MESSAGES: :[[@LINE-5]]:9: warning: macro 'PAREN3' defines an integral constant; prefer an enum instead
150+
// CHECK-MESSAGES: :[[@LINE-5]]:9: warning: macro 'PAREN4' defines an integral constant; prefer an enum instead
151+
// CHECK-FIXES: enum {
152+
// CHECK-FIXES-NEXT: PAREN1 = (-1),
153+
// CHECK-FIXES-NEXT: PAREN2 = (1),
154+
// CHECK-FIXES-NEXT: PAREN3 = (+1),
155+
// CHECK-FIXES-NEXT: PAREN4 = (~1)
156+
// CHECK-FIXES-NEXT: };
157+
158+
// More complicated parenthesized expressions are excluded.
159+
// Expansions that are not surrounded by parentheses are excluded.
160+
// Nested matching parentheses are stripped.
161+
#define COMPLEX_PAREN1 (x+1)
162+
#define COMPLEX_PAREN2 (x+1
163+
#define COMPLEX_PAREN3 (())
164+
#define COMPLEX_PAREN4 ()
165+
#define COMPLEX_PAREN5 (+1)
166+
#define COMPLEX_PAREN6 ((+1))
167+
// CHECK-MESSAGES: :[[@LINE-2]]:1: warning: replace macro with enum
168+
// CHECK-MESSAGES: :[[@LINE-3]]:9: warning: macro 'COMPLEX_PAREN5' defines an integral constant; prefer an enum instead
169+
// CHECK-MESSAGES: :[[@LINE-3]]:9: warning: macro 'COMPLEX_PAREN6' defines an integral constant; prefer an enum instead
170+
// CHECK-FIXES: enum {
171+
// CHECK-FIXES-NEXT: COMPLEX_PAREN5 = (+1),
172+
// CHECK-FIXES-NEXT: COMPLEX_PAREN6 = ((+1))
173+
// CHECK-FIXES-NEXT: };
174+
140175
// Macros appearing in conditional expressions can't be replaced
141176
// by enums.
142177
#define USE_FOO 1

0 commit comments

Comments
 (0)