https://github.com/christophe-calmejane updated https://github.com/llvm/llvm-project/pull/182319
>From a79b9a91c200a75157286842524bfeb3ebe719f8 Mon Sep 17 00:00:00 2001 From: Christophe Calmejane <[email protected]> Date: Wed, 18 Feb 2026 17:17:36 +0100 Subject: [PATCH 1/3] [clang-format] Fix BeforeLambdaBody causing incorrect argument wrapping When BraceWrapping.BeforeLambdaBody is true and a function call contains a lambda that is not the last argument (either followed by more arguments or when multiple lambdas are present), the DisallowLineBreaks heuristic in ContinuationIndenter::addTokenOnCurrentLine incorrectly prevents line breaks in the parent scope. This causes all arguments before the lambda to be wrapped to a new line, even though they would fit on the first line. The DisallowLineBreaks heuristic is designed to keep arguments together when lambda bodies are inlined. However, with BeforeLambdaBody, lambda bodies are intentionally expanded using Allman-style braces, so the heuristic should not apply. Fix: Return false from DisallowLineBreaks when BeforeLambdaBody is set. Before: func( a, b, [](int x) { return x; }, c); After: func(a, b, [](int x) { return x; }, c); --- clang/lib/Format/ContinuationIndenter.cpp | 7 +++ clang/unittests/Format/FormatTest.cpp | 58 +++++++++++++++++++++++ 2 files changed, 65 insertions(+) diff --git a/clang/lib/Format/ContinuationIndenter.cpp b/clang/lib/Format/ContinuationIndenter.cpp index cb3dee9d75c23c..6deedb90c273da 100644 --- a/clang/lib/Format/ContinuationIndenter.cpp +++ b/clang/lib/Format/ContinuationIndenter.cpp @@ -811,6 +811,13 @@ void ContinuationIndenter::addTokenOnCurrentLine(LineState &State, bool DryRun, return false; } + // When BeforeLambdaBody is set, lambda bodies are intentionally expanded + // onto their own lines. Do not prevent line breaks in the parent scope, + // as that would force all arguments (including those before the lambda) + // to be wrapped to new lines. + if (Style.BraceWrapping.BeforeLambdaBody) + return false; + // For example, `/*Newline=*/false`. if (Previous.is(TT_BlockComment) && Current.SpacesRequiredBefore == 0) return false; diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp index e9cf68156e63e3..2bcd06cd419184 100644 --- a/clang/unittests/Format/FormatTest.cpp +++ b/clang/unittests/Format/FormatTest.cpp @@ -21740,6 +21740,64 @@ TEST_F(FormatTest, FormatsLambdas) { " })));\n" "}", Style); + + // Verify that BeforeLambdaBody does not cause arguments before the lambda to + // be wrapped to a new line when the lambda is not the last argument. + FormatStyle BeforeLambdaBodyStyle = getLLVMStyle(); + BeforeLambdaBodyStyle.BreakBeforeBraces = FormatStyle::BS_Custom; + BeforeLambdaBodyStyle.BraceWrapping.BeforeLambdaBody = true; + BeforeLambdaBodyStyle.AllowShortLambdasOnASingleLine = + FormatStyle::ShortLambdaStyle::SLS_None; + + // Lambda followed by another argument: args before lambda stay on first line. + verifyFormat("void test() {\n" + " func(a, b,\n" + " [](int x)\n" + " {\n" + " return x;\n" + " },\n" + " c);\n" + "}", + BeforeLambdaBodyStyle); + // Two lambdas as arguments: args before lambdas stay on first line. + verifyFormat("void test() {\n" + " func(a, b,\n" + " [](int x)\n" + " {\n" + " return x;\n" + " },\n" + " [](int y)\n" + " {\n" + " return y;\n" + " });\n" + "}", + BeforeLambdaBodyStyle); + + BeforeLambdaBodyStyle.AllowShortLambdasOnASingleLine = + FormatStyle::ShortLambdaStyle::SLS_Empty; + // With SLS_Empty, lambda followed by another argument. + verifyFormat("void test() {\n" + " func(a, b,\n" + " [](int x)\n" + " {\n" + " return x;\n" + " },\n" + " c);\n" + "}", + BeforeLambdaBodyStyle); + // With SLS_Empty, two lambdas as arguments. + verifyFormat("void test() {\n" + " func(a, b,\n" + " [](int x)\n" + " {\n" + " return x;\n" + " },\n" + " [](int y)\n" + " {\n" + " return y;\n" + " });\n" + "}", + BeforeLambdaBodyStyle); } TEST_F(FormatTest, LambdaWithLineComments) { >From 17b167ea7ab4b37adda166f4c1e3c747a583d306 Mon Sep 17 00:00:00 2001 From: Christophe Calmejane <[email protected]> Date: Fri, 20 Feb 2026 14:27:16 +0100 Subject: [PATCH 2/3] Changed implementation according to code review --- clang/lib/Format/ContinuationIndenter.cpp | 18 ++++++++++-------- clang/lib/Format/TokenAnnotator.cpp | 17 +++++++++++++++++ 2 files changed, 27 insertions(+), 8 deletions(-) diff --git a/clang/lib/Format/ContinuationIndenter.cpp b/clang/lib/Format/ContinuationIndenter.cpp index 6deedb90c273da..905cf7a33516fb 100644 --- a/clang/lib/Format/ContinuationIndenter.cpp +++ b/clang/lib/Format/ContinuationIndenter.cpp @@ -478,7 +478,7 @@ bool ContinuationIndenter::mustBreak(const LineState &State) { if (Style.BraceWrapping.BeforeLambdaBody && Current.CanBreakBefore && Current.is(TT_LambdaLBrace) && Previous.isNot(TT_LineComment)) { auto LambdaBodyLength = getLengthToMatchingParen(Current, State.Stack); - return LambdaBodyLength > getColumnLimit(State); + return Current.MustBreakBefore || LambdaBodyLength > getColumnLimit(State); } if (Style.BraceWrapping.AfterRequiresExpression && Current.CanBreakBefore && Current.is(TT_RequiresExpressionLBrace) && @@ -811,13 +811,6 @@ void ContinuationIndenter::addTokenOnCurrentLine(LineState &State, bool DryRun, return false; } - // When BeforeLambdaBody is set, lambda bodies are intentionally expanded - // onto their own lines. Do not prevent line breaks in the parent scope, - // as that would force all arguments (including those before the lambda) - // to be wrapped to new lines. - if (Style.BraceWrapping.BeforeLambdaBody) - return false; - // For example, `/*Newline=*/false`. if (Previous.is(TT_BlockComment) && Current.SpacesRequiredBefore == 0) return false; @@ -832,6 +825,15 @@ void ContinuationIndenter::addTokenOnCurrentLine(LineState &State, bool DryRun, if (Prev->BlockParameterCount == 0) return false; + // If any lambda brace in the argument list has MustBreakBefore set + // (e.g. for Allman-style BeforeLambdaBody), the "all arguments on one + // line" strategy is impossible, so allow natural line wrapping. + for (const auto *Tok = Prev->Next; Tok && Tok != Prev->MatchingParen; + Tok = Tok->Next) { + if (Tok->is(TT_LambdaLBrace) && Tok->MustBreakBefore) + return false; + } + // Multiple lambdas in the same function call. if (Prev->BlockParameterCount > 1) return true; diff --git a/clang/lib/Format/TokenAnnotator.cpp b/clang/lib/Format/TokenAnnotator.cpp index b3bc5842a40114..9f0af5532ec870 100644 --- a/clang/lib/Format/TokenAnnotator.cpp +++ b/clang/lib/Format/TokenAnnotator.cpp @@ -6238,6 +6238,23 @@ bool TokenAnnotator::mustBreakBefore(AnnotatedLine &Line, return true; } + // When BeforeLambdaBody is set, force a break before function argument lambda + // braces. This ensures Allman-style formatting is applied even when the + // DisallowLineBreaks heuristic would otherwise prevent line breaks in the + // parent scope (e.g. with multiple lambdas or a lambda followed by another + // argument). Only force the break when the lambda brace will definitely be on + // a new line: SLS_None always breaks, and SLS_Empty breaks for non-empty + // lambdas. For SLS_Inline/SLS_All, the brace may stay inline for short + // lambdas, so we let the runtime decision in mustBreak() handle it. + if (Style.BraceWrapping.BeforeLambdaBody && Right.is(TT_LambdaLBrace) && + IsFunctionArgument(Right)) { + auto SLS = Style.AllowShortLambdasOnASingleLine; + if (SLS == FormatStyle::SLS_None || + (SLS == FormatStyle::SLS_Empty && !Right.Children.empty())) { + return true; + } + } + // Put multiple Java annotation on a new line. if ((Style.isJava() || Style.isJavaScript()) && Left.is(TT_LeadingJavaAnnotation) && >From ed7adfacfedb5cf498f19294d0d2eb7f545b34d5 Mon Sep 17 00:00:00 2001 From: Christophe Calmejane <[email protected]> Date: Tue, 6 Oct 2026 17:51:15 +0200 Subject: [PATCH 3/3] More code review changes --- clang/lib/Format/ContinuationIndenter.cpp | 17 ++++++++--------- clang/lib/Format/TokenAnnotator.cpp | 12 +++--------- clang/unittests/Format/FormatTest.cpp | 6 ------ 3 files changed, 11 insertions(+), 24 deletions(-) diff --git a/clang/lib/Format/ContinuationIndenter.cpp b/clang/lib/Format/ContinuationIndenter.cpp index 905cf7a33516fb..bb8179cbc4f95f 100644 --- a/clang/lib/Format/ContinuationIndenter.cpp +++ b/clang/lib/Format/ContinuationIndenter.cpp @@ -475,11 +475,6 @@ bool ContinuationIndenter::mustBreak(const LineState &State) { const FormatToken &Current = *State.NextToken; const FormatToken &Previous = *Current.Previous; const auto &CurrentState = State.Stack.back(); - if (Style.BraceWrapping.BeforeLambdaBody && Current.CanBreakBefore && - Current.is(TT_LambdaLBrace) && Previous.isNot(TT_LineComment)) { - auto LambdaBodyLength = getLengthToMatchingParen(Current, State.Stack); - return Current.MustBreakBefore || LambdaBodyLength > getColumnLimit(State); - } if (Style.BraceWrapping.AfterRequiresExpression && Current.CanBreakBefore && Current.is(TT_RequiresExpressionLBrace) && getLengthToMatchingParen(Current, State.Stack) > getColumnLimit(State)) { @@ -492,6 +487,12 @@ bool ContinuationIndenter::mustBreak(const LineState &State) { Style.ColumnLimit > 0)))) { return true; } + if (Style.BraceWrapping.BeforeLambdaBody && Current.CanBreakBefore && + Current.is(TT_LambdaLBrace) && Previous.isNot(TT_LineComment)) { + const auto LambdaBodyLength = + getLengthToMatchingParen(Current, State.Stack); + return LambdaBodyLength > getColumnLimit(State); + } if (CurrentState.BreakBeforeClosingBrace && (Current.closesBlockOrBlockTypeList(Style) || (Current.is(tok::r_brace) && Current.MatchingParen && @@ -825,10 +826,8 @@ void ContinuationIndenter::addTokenOnCurrentLine(LineState &State, bool DryRun, if (Prev->BlockParameterCount == 0) return false; - // If any lambda brace in the argument list has MustBreakBefore set - // (e.g. for Allman-style BeforeLambdaBody), the "all arguments on one - // line" strategy is impossible, so allow natural line wrapping. - for (const auto *Tok = Prev->Next; Tok && Tok != Prev->MatchingParen; + // A lambda brace that must break makes the single-line strategy impossible. + for (const auto *Tok = &Current; Tok && Tok != Prev->MatchingParen; Tok = Tok->Next) { if (Tok->is(TT_LambdaLBrace) && Tok->MustBreakBefore) return false; diff --git a/clang/lib/Format/TokenAnnotator.cpp b/clang/lib/Format/TokenAnnotator.cpp index 9f0af5532ec870..382f15081ad741 100644 --- a/clang/lib/Format/TokenAnnotator.cpp +++ b/clang/lib/Format/TokenAnnotator.cpp @@ -6238,17 +6238,11 @@ bool TokenAnnotator::mustBreakBefore(AnnotatedLine &Line, return true; } - // When BeforeLambdaBody is set, force a break before function argument lambda - // braces. This ensures Allman-style formatting is applied even when the - // DisallowLineBreaks heuristic would otherwise prevent line breaks in the - // parent scope (e.g. with multiple lambdas or a lambda followed by another - // argument). Only force the break when the lambda brace will definitely be on - // a new line: SLS_None always breaks, and SLS_Empty breaks for non-empty - // lambdas. For SLS_Inline/SLS_All, the brace may stay inline for short - // lambdas, so we let the runtime decision in mustBreak() handle it. + // SLS_Inline and SLS_All may keep short lambdas inline, so the break is + // decided later in ContinuationIndenter::mustBreak(). if (Style.BraceWrapping.BeforeLambdaBody && Right.is(TT_LambdaLBrace) && IsFunctionArgument(Right)) { - auto SLS = Style.AllowShortLambdasOnASingleLine; + const auto SLS = Style.AllowShortLambdasOnASingleLine; if (SLS == FormatStyle::SLS_None || (SLS == FormatStyle::SLS_Empty && !Right.Children.empty())) { return true; diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp index 2bcd06cd419184..d346036448140c 100644 --- a/clang/unittests/Format/FormatTest.cpp +++ b/clang/unittests/Format/FormatTest.cpp @@ -21741,15 +21741,12 @@ TEST_F(FormatTest, FormatsLambdas) { "}", Style); - // Verify that BeforeLambdaBody does not cause arguments before the lambda to - // be wrapped to a new line when the lambda is not the last argument. FormatStyle BeforeLambdaBodyStyle = getLLVMStyle(); BeforeLambdaBodyStyle.BreakBeforeBraces = FormatStyle::BS_Custom; BeforeLambdaBodyStyle.BraceWrapping.BeforeLambdaBody = true; BeforeLambdaBodyStyle.AllowShortLambdasOnASingleLine = FormatStyle::ShortLambdaStyle::SLS_None; - // Lambda followed by another argument: args before lambda stay on first line. verifyFormat("void test() {\n" " func(a, b,\n" " [](int x)\n" @@ -21759,7 +21756,6 @@ TEST_F(FormatTest, FormatsLambdas) { " c);\n" "}", BeforeLambdaBodyStyle); - // Two lambdas as arguments: args before lambdas stay on first line. verifyFormat("void test() {\n" " func(a, b,\n" " [](int x)\n" @@ -21775,7 +21771,6 @@ TEST_F(FormatTest, FormatsLambdas) { BeforeLambdaBodyStyle.AllowShortLambdasOnASingleLine = FormatStyle::ShortLambdaStyle::SLS_Empty; - // With SLS_Empty, lambda followed by another argument. verifyFormat("void test() {\n" " func(a, b,\n" " [](int x)\n" @@ -21785,7 +21780,6 @@ TEST_F(FormatTest, FormatsLambdas) { " c);\n" "}", BeforeLambdaBodyStyle); - // With SLS_Empty, two lambdas as arguments. verifyFormat("void test() {\n" " func(a, b,\n" " [](int x)\n" _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
