[clang] 6dcde65 - This is a retry of https://reviews.llvm.org/D114583, which was backed
Owen Pan via cfe-commits
cfe-commits at lists.llvm.org
Mon May 22 20:26:03 PDT 2023
Author: Galen Elias
Date: 2023-05-22T20:25:55-07:00
New Revision: 6dcde658b2380d7ca1451ea5d1099af3e294ea16
URL: https://github.com/llvm/llvm-project/commit/6dcde658b2380d7ca1451ea5d1099af3e294ea16
DIFF: https://github.com/llvm/llvm-project/commit/6dcde658b2380d7ca1451ea5d1099af3e294ea16.diff
LOG: This is a retry of https://reviews.llvm.org/D114583, which was backed
out for regressions.
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 adding tracking of the previous token to the LBraceStack
to help scope this particular case.
TokenAnnotatorTests cherry picked from https://reviews.llvm.org/D150452.
Fixes #33891.
Fixes #52911.
Differential Revision: https://reviews.llvm.org/D150403
Added:
Modified:
clang/lib/Format/UnwrappedLineParser.cpp
clang/unittests/Format/FormatTest.cpp
clang/unittests/Format/TokenAnnotatorTest.cpp
Removed:
################################################################################
diff --git a/clang/lib/Format/UnwrappedLineParser.cpp b/clang/lib/Format/UnwrappedLineParser.cpp
index 31b45fa7cc898..66c1205757257 100644
--- a/clang/lib/Format/UnwrappedLineParser.cpp
+++ b/clang/lib/Format/UnwrappedLineParser.cpp
@@ -491,7 +491,11 @@ void UnwrappedLineParser::calculateBraceTypes(bool ExpectClassBody) {
// Keep a stack of positions of lbrace tokens. We will
// update information about whether an lbrace starts a
// braced init list or a
diff erent block during the loop.
- SmallVector<FormatToken *, 8> LBraceStack;
+ struct StackEntry {
+ FormatToken *Tok;
+ const FormatToken *PrevTok;
+ };
+ SmallVector<StackEntry, 8> LBraceStack;
assert(Tok->is(tok::l_brace));
do {
// Get next non-comment token.
@@ -521,12 +525,12 @@ void UnwrappedLineParser::calculateBraceTypes(bool ExpectClassBody) {
} else {
Tok->setBlockKind(BK_Unknown);
}
- LBraceStack.push_back(Tok);
+ LBraceStack.push_back({Tok, PrevTok});
break;
case tok::r_brace:
if (LBraceStack.empty())
break;
- if (LBraceStack.back()->is(BK_Unknown)) {
+ if (LBraceStack.back().Tok->is(BK_Unknown)) {
bool ProbablyBracedList = false;
if (Style.Language == FormatStyle::LK_Proto) {
ProbablyBracedList = NextTok->isOneOf(tok::comma, tok::r_square);
@@ -554,7 +558,7 @@ void UnwrappedLineParser::calculateBraceTypes(bool ExpectClassBody) {
// If we already marked the opening brace as braced list, the closing
// must also be part of it.
- ProbablyBracedList = LBraceStack.back()->is(TT_BracedListLBrace);
+ ProbablyBracedList = LBraceStack.back().Tok->is(TT_BracedListLBrace);
ProbablyBracedList = ProbablyBracedList ||
(Style.isJavaScript() &&
@@ -570,8 +574,14 @@ void UnwrappedLineParser::calculateBraceTypes(bool ExpectClassBody) {
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) && LBraceStack.back().PrevTok &&
+ LBraceStack.back().PrevTok->is(tok::identifier));
ProbablyBracedList =
ProbablyBracedList ||
@@ -595,10 +605,10 @@ void UnwrappedLineParser::calculateBraceTypes(bool ExpectClassBody) {
}
if (ProbablyBracedList) {
Tok->setBlockKind(BK_BracedInit);
- LBraceStack.back()->setBlockKind(BK_BracedInit);
+ LBraceStack.back().Tok->setBlockKind(BK_BracedInit);
} else {
Tok->setBlockKind(BK_Block);
- LBraceStack.back()->setBlockKind(BK_Block);
+ LBraceStack.back().Tok->setBlockKind(BK_Block);
}
}
LBraceStack.pop_back();
@@ -615,8 +625,8 @@ void UnwrappedLineParser::calculateBraceTypes(bool ExpectClassBody) {
case tok::kw_switch:
case tok::kw_try:
case tok::kw___try:
- if (!LBraceStack.empty() && LBraceStack.back()->is(BK_Unknown))
- LBraceStack.back()->setBlockKind(BK_Block);
+ if (!LBraceStack.empty() && LBraceStack.back().Tok->is(BK_Unknown))
+ LBraceStack.back().Tok->setBlockKind(BK_Block);
break;
default:
break;
@@ -626,9 +636,9 @@ void UnwrappedLineParser::calculateBraceTypes(bool ExpectClassBody) {
} while (Tok->isNot(tok::eof) && !LBraceStack.empty());
// Assume other blocks for all unclosed opening braces.
- for (FormatToken *LBrace : LBraceStack)
- if (LBrace->is(BK_Unknown))
- LBrace->setBlockKind(BK_Block);
+ for (const auto &Entry : LBraceStack)
+ if (Entry.Tok->is(BK_Unknown))
+ Entry.Tok->setBlockKind(BK_Block);
FormatTok = Tokens->setPosition(StoredPosition);
}
diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp
index 942c6259015e9..28a4008080566 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -13732,6 +13732,26 @@ TEST_F(FormatTest, FormatsBracedListsInColumnLayout) {
" 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(
diff --git a/clang/unittests/Format/TokenAnnotatorTest.cpp b/clang/unittests/Format/TokenAnnotatorTest.cpp
index b875b6f7144ef..6a3ecc6deb5f9 100644
--- a/clang/unittests/Format/TokenAnnotatorTest.cpp
+++ b/clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -40,6 +40,8 @@ class TokenAnnotatorTest : public ::testing::Test {
EXPECT_EQ((FormatTok)->getType(), Type) << *(FormatTok)
#define EXPECT_TOKEN_PRECEDENCE(FormatTok, Prec) \
EXPECT_EQ((FormatTok)->getPrecedence(), Prec) << *(FormatTok)
+#define EXPECT_BRACE_KIND(FormatTok, Kind) \
+ EXPECT_EQ(FormatTok->getBlockKind(), Kind) << *(FormatTok)
#define EXPECT_TOKEN(FormatTok, Kind, Type) \
do { \
EXPECT_TOKEN_KIND(FormatTok, Kind); \
@@ -1800,6 +1802,22 @@ TEST_F(TokenAnnotatorTest, UnderstandsLabels) {
EXPECT_TOKEN(Tokens[3], tok::colon, TT_CaseLabelColon);
}
+TEST_F(TokenAnnotatorTest, UnderstandsNestedBlocks) {
+ // The closing braces are not annotated. It doesn't seem to cause a problem.
+ // So we only test for the opening braces.
+ auto Tokens = annotate("{\n"
+ " {\n"
+ " { int a = 0; }\n"
+ " }\n"
+ " {}\n"
+ "}");
+ ASSERT_EQ(Tokens.size(), 14u) << Tokens;
+ EXPECT_BRACE_KIND(Tokens[0], BK_Block);
+ EXPECT_BRACE_KIND(Tokens[1], BK_Block);
+ EXPECT_BRACE_KIND(Tokens[2], BK_Block);
+ EXPECT_BRACE_KIND(Tokens[10], BK_Block);
+}
+
} // namespace
} // namespace format
} // namespace clang
More information about the cfe-commits
mailing list