Skip to content

Commit

Permalink
[clang-format] Fix bugs with "LambdaBodyIndentation: OuterScope"
Browse files Browse the repository at this point in the history 
The previous implementation of the option corrupted the parenthesis
state stack. (See https://reviews.llvm.org/D102706.)

Fixes #55708.
Fixes #53212.
Fixes #52846.
Fixes #59954.

Differential Revision: https://reviews.llvm.org/D146042
  • Loading branch information
@jp4a50 @owenca
jp4a50 authored and owenca committed Apr 5, 2023
1 parent 4151477 commit 5c614bd
Show file tree
Hide file tree
Showing 6 changed files with 104 additions and 37 deletions.
11 changes: 6 additions & 5 deletions clang/docs/ClangFormatStyleOptions.rst
View file
Expand Up @@ -3540,11 +3540,7 @@ the configuration (without a prefix: ``Auto``).
causes the lambda body to be indented one additional level relative to
the indentation level of the signature. ``OuterScope`` forces the lambda
body to be indented one additional level relative to the parent scope
containing the lambda signature. For callback-heavy code, it may improve
readability to have the signature indented two levels and to use
``OuterScope``. The KJ style guide requires ``OuterScope``.
`KJ style guide
<https://github.com/capnproto/capnproto/blob/master/style-guide.md>`_
containing the lambda signature.

Possible values:

Expand All @@ -3569,6 +3565,11 @@ the configuration (without a prefix: ``Auto``).
return;
});

someMethod(someOtherMethod(
[](SomeReallyLongLambdaSignatureArgument foo) {
return;
}));



.. _Language:
Expand Down
1 change: 1 addition & 0 deletions clang/docs/ReleaseNotes.rst
View file
Expand Up @@ -427,6 +427,7 @@ clang-format
put the initializers on the next line only.
- Add additional Qualifier Ordering support for special cases such
as templates, requires clauses, long qualified names.
- Fix all known issues associated with ``LambdaBodyIndentation: OuterScope``.

libclang
--------
Expand Down
11 changes: 6 additions & 5 deletions clang/include/clang/Format/Format.h
View file
Expand Up @@ -2670,6 +2670,11 @@ struct FormatStyle {
/// [](SomeReallyLongLambdaSignatureArgument foo) {
/// return;
/// });
///
/// someMethod(someOtherMethod(
/// [](SomeReallyLongLambdaSignatureArgument foo) {
/// return;
/// }));
/// \endcode
LBI_OuterScope,
};
Expand All @@ -2678,11 +2683,7 @@ struct FormatStyle {
/// causes the lambda body to be indented one additional level relative to
/// the indentation level of the signature. ``OuterScope`` forces the lambda
/// body to be indented one additional level relative to the parent scope
/// containing the lambda signature. For callback-heavy code, it may improve
/// readability to have the signature indented two levels and to use
/// ``OuterScope``. The KJ style guide requires ``OuterScope``.
/// `KJ style guide
/// <https://github.com/capnproto/capnproto/blob/master/style-guide.md>`_
/// containing the lambda signature.
/// \version 13
LambdaBodyIndentationKind LambdaBodyIndentation;

Expand Down
14 changes: 12 additions & 2 deletions clang/lib/Format/ContinuationIndenter.cpp
View file
Expand Up @@ -1125,8 +1125,14 @@ unsigned ContinuationIndenter::getNewLineColumn(const LineState &State) {
Style.IndentWidth;
}

if (NextNonComment->is(tok::l_brace) && NextNonComment->is(BK_Block))
return Current.NestingLevel == 0 ? State.FirstIndent : CurrentState.Indent;
if (NextNonComment->is(tok::l_brace) && NextNonComment->is(BK_Block)) {
if (Current.NestingLevel == 0 ||
(Style.LambdaBodyIndentation == FormatStyle::LBI_OuterScope &&
State.NextToken->is(TT_LambdaLBrace))) {
return State.FirstIndent;
}
return CurrentState.Indent;
}
if ((Current.isOneOf(tok::r_brace, tok::r_square) ||
(Current.is(tok::greater) &&
(Style.Language == FormatStyle::LK_Proto ||
Expand Down Expand Up @@ -1830,6 +1836,10 @@ void ContinuationIndenter::moveStatePastScopeCloser(LineState &State) {
}

void ContinuationIndenter::moveStateToNewBlock(LineState &State) {
if (Style.LambdaBodyIndentation == FormatStyle::LBI_OuterScope &&
State.NextToken->is(TT_LambdaLBrace)) {
State.Stack.back().NestedBlockIndent = State.FirstIndent;
}
unsigned NestedBlockIndent = State.Stack.back().NestedBlockIndent;
// ObjC block sometimes follow special indentation rules.
unsigned NewIndent =
Expand Down
11 changes: 0 additions & 11 deletions clang/lib/Format/UnwrappedLineFormatter.cpp
View file
Expand Up @@ -1003,17 +1003,6 @@ class LineFormatter {

int AdditionalIndent =
P.Indent - Previous.Children[0]->Level * Style.IndentWidth;

if (Style.LambdaBodyIndentation == FormatStyle::LBI_OuterScope &&
P.NestedBlockIndent == P.LastSpace) {
if (State.NextToken->MatchingParen &&
State.NextToken->MatchingParen->is(TT_LambdaLBrace)) {
State.Stack.pop_back();
}
if (LBrace->is(TT_LambdaLBrace))
AdditionalIndent = 0;
}

Penalty +=
BlockFormatter->format(Previous.Children, DryRun, AdditionalIndent,
/*FixBadIndentation=*/true);
Expand Down
93 changes: 79 additions & 14 deletions clang/unittests/Format/FormatTest.cpp
View file
Expand Up @@ -22009,9 +22009,9 @@ TEST_F(FormatTest, FormatsLambdas) {
verifyFormat("void test() {\n"
" (\n"
" []() -> auto {\n"
" int b = 32;\n"
" return 3;\n"
" },\n"
" int b = 32;\n"
" return 3;\n"
" },\n"
" foo, bar)\n"
" .foo();\n"
"}",
Expand All @@ -22025,17 +22025,82 @@ TEST_F(FormatTest, FormatsLambdas) {
" .bar();\n"
"}",
Style);
Style = getGoogleStyle();
Style.LambdaBodyIndentation = FormatStyle::LBI_OuterScope;
verifyFormat("#define A \\\n"
" [] { \\\n"
" xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx( \\\n"
" xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx); \\\n"
" }",
Style);
// TODO: The current formatting has a minor issue that's not worth fixing
// right now whereby the closing brace is indented relative to the signature
// instead of being aligned. This only happens with macros.
verifyFormat("#define A \\\n"
" [] { \\\n"
" xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx( \\\n"
" xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx); \\\n"
" }",
Style);
verifyFormat("void foo() {\n"
" aFunction(1, b(c(foo, bar, baz, [](d) {\n"
" auto f = e(d);\n"
" return f;\n"
" })));\n"
"}",
Style);
Style.AlignAfterOpenBracket = FormatStyle::BAS_AlwaysBreak;
verifyFormat("void foo() {\n"
" aFunction(\n"
" 1, b(c(\n"
" [](d) -> Foo {\n"
" auto f = e(d);\n"
" return f;\n"
" },\n"
" foo, Bar{},\n"
" [] {\n"
" auto g = h();\n"
" return g;\n"
" },\n"
" baz)));\n"
"}",
Style);
verifyFormat("void foo() {\n"
" aFunction(1, b(c(foo, Bar{}, baz, [](d) -> Foo {\n"
" auto f = e(\n"
" foo,\n"
" [&] {\n"
" auto g = h();\n"
" return g;\n"
" },\n"
" qux,\n"
" [&] -> Bar {\n"
" auto i = j();\n"
" return i;\n"
" });\n"
" return f;\n"
" })));\n"
"}",
Style);
verifyFormat("Namespace::Foo::Foo(\n"
" LongClassName bar, AnotherLongClassName baz)\n"
" : baz{baz}, func{[&] {\n"
" auto qux = bar;\n"
" return aFunkyFunctionCall(qux);\n"
"}} {}",
Style);
Style.BreakBeforeBraces = FormatStyle::BS_Custom;
Style.BraceWrapping.BeforeLambdaBody = true;
verifyFormat("void foo() {\n"
" aFunction(\n"
" 1, b(c(foo, Bar{}, baz,\n"
" [](d) -> Foo\n"
" {\n"
" auto f = e(\n"
" [&]\n"
" {\n"
" auto g = h();\n"
" return g;\n"
" },\n"
" qux,\n"
" [&] -> Bar\n"
" {\n"
" auto i = j();\n"
" return i;\n"
" });\n"
" return f;\n"
" })));\n"
"}",
Style);
}

TEST_F(FormatTest, LambdaWithLineComments) {
Expand Down

0 comments on commit 5c614bd

Please sign in to comment.