https://github.com/gedare created 
https://github.com/llvm/llvm-project/pull/93140

Fixes #55731 
Fixes #73584 

The reported formatting problems were related to ignoring deep nesting of 
"simple" functions (causing #54808) and to allowing the trailing annotation to 
become separated from the closing parens, which allowed a break to occur 
between the closing parens and the trailing annotation. The fix for the nesting 
of "simple" functions is to detect them more carefully. "Simple" was defined in 
a comment as being a single non-expression argument. I tried to stay as close 
to the original intent of the implementation while fixing the various bad 
formatting reports.

In the process of fixing these bugs, some latent bugs were discovered related 
to how JavaScript Template Strings are handled. Those are also fixed here.

>From 27a2da876926d2157ea9f18c5fd71a2e81e097fc Mon Sep 17 00:00:00 2001
From: Gedare Bloom <ged...@rtems.org>
Date: Tue, 21 May 2024 22:21:59 -0600
Subject: [PATCH] [clang-format] Improve BlockIndent at ColumnLimit

Fixes #55731
Fixes #73584

The reported formatting problems were related to ignoring deep nesting
of "simple" functions (causing #54808) and to allowing the trailing
annotation to become separated from the closing parens, which allowed a
break to occur between the closing parens and the trailing annotation.
The fix for the nesting of "simple" functions is to detect them more
carefully. "Simple" was defined in a comment as being a single
non-expression argument. I tried to stay as close to the original intent
of the implementation while fixing the various bad formatting reports.

In the process of fixing these bugs, some latent bugs were discovered
related to how JavaScript Template Strings are handled. Those are also
fixed here.
---
 clang/lib/Format/ContinuationIndenter.cpp | 47 +++++++++++++++++++++--
 clang/lib/Format/TokenAnnotator.cpp       |  6 +++
 clang/unittests/Format/FormatTest.cpp     | 22 +++++++++++
 3 files changed, 72 insertions(+), 3 deletions(-)

diff --git a/clang/lib/Format/ContinuationIndenter.cpp 
b/clang/lib/Format/ContinuationIndenter.cpp
index 6b9fbfe0ebf53..b0a232e97b314 100644
--- a/clang/lib/Format/ContinuationIndenter.cpp
+++ b/clang/lib/Format/ContinuationIndenter.cpp
@@ -803,6 +803,46 @@ void ContinuationIndenter::addTokenOnCurrentLine(LineState 
&State, bool DryRun,
     return !Tok.Previous->isOneOf(TT_CastRParen, tok::kw_for, tok::kw_while,
                                   tok::kw_switch);
   };
+  // Detecting functions is brittle. It would be better if we could annotate
+  // the LParen type of functions/calls.
+  const auto IsFunctionDeclParen = [&](const FormatToken &Tok) {
+    return Tok.is(tok::l_paren) && Tok.Previous &&
+           (Tok.Previous->is(TT_FunctionDeclarationName) ||
+            (Tok.Previous->Previous &&
+             Tok.Previous->Previous->is(tok::coloncolon) &&
+             Tok.Previous->Previous->Previous &&
+             
Tok.Previous->Previous->Previous->is(TT_FunctionDeclarationName)));
+  };
+  const auto IsFunctionCallParen = [&](const FormatToken &Tok) {
+    return Tok.is(tok::l_paren) && Tok.ParameterCount > 0 && Tok.Previous &&
+           Tok.Previous->is(tok::identifier);
+  };
+  const auto IsInTemplateString = [&](const FormatToken &Tok) {
+    if (!Style.isJavaScript())
+      return false;
+    for (const FormatToken *Prev = &Tok; Prev; Prev = Prev->Previous) {
+      if (Prev->is(TT_TemplateString) && Prev->opensScope())
+        return true;
+      if (Prev->is(TT_TemplateString) && Prev->closesScope())
+        break;
+    }
+    return false;
+  };
+  const auto IsNotSimpleFunction = [&](const FormatToken &Tok) {
+    const auto *Previous = Tok.Previous;
+    const auto *Next = Tok.Next;
+    if (Tok.FakeLParens.size() > 0 && Tok.FakeLParens.back() > prec::Unknown)
+      return true;
+    if (Previous &&
+        (IsFunctionDeclParen(*Previous) || IsFunctionCallParen(*Previous))) {
+      if (!IsOpeningBracket(Tok) && Next && !Next->isMemberAccess() &&
+          !IsInTemplateString(Tok) && !IsFunctionDeclParen(*Next) &&
+          !IsFunctionCallParen(*Next)) {
+        return true;
+      }
+    }
+    return false;
+  };
   if ((Style.AlignAfterOpenBracket == FormatStyle::BAS_AlwaysBreak ||
        Style.AlignAfterOpenBracket == FormatStyle::BAS_BlockIndent) &&
       IsOpeningBracket(Previous) && State.Column > getNewLineColumn(State) &&
@@ -813,10 +853,10 @@ void 
ContinuationIndenter::addTokenOnCurrentLine(LineState &State, bool DryRun,
       //       caaaaaaaaaaaall(
       //           caaaaaaaaaaaall(
       //               caaaaaaaaaaaaaaaaaaaaaaall(aaaaaaaaaaaaaa, 
aaaaaaaaa))));
-      Current.FakeLParens.size() > 0 &&
-      Current.FakeLParens.back() > prec::Unknown) {
+      IsNotSimpleFunction(Current)) {
     CurrentState.NoLineBreak = true;
   }
+
   if (Previous.is(TT_TemplateString) && Previous.opensScope())
     CurrentState.NoLineBreak = true;
 
@@ -831,7 +871,8 @@ void ContinuationIndenter::addTokenOnCurrentLine(LineState 
&State, bool DryRun,
       Previous.isNot(TT_TableGenDAGArgOpenerToBreak) &&
       !(Current.MacroParent && Previous.MacroParent) &&
       (Current.isNot(TT_LineComment) ||
-       Previous.isOneOf(BK_BracedInit, TT_VerilogMultiLineListLParen))) {
+       Previous.isOneOf(BK_BracedInit, TT_VerilogMultiLineListLParen)) &&
+      !IsInTemplateString(Current)) {
     CurrentState.Indent = State.Column + Spaces;
     CurrentState.IsAligned = true;
   }
diff --git a/clang/lib/Format/TokenAnnotator.cpp 
b/clang/lib/Format/TokenAnnotator.cpp
index 7c4c76a91f2c5..094e87db2426a 100644
--- a/clang/lib/Format/TokenAnnotator.cpp
+++ b/clang/lib/Format/TokenAnnotator.cpp
@@ -6157,6 +6157,12 @@ bool TokenAnnotator::canBreakBefore(const AnnotatedLine 
&Line,
     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.
   if (Left.is(TT_TrailingAnnotation)) {
diff --git a/clang/unittests/Format/FormatTest.cpp 
b/clang/unittests/Format/FormatTest.cpp
index a9df994189f00..0413f1f08fd9c 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -9305,6 +9305,28 @@ TEST_F(FormatTest, AlignsAfterOpenBracket) {
       "    aaaaaaaaaaaaaaaa\n"
       ");",
       Style);
+  verifyFormat(
+      "bool aaaaaaaaaaaaaaaaaaaaaaaaaaa(\n"
+      "    const bool &aaaaaaaaa, const void *aaaaaaaaaa\n"
+      ") const {\n"
+      "  return true;\n"
+      "}",
+      Style);
+  verifyFormat(
+      "bool aaaaaaaaaaaaaaaaaaaaaaaa(\n"
+      "    const bool &aaaaaaaaaa, const void *aaaaaaaaaa\n"
+      ") const;",
+      Style);
+  verifyFormat(
+      "void aaaaaaaaa(\n"
+      "    int aaaaaa, int bbbbbb, int cccccc, int dddddddddd\n"
+      ") const noexcept -> std::vector<of_very_long_type>;",
+      Style);
+  verifyFormat(
+      "aaaaaaaaaaaaaaaaaaa(\n"
+      "    \"a aaaaaaa aaaaaaaaaaaaaaaaa aaaaaaaaaaaaaaa aaaaaaaaaaaaa\"\n"
+      ");",
+      Style);
 }
 
 TEST_F(FormatTest, ParenthesesAndOperandAlignment) {

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to