[clang-format] Reorder TokenAnnotator::canBreakBefore

[gedare]

Move the checks related to breaking before right braces and right parens earlier to avoid conflicting checks that prevent breaking based on the left-hand token. This allows properly formatting declarations with pointers and references at a minimum.

[llvmbot]

@llvm/pr-subscribers-clang-format

Author: Gedare Bloom (gedare)

Changes

Move the checks related to breaking before right braces and right parens earlier to avoid conflicting checks that prevent breaking based on the left-hand token. This allows properly formatting declarations with pointers and references at a minimum.

---

Full diff: https://github.com/llvm/llvm-project/pull/119044.diff

2 Files Affected:

• (modified) clang/lib/Format/TokenAnnotator.cpp (+28-27)
• (modified) clang/unittests/Format/FormatTest.cpp (+17)

diff --git a/clang/lib/Format/TokenAnnotator.cpp b/clang/lib/Format/TokenAnnotator.cpp
index bc5239209f3aab..d41e63393a3a4c 100644
--- a/clang/lib/Format/TokenAnnotator.cpp
+++ b/clang/lib/Format/TokenAnnotator.cpp
@@ -6105,6 +6105,34 @@ bool TokenAnnotator::canBreakBefore(const AnnotatedLine &Line,
       return false;
   }
 
+
+  // We only break before r_brace if there was a corresponding break before
+  // the l_brace, which is tracked by BreakBeforeClosingBrace.
+  if (Right.is(tok::r_brace)) {
+    return Right.MatchingParen && (Right.MatchingParen->is(BK_Block) ||
+                                   (Right.isBlockIndentedInitRBrace(Style)));
+  }
+
+  // We only break before r_paren if we're in a block indented context.
+  if (Right.is(tok::r_paren)) {
+    if (Style.AlignAfterOpenBracket != FormatStyle::BAS_BlockIndent ||
+        !Right.MatchingParen) {
+      return false;
+    }
+    auto Next = Right.Next;
+    if (Next && Next->is(tok::r_paren))
+      Next = Next->Next;
+    if (Next && Next->is(tok::l_paren))
+      return false;
+    const FormatToken *Previous = Right.MatchingParen->Previous;
+    return !(Previous && (Previous->is(tok::kw_for) || Previous->isIf()));
+  }
+
+  if (Left.isOneOf(tok::r_paren, TT_TrailingAnnotation) &&
+      Right.is(TT_TrailingAnnotation) &&
+      Style.AlignAfterOpenBracket == FormatStyle::BAS_BlockIndent) {
+    return false;
+  }
   if (Left.is(tok::at))
     return false;
   if (Left.Tok.getObjCKeywordID() == tok::objc_interface)
@@ -6260,33 +6288,6 @@ bool TokenAnnotator::canBreakBefore(const AnnotatedLine &Line,
     return false;
   }
 
-  // We only break before r_brace if there was a corresponding break before
-  // the l_brace, which is tracked by BreakBeforeClosingBrace.
-  if (Right.is(tok::r_brace)) {
-    return Right.MatchingParen && (Right.MatchingParen->is(BK_Block) ||
-                                   (Right.isBlockIndentedInitRBrace(Style)));
-  }
-
-  // We only break before r_paren if we're in a block indented context.
-  if (Right.is(tok::r_paren)) {
-    if (Style.AlignAfterOpenBracket != FormatStyle::BAS_BlockIndent ||
-        !Right.MatchingParen) {
-      return false;
-    }
-    auto Next = Right.Next;
-    if (Next && Next->is(tok::r_paren))
-      Next = Next->Next;
-    if (Next && Next->is(tok::l_paren))
-      return false;
-    const FormatToken *Previous = Right.MatchingParen->Previous;
-    return !(Previous && (Previous->is(tok::kw_for) || Previous->isIf()));
-  }
-
-  if (Left.isOneOf(tok::r_paren, TT_TrailingAnnotation) &&
-      Right.is(TT_TrailingAnnotation) &&
-      Style.AlignAfterOpenBracket == FormatStyle::BAS_BlockIndent) {
-    return false;
-  }
 
   // Allow breaking after a trailing annotation, e.g. after a method
   // declaration.
diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp
index 250e51b5421664..47a22fd3826291 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -9383,6 +9383,13 @@ TEST_F(FormatTest, AlignsAfterOpenBracket) {
       "        aaaaaaaaaaaaaaaaaaaa(aaaaaaaaaaaaaaaaa, aaaaaaaaaaaaaaaa)) &&\n"
       "    aaaaaaaaaaaaaaaa);",
       Style);
+  verifyFormat("void foo(\n"
+               "    void (*foobarpntr)(\n"
+               "        aaaaaaaaaaaaaaaaaa *,\n"
+               "        bbbbbbbbbbbbbb *,\n"
+               "        cccccccccccccccccccc *,\n"
+               "        dddddddddddddddddd *));",
+               Style);
   verifyFormat(
       "fooooooooooo(new BARRRRRRRRR(\n"
       "    XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXZZZZZZZZZZZZZZZZZZZZZZZZZ()));",
@@ -9441,6 +9448,16 @@ TEST_F(FormatTest, AlignsAfterOpenBracket) {
       "    aaaaaaaaaaaaaaaa\n"
       ");",
       Style);
+  verifyFormat("void foo(\n"
+               "    void (*foobarpntr)(\n"
+               "        aaaaaaaaaaaaaaaaaa *,\n"
+               "        bbbbbbbbbbbbbb *,\n"
+               "        cccccccccccccccccccc *,\n"
+               "        dddddddddddddddddd *\n"
+               "    )\n"
+               ");",
+               Style);
+
   verifyFormat("aaaaaaa<bbbbbbbb> const aaaaaaaaaa{\n"
                "    aaaaaaaaaaaaa(aaaaaaaaaaa, aaaaaaaaaaaaaaaa)\n"
                "};",

[gedare]

Without this patch, the test case on main:

[ RUN      ] FormatTest.AlignsAfterOpenBracket
/home/gedare/rtems/llvm-project/clang/unittests/Format/FormatTestBase.h:90: Failure
Expected equality of these values:
  ExpectedCode
    Which is: "void foo(\n    void (*foobarpntr)(\n        aaaaaaaaaaaaaaaaaa *,\n        bbbbbbbbbbbbbb *,\n        cccccccccccccccccccc *,\n        dddddddddddddddddd *\n    )\n);"
  FormattedCode
    Which is: "void foo(\n    void (*foobarpntr)(aaaaaaaaaaaaaaaaaa *, bbbbbbbbbbbbbb *, cccccccccccccccccccc *, dddddddddddddddddd *)\n);"
With diff:
@@ -1,8 +1,3 @@
 void foo(
-    void (*foobarpntr)(
-        aaaaaaaaaaaaaaaaaa *,
-        bbbbbbbbbbbbbb *,
-        cccccccccccccccccccc *,
-        dddddddddddddddddd *
-    )
+    void (*foobarpntr)(aaaaaaaaaaaaaaaaaa *, bbbbbbbbbbbbbb *, cccccccccccccccccccc *, dddddddddddddddddd *)
 );

Google Test trace:
/home/gedare/rtems/llvm-project/clang/unittests/Format/FormatTest.cpp:9451: void foo(
    void (*foobarpntr)(
        aaaaaaaaaaaaaaaaaa *,
        bbbbbbbbbbbbbb *,
        cccccccccccccccccccc *,
        dddddddddddddddddd *
    )
);

[  FAILED  ] FormatTest.AlignsAfterOpenBracket (379 ms)

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.