Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[clang-format] Correctly compute SplitPenalty of TrailingReturnArrow #105613

Closed
wants to merge 1 commit into from

Conversation

owenca
Copy link
Contributor

@owenca owenca commented Aug 22, 2024

Fixes #105480.

@llvmbot
Copy link
Member

llvmbot commented Aug 22, 2024

@llvm/pr-subscribers-clang-format

Author: Owen Pan (owenca)

Changes

Fixes #105480.


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

2 Files Affected:

  • (modified) clang/lib/Format/TokenAnnotator.cpp (+2-2)
  • (modified) clang/unittests/Format/TokenAnnotatorTest.cpp (+16)
diff --git a/clang/lib/Format/TokenAnnotator.cpp b/clang/lib/Format/TokenAnnotator.cpp
index 9d4204655b8ed6..fc1bacf8c5ac66 100644
--- a/clang/lib/Format/TokenAnnotator.cpp
+++ b/clang/lib/Format/TokenAnnotator.cpp
@@ -4050,7 +4050,7 @@ void TokenAnnotator::calculateFormattingInformation(AnnotatedLine &Line) const {
                              ChildSize + Current->SpacesRequiredBefore;
     }
 
-    if (Current->is(TT_CtorInitializerColon))
+    if (Current->isOneOf(TT_CtorInitializerColon, TT_LambdaLSquare))
       InFunctionDecl = false;
 
     // FIXME: Only calculate this if CanBreakBefore is true once static
@@ -4211,7 +4211,7 @@ unsigned TokenAnnotator::splitPenalty(const AnnotatedLine &Line,
   }
   if (Right.is(TT_PointerOrReference))
     return 190;
-  if (Right.is(TT_TrailingReturnArrow))
+  if (!InFunctionDecl && Right.is(TT_TrailingReturnArrow))
     return 110;
   if (Left.is(tok::equal) && Right.is(tok::l_brace))
     return 160;
diff --git a/clang/unittests/Format/TokenAnnotatorTest.cpp b/clang/unittests/Format/TokenAnnotatorTest.cpp
index 99798de43e70ff..b057ac49659acf 100644
--- a/clang/unittests/Format/TokenAnnotatorTest.cpp
+++ b/clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -42,6 +42,8 @@ class TokenAnnotatorTest : public testing::Test {
   EXPECT_EQ((FormatTok)->getPrecedence(), Prec) << *(FormatTok)
 #define EXPECT_BRACE_KIND(FormatTok, Kind)                                     \
   EXPECT_EQ(FormatTok->getBlockKind(), Kind) << *(FormatTok)
+#define EXPECT_SPLIT_PENALTY(FormatTok, Penalty)                               \
+  EXPECT_EQ(FormatTok->SplitPenalty, (unsigned)(Penalty)) << *(FormatTok)
 #define EXPECT_TOKEN(FormatTok, Kind, Type)                                    \
   do {                                                                         \
     EXPECT_TOKEN_KIND(FormatTok, Kind);                                        \
@@ -3369,6 +3371,20 @@ TEST_F(TokenAnnotatorTest, GNULanguageStandard) {
   EXPECT_TOKEN(Tokens[2], tok::spaceship, TT_BinaryOperator);
 }
 
+TEST_F(TokenAnnotatorTest, SplitPenalty) {
+  auto Style = getLLVMStyle();
+  Style.ColumnLimit = 20;
+
+  auto Tokens = annotate("class foo {\n"
+                         "  auto bar()\n"
+                         "      -> bool;\n"
+                         "};",
+                         Style);
+  ASSERT_EQ(Tokens.size(), 13u);
+  EXPECT_TOKEN(Tokens[7], tok::arrow, TT_TrailingReturnArrow);
+  EXPECT_SPLIT_PENALTY(Tokens[7], 23);
+}
+
 } // namespace
 } // namespace format
 } // namespace clang

@owenca owenca added this to the LLVM 19.X Release milestone Aug 22, 2024
@owenca
Copy link
Contributor Author

owenca commented Aug 24, 2024

After reviewing commit e00d32a, I've come to the conclusion that it would be better to revert it and keep lambda arrows differentiated from function arrows.

@owenca
Copy link
Contributor Author

owenca commented Aug 24, 2024

See #105923.

@owenca owenca closed this Aug 24, 2024
@owenca owenca deleted the 105480 branch August 29, 2024 02:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging this pull request may close these issues.

Clang-format: Trailing return type formatting regression starting from v18
4 participants