[clang] [clang-format] Disable OuterScope lambda indentation behaviour for constructor initializers (PR #66755)
https://github.com/HazardyKnusperkeks closed https://github.com/llvm/llvm-project/pull/66755 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Disable OuterScope lambda indentation behaviour for constructor initializers (PR #66755)
jp4a50 wrote: Thanks for the reviews! Can someone please merge for me? @HazardyKnusperkeks @owenca https://github.com/llvm/llvm-project/pull/66755 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Disable OuterScope lambda indentation behaviour for constructor initializers (PR #66755)
https://github.com/mydeveloperday approved this pull request. https://github.com/llvm/llvm-project/pull/66755 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Disable OuterScope lambda indentation behaviour for constructor initializers (PR #66755)
https://github.com/mydeveloperday resolved https://github.com/llvm/llvm-project/pull/66755 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Disable OuterScope lambda indentation behaviour for constructor initializers (PR #66755)
https://github.com/mydeveloperday approved this pull request. https://github.com/llvm/llvm-project/pull/66755 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Disable OuterScope lambda indentation behaviour for constructor initializers (PR #66755)
jp4a50 wrote: > 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. https://github.com/llvm/llvm-project/pull/66755 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Disable OuterScope lambda indentation behaviour for constructor initializers (PR #66755)
https://github.com/jp4a50 updated
https://github.com/llvm/llvm-project/pull/66755
>From e07d263a37ef37a21bb5cf424b14be83088eed3d Mon Sep 17 00:00:00 2001
From: Jon Phillips
Date: Tue, 19 Sep 2023 10:29:57 +0100
Subject: [PATCH 1/3] [clang-format] Disable OuterScope lambda indentation
behaviour for 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.
---
clang/lib/Format/ContinuationIndenter.cpp | 3 ++-
clang/unittests/Format/FormatTest.cpp | 30 +--
2 files changed, 25 insertions(+), 8 deletions(-)
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 ) {
void ContinuationIndenter::moveStateToNewBlock(LineState ) {
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 , const auto ) {\n"
- " return foo.baz < bar.baz;\n"
- "});",
+ verifyFormat("void test() {\n"
+ " std::sort(v.begin(), v.end(),\n"
+ "[](const auto , const auto ) {\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 qux;\n"
+ " };\n"
+ "}\n",
Style);
Style.AlignAfterOpenBracket = FormatStyle::BAS_AlwaysBreak;
// FIXME: The following test should pass, but fails at the time of writing.
>From 7d7d8e51448f9bcb329c12d738f58e016c0f81ee Mon Sep 17 00:00:00 2001
From: Jon Phillips
Date: Fri, 22 Sep 2023 16:18:45 +0100
Subject: [PATCH 2/3] [clang-format] Add more tests for constructor initializer
formatting when using OuterScope lambda body indentation.
---
clang/lib/Format/ContinuationIndenter.cpp | 2 +-
clang/unittests/Format/FormatTest.cpp | 25 ++-
2 files changed, 25 insertions(+), 2 deletions(-)
diff --git a/clang/lib/Format/ContinuationIndenter.cpp
b/clang/lib/Format/ContinuationIndenter.cpp
index 0fa70eace90f416..70abef1169751ce 100644
--- a/clang/lib/Format/ContinuationIndenter.cpp
+++ b/clang/lib/Format/ContinuationIndenter.cpp
@@ -1955,7 +1955,7 @@ void
ContinuationIndenter::moveStatePastScopeCloser(LineState ) {
void ContinuationIndenter::moveStateToNewBlock(LineState ) {
if (Style.LambdaBodyIndentation == FormatStyle::LBI_OuterScope &&
- State.NextToken->is(TT_LambdaLBrace) && State.Line &&
+ State.NextToken->is(TT_LambdaLBrace) &&
!State.Line->MightBeFunctionDecl) {
State.Stack.back().NestedBlockIndent = State.FirstIndent;
}
diff --git a/clang/unittests/Format/FormatTest.cpp
b/clang/unittests/Format/FormatTest.cpp
index cb6b22b32e63350..6c67c4253254da1 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -22568,6 +22568,12 @@ TEST_F(FormatTest, FormatsLambdas) {
"x);
\\\n"
[clang] [clang-format] Disable OuterScope lambda indentation behaviour for constructor initializers (PR #66755)
HazardyKnusperkeks wrote: You should adapt the documentation. https://github.com/llvm/llvm-project/pull/66755 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Disable OuterScope lambda indentation behaviour for constructor initializers (PR #66755)
https://github.com/owenca approved this pull request. LGTM, but please wait for @mydeveloperday. https://github.com/llvm/llvm-project/pull/66755 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Disable OuterScope lambda indentation behaviour for constructor initializers (PR #66755)
https://github.com/owenca resolved https://github.com/llvm/llvm-project/pull/66755 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Disable OuterScope lambda indentation behaviour for constructor initializers (PR #66755)
https://github.com/owenca resolved https://github.com/llvm/llvm-project/pull/66755 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Disable OuterScope lambda indentation behaviour for constructor initializers (PR #66755)
https://github.com/owenca resolved https://github.com/llvm/llvm-project/pull/66755 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Disable OuterScope lambda indentation behaviour for constructor initializers (PR #66755)
@@ -22537,10 +22537,12 @@ TEST_F(FormatTest, FormatsLambdas) {
" }\n"
"}",
Style);
- verifyFormat("std::sort(v.begin(), v.end(),\n"
- " [](const auto , const auto ) {\n"
- " return foo.baz < bar.baz;\n"
- "});",
jp4a50 wrote:
I've added a test which puts the `std::sort(...)` in a macro definition. It
works as expected.
I haven't added the original test back since I still think it's misleading but
if you still think I should keep it I will add it back (and adjust it to pass
as-is). Just let me know.
https://github.com/llvm/llvm-project/pull/66755
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Disable OuterScope lambda indentation behaviour for constructor initializers (PR #66755)
https://github.com/jp4a50 updated
https://github.com/llvm/llvm-project/pull/66755
>From e07d263a37ef37a21bb5cf424b14be83088eed3d Mon Sep 17 00:00:00 2001
From: Jon Phillips
Date: Tue, 19 Sep 2023 10:29:57 +0100
Subject: [PATCH 1/2] [clang-format] Disable OuterScope lambda indentation
behaviour for 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.
---
clang/lib/Format/ContinuationIndenter.cpp | 3 ++-
clang/unittests/Format/FormatTest.cpp | 30 +--
2 files changed, 25 insertions(+), 8 deletions(-)
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 ) {
void ContinuationIndenter::moveStateToNewBlock(LineState ) {
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 , const auto ) {\n"
- " return foo.baz < bar.baz;\n"
- "});",
+ verifyFormat("void test() {\n"
+ " std::sort(v.begin(), v.end(),\n"
+ "[](const auto , const auto ) {\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 qux;\n"
+ " };\n"
+ "}\n",
Style);
Style.AlignAfterOpenBracket = FormatStyle::BAS_AlwaysBreak;
// FIXME: The following test should pass, but fails at the time of writing.
>From 7d7d8e51448f9bcb329c12d738f58e016c0f81ee Mon Sep 17 00:00:00 2001
From: Jon Phillips
Date: Fri, 22 Sep 2023 16:18:45 +0100
Subject: [PATCH 2/2] [clang-format] Add more tests for constructor initializer
formatting when using OuterScope lambda body indentation.
---
clang/lib/Format/ContinuationIndenter.cpp | 2 +-
clang/unittests/Format/FormatTest.cpp | 25 ++-
2 files changed, 25 insertions(+), 2 deletions(-)
diff --git a/clang/lib/Format/ContinuationIndenter.cpp
b/clang/lib/Format/ContinuationIndenter.cpp
index 0fa70eace90f416..70abef1169751ce 100644
--- a/clang/lib/Format/ContinuationIndenter.cpp
+++ b/clang/lib/Format/ContinuationIndenter.cpp
@@ -1955,7 +1955,7 @@ void
ContinuationIndenter::moveStatePastScopeCloser(LineState ) {
void ContinuationIndenter::moveStateToNewBlock(LineState ) {
if (Style.LambdaBodyIndentation == FormatStyle::LBI_OuterScope &&
- State.NextToken->is(TT_LambdaLBrace) && State.Line &&
+ State.NextToken->is(TT_LambdaLBrace) &&
!State.Line->MightBeFunctionDecl) {
State.Stack.back().NestedBlockIndent = State.FirstIndent;
}
diff --git a/clang/unittests/Format/FormatTest.cpp
b/clang/unittests/Format/FormatTest.cpp
index cb6b22b32e63350..6c67c4253254da1 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -22568,6 +22568,12 @@ TEST_F(FormatTest, FormatsLambdas) {
"x);
\\\n"
[clang] [clang-format] Disable OuterScope lambda indentation behaviour for constructor initializers (PR #66755)
@@ -22537,10 +22537,12 @@ TEST_F(FormatTest, FormatsLambdas) {
" }\n"
"}",
Style);
- verifyFormat("std::sort(v.begin(), v.end(),\n"
- " [](const auto , const auto ) {\n"
- " return foo.baz < bar.baz;\n"
- "});",
owenca wrote:
> I can add this back in but the reason I removed it in favour of putting the
> equivalent code inside block scope is because the code in this example is a
> violation of C++ grammar given it is not actually a declaration but is at
> namespace/global scope.
Yeah, but many clang-format test cases (`return` statements, function calls,
etc) seem "invalid" because they are not wrapped in a block. What if the test
case above is in a macro definition?
https://github.com/llvm/llvm-project/pull/66755
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Disable OuterScope lambda indentation behaviour for constructor initializers (PR #66755)
@@ -22537,10 +22537,12 @@ TEST_F(FormatTest, FormatsLambdas) {
" }\n"
"}",
Style);
- verifyFormat("std::sort(v.begin(), v.end(),\n"
- " [](const auto , const auto ) {\n"
- " return foo.baz < bar.baz;\n"
- "});",
+ verifyFormat("void test() {\n"
jp4a50 wrote:
I've answered the equivalent question here:
https://github.com/llvm/llvm-project/pull/66755#discussion_r1331456960
https://github.com/llvm/llvm-project/pull/66755
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Disable OuterScope lambda indentation behaviour for constructor initializers (PR #66755)
@@ -22537,10 +22537,12 @@ TEST_F(FormatTest, FormatsLambdas) {
" }\n"
"}",
Style);
- verifyFormat("std::sort(v.begin(), v.end(),\n"
- " [](const auto , const auto ) {\n"
- " return foo.baz < bar.baz;\n"
- "});",
jp4a50 wrote:
I can add this back in but the reason I removed it in favour of putting the
equivalent code inside block scope is because the code in this example is a
violation of C++ grammar given it is not actually a declaration but is at
namespace/global scope. As such, the new logic incorrectly assumes that this is
a declaration and would therefore would **not** indent the lambda body wrt. the
start of the expression, which I think would be misleading to readers.
Simply putting this code inside block scope captures the intention of original
test case, I think.
https://github.com/llvm/llvm-project/pull/66755
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Disable OuterScope lambda indentation behaviour for constructor initializers (PR #66755)
jp4a50 wrote: > 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. https://github.com/llvm/llvm-project/pull/66755 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Disable OuterScope lambda indentation behaviour for constructor initializers (PR #66755)
@@ -22537,10 +22537,12 @@ TEST_F(FormatTest, FormatsLambdas) {
" }\n"
"}",
Style);
- verifyFormat("std::sort(v.begin(), v.end(),\n"
- " [](const auto , const auto ) {\n"
- " return foo.baz < bar.baz;\n"
- "});",
+ verifyFormat("void test() {\n"
mydeveloperday wrote:
beyonce rule please don't change a test to fit your code change, or we get
regressions... was the test wrong before?
https://github.com/llvm/llvm-project/pull/66755
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Disable OuterScope lambda indentation behaviour for constructor initializers (PR #66755)
https://github.com/mydeveloperday requested changes to this pull request. https://github.com/llvm/llvm-project/pull/66755 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Disable OuterScope lambda indentation behaviour for constructor initializers (PR #66755)
https://github.com/mydeveloperday edited https://github.com/llvm/llvm-project/pull/66755 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Disable OuterScope lambda indentation behaviour for constructor initializers (PR #66755)
mydeveloperday wrote: 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? https://github.com/llvm/llvm-project/pull/66755 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Disable OuterScope lambda indentation behaviour for constructor initializers (PR #66755)
@@ -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 qux;\n"
+ " };\n"
+ "}\n",
owenca wrote:
```suggestion
"}",
```
https://github.com/llvm/llvm-project/pull/66755
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Disable OuterScope lambda indentation behaviour for constructor initializers (PR #66755)
@@ -1955,7 +1955,8 @@ void
ContinuationIndenter::moveStatePastScopeCloser(LineState ) {
void ContinuationIndenter::moveStateToNewBlock(LineState ) {
if (Style.LambdaBodyIndentation == FormatStyle::LBI_OuterScope &&
- State.NextToken->is(TT_LambdaLBrace)) {
+ State.NextToken->is(TT_LambdaLBrace) && State.Line &&
owenca wrote:
```suggestion
State.NextToken->is(TT_LambdaLBrace) &&
```
https://github.com/llvm/llvm-project/pull/66755
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Disable OuterScope lambda indentation behaviour for constructor initializers (PR #66755)
https://github.com/owenca requested changes to this pull request. Can you add test cases with `BreakConstructorInitializer` set to `AfterColon`? https://github.com/llvm/llvm-project/pull/66755 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Disable OuterScope lambda indentation behaviour for constructor initializers (PR #66755)
@@ -22537,10 +22537,12 @@ TEST_F(FormatTest, FormatsLambdas) {
" }\n"
"}",
Style);
- verifyFormat("std::sort(v.begin(), v.end(),\n"
- " [](const auto , const auto ) {\n"
- " return foo.baz < bar.baz;\n"
- "});",
owenca wrote:
Can you put it back in? In general, we want to keep existing test cases.
https://github.com/llvm/llvm-project/pull/66755
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Disable OuterScope lambda indentation behaviour for constructor initializers (PR #66755)
https://github.com/owenca edited https://github.com/llvm/llvm-project/pull/66755 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Disable OuterScope lambda indentation behaviour for constructor initializers (PR #66755)
llvmbot wrote:
@llvm/pr-subscribers-clang-format
Changes
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.
---
Full diff: https://github.com/llvm/llvm-project/pull/66755.diff
2 Files Affected:
- (modified) clang/lib/Format/ContinuationIndenter.cpp (+2-1)
- (modified) clang/unittests/Format/FormatTest.cpp (+23-7)
``diff
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 ) {
void ContinuationIndenter::moveStateToNewBlock(LineState ) {
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 , const auto ) {\n"
- " return foo.baz < bar.baz;\n"
- "});",
+ verifyFormat("void test() {\n"
+ " std::sort(v.begin(), v.end(),\n"
+ "[](const auto , const auto ) {\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 qux;\n"
+ " };\n"
+ "}\n",
Style);
Style.AlignAfterOpenBracket = FormatStyle::BAS_AlwaysBreak;
// FIXME: The following test should pass, but fails at the time of writing.
``
https://github.com/llvm/llvm-project/pull/66755
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Disable OuterScope lambda indentation behaviour for constructor initializers (PR #66755)
https://github.com/jp4a50 created
https://github.com/llvm/llvm-project/pull/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.
>From e07d263a37ef37a21bb5cf424b14be83088eed3d Mon Sep 17 00:00:00 2001
From: Jon Phillips
Date: Tue, 19 Sep 2023 10:29:57 +0100
Subject: [PATCH] [clang-format] Disable OuterScope lambda indentation
behaviour for 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.
---
clang/lib/Format/ContinuationIndenter.cpp | 3 ++-
clang/unittests/Format/FormatTest.cpp | 30 +--
2 files changed, 25 insertions(+), 8 deletions(-)
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 ) {
void ContinuationIndenter::moveStateToNewBlock(LineState ) {
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 , const auto ) {\n"
- " return foo.baz < bar.baz;\n"
- "});",
+ verifyFormat("void test() {\n"
+ " std::sort(v.begin(), v.end(),\n"
+ "[](const auto , const auto ) {\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 qux;\n"
+ " };\n"
+ "}\n",
Style);
Style.AlignAfterOpenBracket = FormatStyle::BAS_AlwaysBreak;
// FIXME: The following test should pass, but fails at the time of writing.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
