galenelias created this revision.
galenelias added reviewers: cpplearner, HazardyKnusperkeks, MyDeveloperDay.
Herald added projects: All, clang, clang-format.
Herald added a subscriber: cfe-commits.
Herald added reviewers: rymiel, owenpan.
galenelias requested review of this revision.
This is a retry of https://reviews.llvm.org/D114583, which was backed
out for regressions. This fixes
https://github.com/llvm/llvm-project/issues/33891.
Clang Format is detecting a nested scope followed by another open brace
as a braced initializer list due to incorrectly thinking it's matching a
braced initializer at the end of a constructor initializer list which is
followed by the body open brace.
Unfortunately, UnwrappedLineParser isn't doing a very detailed parse, so
it's not super straightforward to distinguish these cases given the
current structure of calculateBraceTypes. My current hypothesis is that
these can be disambiguated by looking at the token preceding the
l_brace, as initializer list parameters will be preceded by an
identifier, but a scope block generally will not (barring the MACRO
wildcard).
To this end, I am tracking the previous token type in a stack parallel
to LBraceStack to help scope this particular case.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D150403
Files:
clang/lib/Format/UnwrappedLineParser.cpp
clang/unittests/Format/FormatTest.cpp
Index: clang/unittests/Format/FormatTest.cpp
===================================================================
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -13731,6 +13731,26 @@
" struct Dummy {};\n"
" f(v);\n"
"}");
+ verifyFormat("void foo() {\n"
+ " { // asdf\n"
+ " { int a; }\n"
+ " }\n"
+ " {\n"
+ " { int b; }\n"
+ " }\n"
+ "}");
+ verifyFormat("namespace n {\n"
+ "void foo() {\n"
+ " {\n"
+ " {\n"
+ " statement();\n"
+ " if (false) {\n"
+ " }\n"
+ " }\n"
+ " }\n"
+ " {}\n"
+ "}\n"
+ "} // namespace n");
// Long lists should be formatted in columns even if they are nested.
verifyFormat(
Index: clang/lib/Format/UnwrappedLineParser.cpp
===================================================================
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -492,6 +492,9 @@
// update information about whether an lbrace starts a
// braced init list or a different block during the loop.
SmallVector<FormatToken *, 8> LBraceStack;
+ // Track the previous token type corresponding to our lbraces // to help
+ // detect brace types
+ SmallVector<tok::TokenKind, 8> PrevTokenKindStack;
assert(Tok->is(tok::l_brace));
do {
// Get next non-comment token.
@@ -522,6 +525,8 @@
Tok->setBlockKind(BK_Unknown);
}
LBraceStack.push_back(Tok);
+ PrevTokenKindStack.push_back(PrevTok ? PrevTok->Tok.getKind()
+ : tok::unknown);
break;
case tok::r_brace:
if (LBraceStack.empty())
@@ -570,8 +575,13 @@
ProbablyBracedList =
ProbablyBracedList ||
NextTok->isOneOf(tok::comma, tok::period, tok::colon,
- tok::r_paren, tok::r_square, tok::l_brace,
- tok::ellipsis);
+ tok::r_paren, tok::r_square, tok::ellipsis);
+
+ // Distinguish between braced list in a constructor initializer list
+ // followed by constructor body, or just adjacent blocks
+ ProbablyBracedList = ProbablyBracedList ||
+ NextTok->is(tok::l_brace) &&
+ PrevTokenKindStack.back() ==
tok::identifier;
ProbablyBracedList =
ProbablyBracedList ||
@@ -602,6 +612,7 @@
}
}
LBraceStack.pop_back();
+ PrevTokenKindStack.pop_back();
break;
case tok::identifier:
if (!Tok->is(TT_StatementMacro))
Index: clang/unittests/Format/FormatTest.cpp
===================================================================
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -13731,6 +13731,26 @@
" struct Dummy {};\n"
" f(v);\n"
"}");
+ verifyFormat("void foo() {\n"
+ " { // asdf\n"
+ " { int a; }\n"
+ " }\n"
+ " {\n"
+ " { int b; }\n"
+ " }\n"
+ "}");
+ verifyFormat("namespace n {\n"
+ "void foo() {\n"
+ " {\n"
+ " {\n"
+ " statement();\n"
+ " if (false) {\n"
+ " }\n"
+ " }\n"
+ " }\n"
+ " {}\n"
+ "}\n"
+ "} // namespace n");
// Long lists should be formatted in columns even if they are nested.
verifyFormat(
Index: clang/lib/Format/UnwrappedLineParser.cpp
===================================================================
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -492,6 +492,9 @@
// update information about whether an lbrace starts a
// braced init list or a different block during the loop.
SmallVector<FormatToken *, 8> LBraceStack;
+ // Track the previous token type corresponding to our lbraces // to help
+ // detect brace types
+ SmallVector<tok::TokenKind, 8> PrevTokenKindStack;
assert(Tok->is(tok::l_brace));
do {
// Get next non-comment token.
@@ -522,6 +525,8 @@
Tok->setBlockKind(BK_Unknown);
}
LBraceStack.push_back(Tok);
+ PrevTokenKindStack.push_back(PrevTok ? PrevTok->Tok.getKind()
+ : tok::unknown);
break;
case tok::r_brace:
if (LBraceStack.empty())
@@ -570,8 +575,13 @@
ProbablyBracedList =
ProbablyBracedList ||
NextTok->isOneOf(tok::comma, tok::period, tok::colon,
- tok::r_paren, tok::r_square, tok::l_brace,
- tok::ellipsis);
+ tok::r_paren, tok::r_square, tok::ellipsis);
+
+ // Distinguish between braced list in a constructor initializer list
+ // followed by constructor body, or just adjacent blocks
+ ProbablyBracedList = ProbablyBracedList ||
+ NextTok->is(tok::l_brace) &&
+ PrevTokenKindStack.back() == tok::identifier;
ProbablyBracedList =
ProbablyBracedList ||
@@ -602,6 +612,7 @@
}
}
LBraceStack.pop_back();
+ PrevTokenKindStack.pop_back();
break;
case tok::identifier:
if (!Tok->is(TT_StatementMacro))
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
