[clang] [clang-format] Fix erroneous BraceWrapping.BeforeLambdaBody column calcs (PR #76673)

Björn Schäpers via cfe-commits cfe-commits at lists.llvm.org
Tue Jan 2 10:56:54 PST 2024


================
@@ -366,8 +366,14 @@ bool ContinuationIndenter::mustBreak(const LineState &State) {
   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 LambdaBodyLength > getColumnLimit(State);
+    if (Current.MatchingParen->MustBreakBefore)
----------------
HazardyKnusperkeks wrote:

> I noticed there is code in `TokenAnnotator` than can set `MustBreakBefore` on the lambda left brace token:
> 
> https://github.com/llvm/llvm-project/blob/0e01c72c5645259d9a08a1a7ed39cb5cc41ce311/clang/lib/Format/TokenAnnotator.cpp#L5297-L5299
> 
> Introduced in [37c2233](https://github.com/llvm/llvm-project/commit/37c2233097ac44697b87228d86eef1fce10ea5c1) seemingly for code like this:
> 
> ```c++
> auto select = [this]() -> std::unique_ptr<Object> { return MyAssignment::SelectFromList(this); }
> ```
> 
> Both before this current PR and here the `MustBreakBefore` field of the `Current` token is ignored. Should behavior change to observe this and thus force a break?
> 
> Would seem the _BraceWrapping.BeforeLambdaBody_ setting would then force lambda across multiple lines if on, but not if off.
> 
> Should the code in `TokenAnnotator` be changed and maybe only set `CanBreakBefore`?

So as far as I put everything together we should break before the `{`, if `BeforeLambdaBody` is set. Except for short lambdas, if `AllowShortLambdasOnASingleLine` matches our lambda **and** it would fit on a single line. If the lambda has to be broken somewhere, it should break before `{`.

What is needed to be changed I don't know, but also have not the time to investigate. I'm willing to hear your analysis, or maybe @mydeveloperday can shed some light into the issue?

https://github.com/llvm/llvm-project/pull/76673


More information about the cfe-commits mailing list