[clang-format] Disable OuterScope lambda indentation behaviour for constructor initializers #66755
Conversation
constructor initializers By default, OuterScope aligns lambdas to the beginning of the current line. This makes sense for most types of statements within code blocks but leads to unappealing and misleading indentation for lambdas within constructor initializers.
|
@llvm/pr-subscribers-clang @llvm/pr-subscribers-clang-format ChangesBy default, OuterScope aligns lambdas to the beginning of the current line. This makes sense for most types of statements within code blocks but leads to unappealing and misleading indentation for lambdas within constructor initializers. Full diff: https://github.com/llvm/llvm-project/pull/66755.diff 2 Files Affected:
diff --git a/clang/lib/Format/ContinuationIndenter.cpp b/clang/lib/Format/ContinuationIndenter.cpp
index deb3e554fdc124b..0fa70eace90f416 100644
--- a/clang/lib/Format/ContinuationIndenter.cpp
+++ b/clang/lib/Format/ContinuationIndenter.cpp
@@ -1955,7 +1955,8 @@ void ContinuationIndenter::moveStatePastScopeCloser(LineState &State) {
void ContinuationIndenter::moveStateToNewBlock(LineState &State) {
if (Style.LambdaBodyIndentation == FormatStyle::LBI_OuterScope &&
- State.NextToken->is(TT_LambdaLBrace)) {
+ State.NextToken->is(TT_LambdaLBrace) && State.Line &&
+ !State.Line->MightBeFunctionDecl) {
State.Stack.back().NestedBlockIndent = State.FirstIndent;
}
unsigned NestedBlockIndent = State.Stack.back().NestedBlockIndent;
diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp
index 0d0fbdb84e3271b..cb6b22b32e63350 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -22537,10 +22537,12 @@ TEST_F(FormatTest, FormatsLambdas) {
" }\n"
"}",
Style);
- verifyFormat("std::sort(v.begin(), v.end(),\n"
- " [](const auto &foo, const auto &bar) {\n"
- " return foo.baz < bar.baz;\n"
- "});",
+ verifyFormat("void test() {\n"
+ " std::sort(v.begin(), v.end(),\n"
+ " [](const auto &foo, const auto &bar) {\n"
+ " return foo.baz < bar.baz;\n"
+ " });\n"
+ "};",
Style);
verifyFormat("void test() {\n"
" (\n"
@@ -22589,9 +22591,23 @@ TEST_F(FormatTest, FormatsLambdas) {
verifyFormat("Namespace::Foo::Foo(LongClassName bar,\n"
" AnotherLongClassName baz)\n"
" : baz{baz}, func{[&] {\n"
- " auto qux = bar;\n"
- " return aFunkyFunctionCall(qux);\n"
- "}} {}",
+ " auto qux = bar;\n"
+ " return aFunkyFunctionCall(qux);\n"
+ " }} {}",
+ Style);
+ verifyFormat("void foo() {\n"
+ " class Foo {\n"
+ " public:\n"
+ " Foo()\n"
+ " : qux{[](int quux) {\n"
+ " auto tmp = quux;\n"
+ " return tmp;\n"
+ " }} {}\n"
+ "\n"
+ " private:\n"
+ " std::function<void(int quux)> qux;\n"
+ " };\n"
+ "}\n",
Style);
Style.AlignAfterOpenBracket = FormatStyle::BAS_AlwaysBreak;
// FIXME: The following test should pass, but fails at the time of writing.
|
owenca
requested review from
mydeveloperday,
HazardyKnusperkeks,
rymiel and
owenca
|
I'm not sure I understand, looking at https://clang.llvm.org/docs/ClangFormatStyleOptions.html#lambdabodyindentation it seems to explicitly state it should be at the outer function scope, can you point to a github issue this is trying to solve or is this just personal preference? |
When my team originally proposed and authored this option the intention was for it to apply to expression statements within blocks such that statements such that the bodies of lambdas as function call arguments (like the example in the docs) and on the RHS of assignment expressions (etc.) would align with the start of the expression, minimizing indentation. We just didn't think of or consider cases such as lambdas used within constructor initializers where the start of the constructor is not the start of the expression so aligning with respect to the start of the constructor is confusing. So I'd say that this can be considered personal preference to a certain extent but also an attempt to honour the intended behaviour of the option when we proposed it. I accept that implementing this as an edge case is clunkier than I'd like, though. |
when using OuterScope lambda body indentation.
|
You should adapt the documentation. |
Done. Tried to capture the original intention of the option which is to minimize (and make consistent) indentation of lambdas at block scope. This PR addresses the most obvious exception to block scope but there may be more that we can address going forwards. |
|
Thanks for the reviews! Can someone please merge for me? @HazardyKnusperkeks @owenca |
…nstructor initializers (llvm#66755) By default, OuterScope aligns lambdas to the beginning of the current line. This makes sense for most types of statements within code blocks but leads to unappealing and misleading indentation for lambdas within constructor initializers.
By default, OuterScope aligns lambdas to the beginning of the current line. This makes sense for most types of statements within code blocks but leads to unappealing and misleading indentation for lambdas within constructor initializers.