[clang] [clang-format] Don't merge a short block for SBS_Never (PR #88238)

Owen Pan via cfe-commits cfe-commits at lists.llvm.org
Wed Apr 10 00:42:41 PDT 2024


https://github.com/owenca updated https://github.com/llvm/llvm-project/pull/88238

>From 4122a4f0e189afa7aff1010f0061b4f4d2b2a627 Mon Sep 17 00:00:00 2001
From: Owen Pan <owenpiano at gmail.com>
Date: Wed, 10 Apr 2024 00:03:21 -0700
Subject: [PATCH 1/2] [clang-format] Don't merge a short block for SBS_Never

Also fix unit tests.

Fixes #87484.
---
 clang/lib/Format/FormatToken.h                |  2 +
 clang/lib/Format/UnwrappedLineFormatter.cpp   |  8 +-
 clang/lib/Format/UnwrappedLineParser.cpp      |  7 +-
 clang/unittests/Format/BracesRemoverTest.cpp  |  4 +-
 clang/unittests/Format/FormatTest.cpp         | 84 +++++++++++++++----
 .../Format/FormatTestMacroExpansion.cpp       |  7 +-
 clang/unittests/Format/TokenAnnotatorTest.cpp | 20 +++++
 7 files changed, 106 insertions(+), 26 deletions(-)

diff --git a/clang/lib/Format/FormatToken.h b/clang/lib/Format/FormatToken.h
index 48b6a9092a8c09..f651e6228c206d 100644
--- a/clang/lib/Format/FormatToken.h
+++ b/clang/lib/Format/FormatToken.h
@@ -35,6 +35,8 @@ namespace format {
   TYPE(BinaryOperator)                                                         \
   TYPE(BitFieldColon)                                                          \
   TYPE(BlockComment)                                                           \
+  /* l_brace of a block that is not the body of a (e.g. loop) statement. */    \
+  TYPE(BlockLBrace)                                                            \
   TYPE(BracedListLBrace)                                                       \
   /* The colon at the end of a case label. */                                  \
   TYPE(CaseLabelColon)                                                         \
diff --git a/clang/lib/Format/UnwrappedLineFormatter.cpp b/clang/lib/Format/UnwrappedLineFormatter.cpp
index fb31980ab9f491..4ae54e56331bdc 100644
--- a/clang/lib/Format/UnwrappedLineFormatter.cpp
+++ b/clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -796,8 +796,12 @@ class LineJoiner {
       }
     }
 
-    if (const auto *LastNonComment = Line.getLastNonComment();
-        LastNonComment && LastNonComment->is(tok::l_brace)) {
+    if (Line.endsWith(tok::l_brace)) {
+      if (Style.AllowShortBlocksOnASingleLine == FormatStyle::SBS_Never &&
+          Line.First->is(TT_BlockLBrace)) {
+        return 0;
+      }
+
       if (IsSplitBlock && Line.First == Line.Last &&
           I > AnnotatedLines.begin() &&
           (I[-1]->endsWith(tok::kw_else) || IsCtrlStmt(*I[-1]))) {
diff --git a/clang/lib/Format/UnwrappedLineParser.cpp b/clang/lib/Format/UnwrappedLineParser.cpp
index c1f7e2874beb24..603268f771ac52 100644
--- a/clang/lib/Format/UnwrappedLineParser.cpp
+++ b/clang/lib/Format/UnwrappedLineParser.cpp
@@ -395,9 +395,10 @@ bool UnwrappedLineParser::parseLevel(const FormatToken *OpeningBrace,
         ParseDefault();
         continue;
       }
-      if (!InRequiresExpression && FormatTok->isNot(TT_MacroBlockBegin) &&
-          tryToParseBracedList()) {
-        continue;
+      if (!InRequiresExpression && FormatTok->isNot(TT_MacroBlockBegin)) {
+        if (tryToParseBracedList())
+          continue;
+        FormatTok->setFinalizedType(TT_BlockLBrace);
       }
       parseBlock();
       ++StatementCount;
diff --git a/clang/unittests/Format/BracesRemoverTest.cpp b/clang/unittests/Format/BracesRemoverTest.cpp
index 5155eefb9e08c9..2e983b887ffcb2 100644
--- a/clang/unittests/Format/BracesRemoverTest.cpp
+++ b/clang/unittests/Format/BracesRemoverTest.cpp
@@ -209,7 +209,9 @@ TEST_F(BracesRemoverTest, RemoveBraces) {
   verifyFormat("if (a) {\n"
                "  b;\n"
                "} else {\n"
-               "  { c; }\n"
+               "  {\n"
+               "    c;\n"
+               "  }\n"
                "}",
                Style);
 
diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp
index f312a9e21158a9..4906b3350b5b22 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -52,7 +52,13 @@ TEST_F(FormatTest, FormatsUnwrappedLinesAtFirstFormat) {
 }
 
 TEST_F(FormatTest, FormatsNestedBlockStatements) {
-  verifyFormat("{\n  {\n    {}\n  }\n}", "{{{}}}");
+  verifyFormat("{\n"
+               "  {\n"
+               "    {\n"
+               "    }\n"
+               "  }\n"
+               "}",
+               "{{{}}}");
 }
 
 TEST_F(FormatTest, FormatsNestedCall) {
@@ -5669,7 +5675,10 @@ TEST_F(FormatTest, LayoutCodeInMacroDefinitions) {
                getLLVMStyleWithColumns(14));
 }
 
-TEST_F(FormatTest, LayoutRemainingTokens) { verifyFormat("{}"); }
+TEST_F(FormatTest, LayoutRemainingTokens) {
+  verifyFormat("{\n"
+               "}");
+}
 
 TEST_F(FormatTest, MacroDefinitionInsideStatement) {
   verifyFormat("int x,\n"
@@ -6577,7 +6586,11 @@ TEST_F(FormatTest, FormatAlignInsidePreprocessorElseBlock) {
 }
 
 TEST_F(FormatTest, FormatHashIfNotAtStartOfLine) {
-  verifyFormat("{\n  { a #c; }\n}");
+  verifyFormat("{\n"
+               "  {\n"
+               "    a #c;\n"
+               "  }\n"
+               "}");
 }
 
 TEST_F(FormatTest, FormatUnbalancedStructuralElements) {
@@ -6937,13 +6950,13 @@ TEST_F(FormatTest, FormatNestedBlocksInMacros) {
 }
 
 TEST_F(FormatTest, PutEmptyBlocksIntoOneLine) {
-  verifyFormat("{}");
   verifyFormat("enum E {};");
   verifyFormat("enum E {}");
   FormatStyle Style = getLLVMStyle();
   Style.SpaceInEmptyBlock = true;
   verifyFormat("void f() { }", "void f() {}", Style);
   Style.AllowShortBlocksOnASingleLine = FormatStyle::SBS_Empty;
+  verifyFormat("{ }", Style);
   verifyFormat("while (true) { }", "while (true) {}", Style);
   Style.BreakBeforeBraces = FormatStyle::BS_Custom;
   Style.BraceWrapping.BeforeElse = false;
@@ -11527,10 +11540,18 @@ TEST_F(FormatTest, UnderstandsNewAndDelete) {
                "void new (link p);\n"
                "void delete (link p);");
 
-  verifyFormat("{ p->new(); }\n"
-               "{ p->delete(); }",
-               "{ p->new (); }\n"
-               "{ p->delete (); }");
+  verifyFormat("{\n"
+               "  p->new();\n"
+               "}\n"
+               "{\n"
+               "  p->delete();\n"
+               "}",
+               "{\n"
+               "  p->new ();\n"
+               "}\n"
+               "{\n"
+               "  p->delete ();\n"
+               "}");
 
   FormatStyle AfterPlacementOperator = getLLVMStyle();
   AfterPlacementOperator.SpaceBeforeParens = FormatStyle::SBPO_Custom;
@@ -12352,7 +12373,9 @@ TEST_F(FormatTest, FormatsCasts) {
   // FIXME: single value wrapped with paren will be treated as cast.
   verifyFormat("void f(int i = (kValue)*kMask) {}");
 
-  verifyFormat("{ (void)F; }");
+  verifyFormat("{\n"
+               "  (void)F;\n"
+               "}");
 
   // Don't break after a cast's
   verifyFormat("int aaaaaaaaaaaaaaaaaaaaaaaaaaa =\n"
@@ -13575,7 +13598,8 @@ TEST_F(FormatTest, IncorrectAccessSpecifier) {
   verifyFormat("public\n"
                "B {}");
   verifyFormat("public\n"
-               "{}");
+               "{\n"
+               "}");
   verifyFormat("public\n"
                "B { int x; }");
 }
@@ -13632,10 +13656,31 @@ TEST_F(FormatTest, DoesNotTouchUnwrappedLinesWithErrors) {
 }
 
 TEST_F(FormatTest, IncorrectCodeErrorDetection) {
-  verifyFormat("{\n  {}", "{\n{\n}");
-  verifyFormat("{\n  {}", "{\n  {\n}");
-  verifyFormat("{\n  {}", "{\n  {\n  }");
-  verifyFormat("{\n  {}\n}\n}", "{\n  {\n    }\n  }\n}");
+  verifyFormat("{\n"
+               "  {\n"
+               "  }",
+               "{\n"
+               "{\n"
+               "}");
+  verifyFormat("{\n"
+               "  {\n"
+               "  }",
+               "{\n"
+               "  {\n"
+               "}");
+  verifyFormat("{\n"
+               "  {\n"
+               "  }");
+  verifyFormat("{\n"
+               "  {\n"
+               "  }\n"
+               "}\n"
+               "}",
+               "{\n"
+               "  {\n"
+               "    }\n"
+               "  }\n"
+               "}");
 
   verifyFormat("{\n"
                "  {\n"
@@ -14080,10 +14125,14 @@ TEST_F(FormatTest, FormatsBracedListsInColumnLayout) {
                "}");
   verifyFormat("void foo() {\n"
                "  { // asdf\n"
-               "    { int a; }\n"
+               "    {\n"
+               "      int a;\n"
+               "    }\n"
                "  }\n"
                "  {\n"
-               "    { int b; }\n"
+               "    {\n"
+               "      int b;\n"
+               "    }\n"
                "  }\n"
                "}");
   verifyFormat("namespace n {\n"
@@ -14095,7 +14144,8 @@ TEST_F(FormatTest, FormatsBracedListsInColumnLayout) {
                "      }\n"
                "    }\n"
                "  }\n"
-               "  {}\n"
+               "  {\n"
+               "  }\n"
                "}\n"
                "} // namespace n");
 
diff --git a/clang/unittests/Format/FormatTestMacroExpansion.cpp b/clang/unittests/Format/FormatTestMacroExpansion.cpp
index 85ab6ea3794e80..d391fe3d715c38 100644
--- a/clang/unittests/Format/FormatTestMacroExpansion.cpp
+++ b/clang/unittests/Format/FormatTestMacroExpansion.cpp
@@ -43,9 +43,10 @@ TEST_F(FormatTestMacroExpansion, UnexpandConfiguredMacros) {
                "STMT",
                Style);
   verifyFormat("void f() { ID(a *b); }", Style);
-  verifyFormat(R"(ID(
-    { ID(a *b); });
-)",
+  verifyFormat("ID(\n"
+               "    {\n"
+               "      ID(a *b);\n"
+               "    });",
                Style);
   verifyIncompleteFormat("ID3({, ID(a *b),\n"
                          "    ;\n"
diff --git a/clang/unittests/Format/TokenAnnotatorTest.cpp b/clang/unittests/Format/TokenAnnotatorTest.cpp
index c3153cf6b16f07..20ef7f259d8353 100644
--- a/clang/unittests/Format/TokenAnnotatorTest.cpp
+++ b/clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -2856,6 +2856,26 @@ TEST_F(TokenAnnotatorTest, UnderstandsElaboratedTypeSpecifier) {
   EXPECT_TOKEN(Tokens[7], tok::l_brace, TT_FunctionLBrace);
 }
 
+TEST_F(TokenAnnotatorTest, BlockLBrace) {
+  auto Tokens = annotate("{\n"
+                         "  {\n"
+                         "    foo();\n"
+                         "  }\n"
+                         "}");
+  ASSERT_EQ(Tokens.size(), 9u) << Tokens;
+  EXPECT_TOKEN(Tokens[0], tok::l_brace, TT_BlockLBrace);
+  EXPECT_TOKEN(Tokens[1], tok::l_brace, TT_BlockLBrace);
+
+  Tokens = annotate("void bar() {\n"
+                    "  {\n"
+                    "    foo();\n"
+                    "  }\n"
+                    "}");
+  ASSERT_EQ(Tokens.size(), 13u) << Tokens;
+  EXPECT_TOKEN(Tokens[4], tok::l_brace, TT_FunctionLBrace);
+  EXPECT_TOKEN(Tokens[5], tok::l_brace, TT_BlockLBrace);
+}
+
 } // namespace
 } // namespace format
 } // namespace clang

>From a2d63001d6726ad95b90351a4b048348628065bd Mon Sep 17 00:00:00 2001
From: Owen Pan <owenpiano at gmail.com>
Date: Wed, 10 Apr 2024 00:39:52 -0700
Subject: [PATCH 2/2] Add EXPECT_BRACE_KIND() tests.

---
 clang/unittests/Format/TokenAnnotatorTest.cpp | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/clang/unittests/Format/TokenAnnotatorTest.cpp b/clang/unittests/Format/TokenAnnotatorTest.cpp
index 20ef7f259d8353..da02ced8c7a949 100644
--- a/clang/unittests/Format/TokenAnnotatorTest.cpp
+++ b/clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -2864,7 +2864,9 @@ TEST_F(TokenAnnotatorTest, BlockLBrace) {
                          "}");
   ASSERT_EQ(Tokens.size(), 9u) << Tokens;
   EXPECT_TOKEN(Tokens[0], tok::l_brace, TT_BlockLBrace);
+  EXPECT_BRACE_KIND(Tokens[0], BK_Block);
   EXPECT_TOKEN(Tokens[1], tok::l_brace, TT_BlockLBrace);
+  EXPECT_BRACE_KIND(Tokens[1], BK_Block);
 
   Tokens = annotate("void bar() {\n"
                     "  {\n"
@@ -2873,7 +2875,9 @@ TEST_F(TokenAnnotatorTest, BlockLBrace) {
                     "}");
   ASSERT_EQ(Tokens.size(), 13u) << Tokens;
   EXPECT_TOKEN(Tokens[4], tok::l_brace, TT_FunctionLBrace);
+  EXPECT_BRACE_KIND(Tokens[4], BK_Block);
   EXPECT_TOKEN(Tokens[5], tok::l_brace, TT_BlockLBrace);
+  EXPECT_BRACE_KIND(Tokens[5], BK_Block);
 }
 
 } // namespace



More information about the cfe-commits mailing list