caio/clang-p2996
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

This is a retry of https://reviews.llvm.org/D114583, which was backed

Browse Source 
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
This commit is contained in:
Galen Elias
2023-05-22 20:11:17 -07:00
committed by Owen Pan
parent 4fafa39b76
commit 6dcde658b2
3 changed files with 61 additions and 13 deletions
Download Patch File Download Diff File Expand all files Collapse all files

36
clang/lib/Format/UnwrappedLineParser.cpp
View File

@@ -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 different 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);
}

20
clang/unittests/Format/FormatTest.cpp
View File

@@ -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(

18
clang/unittests/Format/TokenAnnotatorTest.cpp
View File

@@ -40,6 +40,8 @@ protected:
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