[PATCH] D146042: [clang-format] Fix numerous issues with "LambdaBodyIndentation: OuterScope" option
Jon Phillips via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Sun Mar 19 04:56:53 PDT 2023
jp4a50 added a comment.
In D146042#4204513 <https://reviews.llvm.org/D146042#4204513>, @owenpan wrote:
> Looks like this patch doesn't put the opening brace at the outerscope for some of the examples from the linked github issues:
>
> $ cat .clang-format
> AllowShortLambdasOnASingleLine: None
> BraceWrapping:
> BeforeLambdaBody: true
> BreakBeforeBraces: Custom
> LambdaBodyIndentation: OuterScope
> $ cat test.cpp
> aaaaaaaaaaaaaaaaaaaa(1,
> b(
> []
> {
> return 0;
> }));
>
> some_function(a_really_long_name, another_long_name, a_third_long_name,
> [&](LineCallback line_callback)
> {
> int a;
> int b;
> });
> $ clang-format test.cpp
> aaaaaaaaaaaaaaaaaaaa(1, b(
> []
> {
> return 0;
> }));
>
> some_function(a_really_long_name, another_long_name, a_third_long_name,
> [&](LineCallback line_callback)
> {
> int a;
> int b;
> });
> $
>
> Shouldn't the expected output be the same as the input?
I'm confident that the patch will indent all those examples correctly when they are at block scope which is the only place those snippets will actually be valid code. I added an exception (see comment in ContinuationIndenter.cpp) to the code to disable `OuterScope`'s behaviour when the line's indentation level is 0 (i.e. statements at namespace scope) because otherwise you can end up with things like:
Namespace::Foo::Foo(
Arg arg1, Arg arg2,
Arg arg3, Arg arg4)
: init1{arg1},
init2{[arg2]() {
return arg2;
}},
init3{arg3},
init4{arg4} {}
The main rationale behind the `OuterScope` style is that the lambda body will be indented similarly to the code block of an `if` statement (or `for` etc) inside function definitions/blocks, but this breaks down when you have something like an constructor initializer which lives outside of a code block. The way I've handled this case isn't particularly elegant, though, so I'd be happy to remove the relevant code from this diff as it's not necessary to fix the issues I've linked - it was an extra bit of behaviour that my team (authors of the original implementation of `OuterScope`) wanted.
FYI I will be away until next Monday now but thanks for the feedback so far.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D146042/new/
https://reviews.llvm.org/D146042
More information about the cfe-commits
mailing list