[clang-format] Recognize nested blocks

[sstwcw]

Summary

Now a block gets recognized as a block if it contains a block.

The special cases we added don't have additional tests because they
are already tested for example in FormatTest.FormatsLambdas and
FormatTestJS.ContainerLiterals.

Fixes https://github.com/llvm/llvm-project/issues/33891

[sstwcw]

• Remove the special case for operator[], it isn't necessary.

[galenelias]

Coincidentally I also sent out a review to fix this issue yesterday, but went with a different approach of trying to scope the ProbablyBracedList logic by just looking at the lbrace previous token.

https://reviews.llvm.org/D150403

[HazardyKnusperkeks]

On this change I like the token annotator tests, and reusing the same stack instead of adding a second one which always has the same size.
But the code changes in D150403 seem to be better, at least I couldn't think directly on something which may not work.

clang/lib/Format/UnwrappedLineParser.cpp

492

Shouldn't it be called NextBraceIsProbablyLambda (or similar)?

659

How does this work on

{
int foo[]={2,5};
int bar[]{2};
}

or

{
SomeClass foo[]={{arg1, whatever}, {arg2, dontcare}};
SomeClass foo[]{{arg1, whatever}, {arg2, dontcare}};
}

?

clang/unittests/Format/TokenAnnotatorTest.cpp

43

Nice!

[sstwcw]

I like D150403 better.

clang/lib/Format/UnwrappedLineParser.cpp

 void UnwrappedLineParser::calculateBraceTypes(bool ExpectClassBody) {
   // We'll parse forward through the tokens until we hit
   // a closing brace or eof - note that getNextToken() will
   // parse macros, so this will magically work inside macro
   // definitions, too.
   unsigned StoredPosition = Tokens->getPosition();
   FormatToken *Tok = FormatTok;
   const FormatToken *PrevTok = Tok->Previous;
+  // A rough guess at whether a block is a lambda expression.
+  bool ProbablyLambda = false;
   // 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, *NextTok;
+    bool ProbablyLambda;
+  };
+  SmallVector<StackEntry, 8> LBraceStack;
   assert(Tok->is(tok::l_brace));
   do {
     // Get next non-comment token.
     FormatToken *NextTok;
     do {
       NextTok = Tokens->getNextToken();
     } while (NextTok->is(tok::comment));
 
case tok::l_brace:
           Tok->setBlockKind(BK_BracedInit);
         } else if (PrevTok->is(tok::r_paren)) {
           // `) { }` can only occur in function or method declarations in JS.
           Tok->setBlockKind(BK_Block);
         }
       } else {
         Tok->setBlockKind(BK_Unknown);
       }
-      LBraceStack.push_back(Tok);
+      LBraceStack.push_back({Tok, PrevTok, NextTok, ProbablyLambda});
       break;
     case tok::r_brace:
       if (LBraceStack.empty())
         break;
-      if (LBraceStack.back()->is(BK_Unknown)) {
+      switch (LBraceStack.back().Tok->getBlockKind()) {
+      case BK_Unknown: {
         bool ProbablyBracedList = false;
         if (Style.Language == FormatStyle::LK_Proto) {
           ProbablyBracedList = NextTok->isOneOf(tok::comma, tok::r_square);
         } else {
           // Skip NextTok over preprocessor lines, otherwise we may not
           // properly diagnose the block as a braced intializer
           // if the comma separator appears after the pp directive.
           while (NextTok->is(tok::hash)) {
case tok::r_brace:
           // Try to detect a braced list. Note that regardless how we mark inner
           // braces here, we will overwrite the BlockKind later if we parse a
           // braced list (where all blocks inside are by default braced lists),
           // or when we explicitly detect blocks (for example while parsing
           // lambdas).
 
           // 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() &&
                                 NextTok->isOneOf(Keywords.kw_of, Keywords.kw_in,
                                                  Keywords.kw_as));
           ProbablyBracedList = ProbablyBracedList ||
                                (Style.isCpp() && NextTok->is(tok::l_paren));
 
case tok::r_brace:
             // We can have an array subscript after a braced init
             // list, but C++11 attributes are expected after blocks.
             NextTok = Tokens->getNextToken();
             ProbablyBracedList = NextTok->isNot(tok::l_square);
           }
         }
         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);
+        }
+        break;
+      case BK_Block:
+        // A block usually means the outer braces are also a block, except for
+        // the cases below where a block is inside an expression.
+        // A statement expression.
+        if ((NextTok && NextTok->is(tok::r_paren)) ||
+            (LBraceStack.back().PrevTok &&
+             LBraceStack.back().PrevTok->isOneOf(tok::l_paren, tok::caret,
+                                                 TT_FatArrow))) {
+          break;
+        }
+        // A lambda expression.  If the closing parenthesis is the condition
+        // part of a for loop, the block should already have been marked as a
+        // block when the `for` keyword is encountered.
+        if (LBraceStack.back().ProbablyLambda)
+          break;
+        if (LBraceStack.size() >= 2 &&
+            LBraceStack.end()[-2].Tok->is(BK_Unknown)) {
+          LBraceStack.end()[-2].Tok->setBlockKind(BK_Block);
+        }
+        break;
+      case BK_BracedInit:
+        break;
       }
       LBraceStack.pop_back();
       break;
     case tok::identifier:
+      if (Style.isJavaScript() && Tok->is(Keywords.kw_function)) {
+        ProbablyLambda = true;
+        break;
+      }
       if (!Tok->is(TT_StatementMacro))
         break;
       [[fallthrough]];
     case tok::at:
     case tok::semi:
     case tok::kw_if:
     case tok::kw_while:
     case tok::kw_for:
     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);
+      ProbablyLambda = false;
+      if (!LBraceStack.empty() && LBraceStack.back().Tok->is(BK_Unknown))
+        LBraceStack.back().Tok->setBlockKind(BK_Block);
+      break;
+    case tok::l_square:
+      ProbablyLambda = true;
       break;
     default:
       break;
     }
     PrevTok = Tok;
     Tok = NextTok;
   } 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 (auto Entry : LBraceStack)
+    if (Entry.Tok->is(BK_Unknown))
+      Entry.Tok->setBlockKind(BK_Block);
 
   FormatTok = Tokens->setPosition(StoredPosition);
 }
 
 template <class T>
 static inline void hash_combine(std::size_t &seed, const T &v) {
   std::hash<T> hasher;
   seed ^= hasher(v) + 0x9e3779b9 + (seed << 6) + (seed >> 2);

clang/unittests/Format/FormatTest.cpp

 }
 
 TEST_F(FormatTest, FormatsUnwrappedLinesAtFirstFormat) {
   EXPECT_EQ("int i;", format("int\ni;"));
 }
 
 TEST_F(FormatTest, FormatsNestedBlockStatements) {
   EXPECT_EQ("{\n  {\n    {}\n  }\n}", format("{{{}}}"));
+  // The innermost block is on a single line because a block with only 1
+  // statement is hard-coded to be squeezed into one line in
+  // ContinuationIndenter, not because it is recognized as an initializer list.
+  verifyFormat("int main() {\n"
+               "  {\n"
+               "    { int a = 0; }\n"
+               "  }\n"
+               "  {}\n"
+               "}");
 }
 
 TEST_F(FormatTest, FormatsNestedCall) {
   verifyFormat("Method(f1, f2(f3));");
   verifyFormat("Method(f1(f2, f3()));");
   verifyFormat("Method(f1(f2, (f3())));");
 }
 

clang/unittests/Format/TokenAnnotatorTest.cpp

 };
 
 #define EXPECT_TOKEN_KIND(FormatTok, Kind)                                     \
   EXPECT_EQ((FormatTok)->Tok.getKind(), Kind) << *(FormatTok)
 #define EXPECT_TOKEN_TYPE(FormatTok, Type)                                     \
   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)
 #define EXPECT_TOKEN(FormatTok, Kind, Type)                                    \
   do {                                                                         \
     EXPECT_TOKEN_KIND(FormatTok, Kind);                                        \
     EXPECT_TOKEN_TYPE(FormatTok, Type);                                        \
   } while (false)
 
 TEST_F(TokenAnnotatorTest, UnderstandsUsesOfStarAndAmp) {
   auto Tokens = annotate("auto x = [](const decltype(x) &ptr) {};");
TEST_F(TokenAnnotatorTest, UnderstandsLabels) {
   Tokens = annotate("{ x: { break; } }");
   ASSERT_EQ(Tokens.size(), 9u) << Tokens;
   EXPECT_TOKEN(Tokens[2], tok::colon, TT_GotoLabelColon);
   Tokens = annotate("{ case x: { break; } }");
   ASSERT_EQ(Tokens.size(), 10u) << Tokens;
   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