Skip to content

Commit e596422

Browse files
committed
[clang-format] Fix template arguments in macros
Fixes llvm#57738 old ``` #define FOO(typeName, realClass) \ { \ #typeName, foo < FooType>(new foo <realClass>(#typeName)) \ } ``` new ``` #define FOO(typeName, realClass) \ { #typeName, foo<FooType>(new foo<realClass>(#typeName)) } ``` Previously, when an UnwrappedLine began with a hash in a macro definition, the program incorrectly assumed the line was a preprocessor directive. It should be stringification. The rule in spaceRequiredBefore was added in 8b52971. Its purpose is to add a space in an include directive. It also added a space to a template opener when the line began with a stringification hash. So we changed it. Reviewed By: HazardyKnusperkeks, owenpan Differential Revision: https://reviews.llvm.org/D133954
1 parent 2183fe2 commit e596422

File tree

6 files changed

+27
-5
lines changed

6 files changed

+27
-5
lines changed

clang/lib/Format/TokenAnnotator.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1427,7 +1427,7 @@ class AnnotatingParser {
14271427
if (!CurrentToken)
14281428
return LT_Invalid;
14291429
NonTemplateLess.clear();
1430-
if (CurrentToken->is(tok::hash)) {
1430+
if (!Line.InMacroBody && CurrentToken->is(tok::hash)) {
14311431
// We were not yet allowed to use C++17 optional when this was being
14321432
// written. So we used LT_Invalid to mark that the line is not a
14331433
// preprocessor directive.
@@ -4241,7 +4241,7 @@ bool TokenAnnotator::spaceRequiredBefore(const AnnotatedLine &Line,
42414241
return false;
42424242
}
42434243
if (Right.is(tok::less) && Left.isNot(tok::l_paren) &&
4244-
Line.startsWith(tok::hash)) {
4244+
Line.Type == LT_ImportStatement) {
42454245
return true;
42464246
}
42474247
if (Right.is(TT_TrailingUnaryOperator))

clang/lib/Format/TokenAnnotator.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ class AnnotatedLine {
4040
: First(Line.Tokens.front().Tok), Level(Line.Level),
4141
MatchingOpeningBlockLineIndex(Line.MatchingOpeningBlockLineIndex),
4242
MatchingClosingBlockLineIndex(Line.MatchingClosingBlockLineIndex),
43-
InPPDirective(Line.InPPDirective),
43+
InPPDirective(Line.InPPDirective), InMacroBody(Line.InMacroBody),
4444
MustBeDeclaration(Line.MustBeDeclaration), MightBeFunctionDecl(false),
4545
IsMultiVariableDeclStmt(false), Affected(false),
4646
LeadingEmptyLinesAffected(false), ChildrenAffected(false),
@@ -130,6 +130,7 @@ class AnnotatedLine {
130130
size_t MatchingOpeningBlockLineIndex;
131131
size_t MatchingClosingBlockLineIndex;
132132
bool InPPDirective;
133+
bool InMacroBody;
133134
bool MustBeDeclaration;
134135
bool MightBeFunctionDecl;
135136
bool IsMultiVariableDeclStmt;

clang/lib/Format/UnwrappedLineParser.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,12 +116,14 @@ class ScopedMacroState : public FormatTokenSource {
116116
TokenSource = this;
117117
Line.Level = 0;
118118
Line.InPPDirective = true;
119+
// InMacroBody gets set after the `#define x` part.
119120
}
120121

121122
~ScopedMacroState() override {
122123
TokenSource = PreviousTokenSource;
123124
ResetToken = Token;
124125
Line.InPPDirective = false;
126+
Line.InMacroBody = false;
125127
Line.Level = PreviousLineLevel;
126128
}
127129

@@ -196,6 +198,7 @@ class ScopedLineState {
196198
Parser.Line = std::make_unique<UnwrappedLine>();
197199
Parser.Line->Level = PreBlockLine->Level;
198200
Parser.Line->InPPDirective = PreBlockLine->InPPDirective;
201+
Parser.Line->InMacroBody = PreBlockLine->InMacroBody;
199202
}
200203

201204
~ScopedLineState() {
@@ -1253,6 +1256,7 @@ void UnwrappedLineParser::parsePPDefine() {
12531256
Line->Level += PPBranchLevel + 1;
12541257
addUnwrappedLine();
12551258
++Line->Level;
1259+
Line->InMacroBody = true;
12561260

12571261
// Errors during a preprocessor directive can only affect the layout of the
12581262
// preprocessor directive, and thus we ignore them. An alternative approach

clang/lib/Format/UnwrappedLineParser.h

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,8 @@ struct UnwrappedLine {
4646

4747
/// Whether this \c UnwrappedLine is part of a preprocessor directive.
4848
bool InPPDirective;
49+
/// Whether it is part of a macro body.
50+
bool InMacroBody;
4951

5052
bool MustBeDeclaration;
5153

@@ -353,8 +355,8 @@ struct UnwrappedLineNode {
353355
};
354356

355357
inline UnwrappedLine::UnwrappedLine()
356-
: Level(0), InPPDirective(false), MustBeDeclaration(false),
357-
MatchingOpeningBlockLineIndex(kInvalidIndex) {}
358+
: Level(0), InPPDirective(false), InMacroBody(false),
359+
MustBeDeclaration(false), MatchingOpeningBlockLineIndex(kInvalidIndex) {}
358360

359361
} // end namespace format
360362
} // end namespace clang

clang/unittests/Format/FormatTest.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9864,6 +9864,10 @@ TEST_F(FormatTest, UnderstandsTemplateParameters) {
98649864
verifyFormat("Constructor(A... a) : a_(X<A>{std::forward<A>(a)}...) {}");
98659865
verifyFormat("< < < < < < < < < < < < < < < < < < < < < < < < < < < < < <");
98669866
verifyFormat("some_templated_type<decltype([](int i) { return i; })>");
9867+
9868+
verifyFormat("#define FOO(typeName, realClass) \\\n"
9869+
" { #typeName, foo<FooType>(new foo<realClass>(#typeName)) }",
9870+
getLLVMStyleWithColumns(60));
98679871
}
98689872

98699873
TEST_F(FormatTest, UnderstandsShiftOperators) {

clang/unittests/Format/TokenAnnotatorTest.cpp

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -284,6 +284,17 @@ TEST_F(TokenAnnotatorTest, UnderstandsVariableTemplates) {
284284
EXPECT_TOKEN(Tokens[13], tok::ampamp, TT_BinaryOperator);
285285
}
286286

287+
TEST_F(TokenAnnotatorTest, UnderstandsTemplatesInMacros) {
288+
auto Tokens =
289+
annotate("#define FOO(typeName) \\\n"
290+
" { #typeName, foo<FooType>(new foo<realClass>(#typeName)) }");
291+
ASSERT_EQ(Tokens.size(), 27u) << Tokens;
292+
EXPECT_TOKEN(Tokens[11], tok::less, TT_TemplateOpener);
293+
EXPECT_TOKEN(Tokens[13], tok::greater, TT_TemplateCloser);
294+
EXPECT_TOKEN(Tokens[17], tok::less, TT_TemplateOpener);
295+
EXPECT_TOKEN(Tokens[19], tok::greater, TT_TemplateCloser);
296+
}
297+
287298
TEST_F(TokenAnnotatorTest, UnderstandsWhitespaceSensitiveMacros) {
288299
FormatStyle Style = getLLVMStyle();
289300
Style.WhitespaceSensitiveMacros.push_back("FOO");

0 commit comments

Comments
 (0)