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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
