[clang] 5c614bd - [clang-format] Fix bugs with "LambdaBodyIndentation: OuterScope"
Owen Pan via cfe-commits
cfe-commits at lists.llvm.org
Wed Apr 5 14:38:58 PDT 2023
Author: Jon Phillips
Date: 2023-04-05T14:38:38-07:00
New Revision: 5c614bd88f1252927ca1f9d9f8e802ef5e1eebe2
URL: https://github.com/llvm/llvm-project/commit/5c614bd88f1252927ca1f9d9f8e802ef5e1eebe2
DIFF: https://github.com/llvm/llvm-project/commit/5c614bd88f1252927ca1f9d9f8e802ef5e1eebe2.diff
LOG: [clang-format] Fix bugs with "LambdaBodyIndentation: OuterScope"
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
Added:
Modified:
clang/docs/ClangFormatStyleOptions.rst
clang/docs/ReleaseNotes.rst
clang/include/clang/Format/Format.h
clang/lib/Format/ContinuationIndenter.cpp
clang/lib/Format/UnwrappedLineFormatter.cpp
clang/unittests/Format/FormatTest.cpp
Removed:
################################################################################
diff --git a/clang/docs/ClangFormatStyleOptions.rst b/clang/docs/ClangFormatStyleOptions.rst
index 527bdff5f5b56..f8ab42f46ba81 100644
--- a/clang/docs/ClangFormatStyleOptions.rst
+++ b/clang/docs/ClangFormatStyleOptions.rst
@@ -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:
@@ -3569,6 +3565,11 @@ the configuration (without a prefix: ``Auto``).
return;
});
+ someMethod(someOtherMethod(
+ [](SomeReallyLongLambdaSignatureArgument foo) {
+ return;
+ }));
+
.. _Language:
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 03d14fc89be4c..c07f23af4a629 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -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
--------
diff --git a/clang/include/clang/Format/Format.h b/clang/include/clang/Format/Format.h
index 755621ec53d63..0dfa052822458 100644
--- a/clang/include/clang/Format/Format.h
+++ b/clang/include/clang/Format/Format.h
@@ -2670,6 +2670,11 @@ struct FormatStyle {
/// [](SomeReallyLongLambdaSignatureArgument foo) {
/// return;
/// });
+ ///
+ /// someMethod(someOtherMethod(
+ /// [](SomeReallyLongLambdaSignatureArgument foo) {
+ /// return;
+ /// }));
/// \endcode
LBI_OuterScope,
};
@@ -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;
diff --git a/clang/lib/Format/ContinuationIndenter.cpp b/clang/lib/Format/ContinuationIndenter.cpp
index 6b33ab0a76650..f5748cfc50890 100644
--- a/clang/lib/Format/ContinuationIndenter.cpp
+++ b/clang/lib/Format/ContinuationIndenter.cpp
@@ -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 ||
@@ -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 =
diff --git a/clang/lib/Format/UnwrappedLineFormatter.cpp b/clang/lib/Format/UnwrappedLineFormatter.cpp
index 97500dfbaade9..b545fa02cefb6 100644
--- a/clang/lib/Format/UnwrappedLineFormatter.cpp
+++ b/clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -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);
diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp
index 8d009d6a33a45..4b7fdddfdf7a0 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -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"
"}",
@@ -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) {
More information about the cfe-commits
mailing list