[PATCH] D67246: clang-format: Add support for formatting lambdas with explicit template parameters.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG41f4d68a50be: clang-format: Add support for formatting
(some) lambdas with explicit template… (authored by thakis).
Herald added a project: clang.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D67246/new/
https://reviews.llvm.org/D67246
Files:
clang/lib/Format/TokenAnnotator.cpp
clang/lib/Format/UnwrappedLineParser.cpp
clang/unittests/Format/FormatTest.cpp
Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -12923,6 +12923,10 @@
" return 1; //\n"
"};");
+ // Lambdas with explicit template argument lists.
+ verifyFormat(
+ "auto L = [] class T, class U>(T &) {};\n");
+
// Multiple lambdas in the same parentheses change indentation rules. These
// lambdas are forced to start on new lines.
verifyFormat("SomeFunction(\n"
@@ -12940,8 +12944,8 @@
"},\n"
"1);\n");
- // A multi-line lambda passed as arg1 forces arg0 to be pushed out, just
like the arg0
- // case above.
+ // A multi-line lambda passed as arg1 forces arg0 to be pushed out, just like
+ // the arg0 case above.
auto Style = getGoogleStyle();
Style.BinPackArguments = false;
verifyFormat("SomeFunction(\n"
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -1440,8 +1440,11 @@
case tok::identifier:
case tok::numeric_constant:
case tok::coloncolon:
+case tok::kw_class:
case tok::kw_mutable:
case tok::kw_noexcept:
+case tok::kw_template:
+case tok::kw_typename:
nextToken();
break;
// Specialization of a template with an integer parameter can contain
@@ -1455,6 +1458,9 @@
// followed by an `a->b` expression, such as:
// ([obj func:arg] + a->b)
// Otherwise the code below would parse as a lambda.
+//
+// FIXME: This heuristic is incorrect for C++20 generic lambdas with
+// explicit template lists: [](U &){}
case tok::plus:
case tok::minus:
case tok::exclaim:
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -40,6 +40,21 @@
return Tok.Tok.getIdentifierInfo() != nullptr;
}
+/// With `Left` being '(', check if we're at either `[...](` or
+/// `[...]<...>(`, where the [ opens a lambda capture list.
+static bool isLambdaParameterList(const FormatToken *Left) {
+ // Skip <...> if present.
+ if (Left->Previous && Left->Previous->is(tok::greater) &&
+ Left->Previous->MatchingParen &&
+ Left->Previous->MatchingParen->is(TT_TemplateOpener))
+Left = Left->Previous->MatchingParen;
+
+ // Check for `[...]`.
+ return Left->Previous && Left->Previous->is(tok::r_square) &&
+ Left->Previous->MatchingParen &&
+ Left->Previous->MatchingParen->is(TT_LambdaLSquare);
+}
+
/// A parser that gathers additional information about tokens.
///
/// The \c TokenAnnotator tries to match parenthesis and square brakets and
@@ -191,9 +206,7 @@
Left->Previous->is(TT_JsTypeColon)) {
// let x: (SomeType);
Contexts.back().IsExpression = false;
-} else if (Left->Previous && Left->Previous->is(tok::r_square) &&
- Left->Previous->MatchingParen &&
- Left->Previous->MatchingParen->is(TT_LambdaLSquare)) {
+} else if (isLambdaParameterList(Left)) {
// This is a parameter list of a lambda expression.
Contexts.back().IsExpression = false;
} else if (Line.InPPDirective &&
Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -12923,6 +12923,10 @@
" return 1; //\n"
"};");
+ // Lambdas with explicit template argument lists.
+ verifyFormat(
+ "auto L = [] class T, class U>(T &) {};\n");
+
// Multiple lambdas in the same parentheses change indentation rules. These
// lambdas are forced to start on new lines.
verifyFormat("SomeFunction(\n"
@@ -12940,8 +12944,8 @@
"},\n"
"1);\n");
- // A multi-line lambda passed as arg1 forces arg0 to be pushed out, just like the arg0
- // case above.
+ // A multi-line lambda passed as arg1 forces arg0 to be pushed out, just like
+ // the arg0 case above.
auto Style = getGoogleStyle();
Style.BinPackArguments = false;
verifyFormat("SomeFunction(\n"
Index: clang/lib/Format/UnwrappedLineParser.cpp
[PATCH] D67246: clang-format: Add support for formatting lambdas with explicit template parameters.
krasimir accepted this revision. krasimir added a comment. This revision is now accepted and ready to land. This looks good with the FIXME. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67246/new/ https://reviews.llvm.org/D67246 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D67246: clang-format: Add support for formatting lambdas with explicit template parameters.
thakis added a comment. Do you think this should land with the comment FIXME for now? It improves formatting of this language feature when that heuristic is not used, and changing the heuristic is independent of the rest of this patch. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67246/new/ https://reviews.llvm.org/D67246 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D67246: clang-format: Add support for formatting lambdas with explicit template parameters.
thakis updated this revision to Diff 219589.
thakis added a comment.
fix nits
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D67246/new/
https://reviews.llvm.org/D67246
Files:
clang/lib/Format/TokenAnnotator.cpp
clang/lib/Format/UnwrappedLineParser.cpp
clang/unittests/Format/FormatTest.cpp
Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -12888,6 +12888,10 @@
" return 1; //\n"
"};");
+ // Lambdas with explicit template argument lists.
+ verifyFormat(
+ "auto L = [] class T, class U>(T &) {};\n");
+
// Multiple lambdas in the same parentheses change indentation rules. These
// lambdas are forced to start on new lines.
verifyFormat("SomeFunction(\n"
@@ -12905,8 +12909,8 @@
"},\n"
"1);\n");
- // A multi-line lambda passed as arg1 forces arg0 to be pushed out, just
like the arg0
- // case above.
+ // A multi-line lambda passed as arg1 forces arg0 to be pushed out, just like
+ // the arg0 case above.
auto Style = getGoogleStyle();
Style.BinPackArguments = false;
verifyFormat("SomeFunction(\n"
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -1440,8 +1440,11 @@
case tok::identifier:
case tok::numeric_constant:
case tok::coloncolon:
+case tok::kw_class:
case tok::kw_mutable:
case tok::kw_noexcept:
+case tok::kw_template:
+case tok::kw_typename:
nextToken();
break;
// Specialization of a template with an integer parameter can contain
@@ -1455,6 +1458,9 @@
// followed by an `a->b` expression, such as:
// ([obj func:arg] + a->b)
// Otherwise the code below would parse as a lambda.
+//
+// FIXME: This heuristic is incorrect for C++20 generic lambdas with
+// explicit template lists: [](U &){}
case tok::plus:
case tok::minus:
case tok::exclaim:
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -40,6 +40,21 @@
return Tok.Tok.getIdentifierInfo() != nullptr;
}
+/// With `Left` being '(', check if we're at either `[...](` or
+/// `[...]<...>(`, where the [ opens a lambda capture list.
+static bool isLambdaParameterList(const FormatToken *Left) {
+ // Skip <...> if present.
+ if (Left->Previous && Left->Previous->is(tok::greater) &&
+ Left->Previous->MatchingParen &&
+ Left->Previous->MatchingParen->is(TT_TemplateOpener))
+Left = Left->Previous->MatchingParen;
+
+ // Check for `[...]`.
+ return Left->Previous && Left->Previous->is(tok::r_square) &&
+ Left->Previous->MatchingParen &&
+ Left->Previous->MatchingParen->is(TT_LambdaLSquare);
+}
+
/// A parser that gathers additional information about tokens.
///
/// The \c TokenAnnotator tries to match parenthesis and square brakets and
@@ -191,9 +206,7 @@
Left->Previous->is(TT_JsTypeColon)) {
// let x: (SomeType);
Contexts.back().IsExpression = false;
-} else if (Left->Previous && Left->Previous->is(tok::r_square) &&
- Left->Previous->MatchingParen &&
- Left->Previous->MatchingParen->is(TT_LambdaLSquare)) {
+} else if (isLambdaParameterList(Left)) {
// This is a parameter list of a lambda expression.
Contexts.back().IsExpression = false;
} else if (Line.InPPDirective &&
Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -12888,6 +12888,10 @@
" return 1; //\n"
"};");
+ // Lambdas with explicit template argument lists.
+ verifyFormat(
+ "auto L = [] class T, class U>(T &) {};\n");
+
// Multiple lambdas in the same parentheses change indentation rules. These
// lambdas are forced to start on new lines.
verifyFormat("SomeFunction(\n"
@@ -12905,8 +12909,8 @@
"},\n"
"1);\n");
- // A multi-line lambda passed as arg1 forces arg0 to be pushed out, just like the arg0
- // case above.
+ // A multi-line lambda passed as arg1 forces arg0 to be pushed out, just like
+ // the arg0 case above.
auto Style = getGoogleStyle();
Style.BinPackArguments = false;
verifyFormat("SomeFunction(\n"
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -1440,8 +1440,11 @@
case tok::identifier:
case
[PATCH] D67246: clang-format: Add support for formatting lambdas with explicit template parameters.
thakis marked 3 inline comments as done.
thakis added a comment.
Thanks for the thorough review! Indeed, this still gets `[](A &){}();` wrong, for the reason you mention.
Comment at: clang/lib/Format/TokenAnnotator.cpp:50
+ // Skip <...> if present.
+ if (Left->Previous && Left->Previous->is(tok::greater) &&
+ Left->Previous->MatchingParen &&
krasimir wrote:
> The first `Left->Previous` check is unnecessary here, following the previous
> `if`.
I removed the previous `if`, it makes the function more symmetric.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D67246/new/
https://reviews.llvm.org/D67246
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D67246: clang-format: Add support for formatting lambdas with explicit template parameters.
krasimir added a comment.
I'll need some more time to investigate the implications of this with respect
to Objective-C disambiguation stuff.
Specifically, this may interact with funny ways with the heuristic outlined in
(old) UnwrappedLineParser.cpp line 1453:
// In a C++ lambda a template type can only occur after an arrow. We use
// this as an heuristic to distinguish between Objective-C expressions
// followed by an `a->b` expression, such as:
// ([obj func:arg] + a->b)
At least we'll have to update this comment accordingly, otherwise it will be
confusing.
Comment at: clang/lib/Format/TokenAnnotator.cpp:44
+/// With `Left` being '(', check if we're at either `[...](` or
+/// `...]<...>(`.
+static bool isLambdaParameterList(const FormatToken *Left) {
nit: add a `[` in the second case, like `[...]<...>(`. Also I'd suggest
extending this sentence with: ", where the `[` opens a lambda capture list",
because we explicitly check the `[` for that and to disambiguate between the
case this is interested in and Objective-C method calls followed by
function-invocation style call, like in `[obj meth]()`.
Comment at: clang/lib/Format/TokenAnnotator.cpp:50
+ // Skip <...> if present.
+ if (Left->Previous && Left->Previous->is(tok::greater) &&
+ Left->Previous->MatchingParen &&
The first `Left->Previous` check is unnecessary here, following the previous
`if`.
Comment at: clang/lib/Format/TokenAnnotator.cpp:55
+
+ // Check for `[...](`.
+ return Left->Previous && Left->Previous->is(tok::r_square) &&
nit: consider replacing with
```
Check for `[...]`
```
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D67246/new/
https://reviews.llvm.org/D67246
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D67246: clang-format: Add support for formatting lambdas with explicit template parameters.
thakis created this revision.
thakis added a reviewer: krasimir.
Ports r359967 to clang-format.
https://reviews.llvm.org/D67246
Files:
clang/lib/Format/TokenAnnotator.cpp
clang/lib/Format/UnwrappedLineParser.cpp
clang/unittests/Format/FormatTest.cpp
Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -12888,6 +12888,10 @@
" return 1; //\n"
"};");
+ // Lambdas with explicit template argument lists.
+ verifyFormat(
+ "auto L = [] class T, class U>(T &) {};\n");
+
// Multiple lambdas in the same parentheses change indentation rules. These
// lambdas are forced to start on new lines.
verifyFormat("SomeFunction(\n"
@@ -12905,8 +12909,8 @@
"},\n"
"1);\n");
- // A multi-line lambda passed as arg1 forces arg0 to be pushed out, just
like the arg0
- // case above.
+ // A multi-line lambda passed as arg1 forces arg0 to be pushed out, just like
+ // the arg0 case above.
auto Style = getGoogleStyle();
Style.BinPackArguments = false;
verifyFormat("SomeFunction(\n"
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -1440,8 +1440,11 @@
case tok::identifier:
case tok::numeric_constant:
case tok::coloncolon:
+case tok::kw_class:
case tok::kw_mutable:
case tok::kw_noexcept:
+case tok::kw_template:
+case tok::kw_typename:
nextToken();
break;
// Specialization of a template with an integer parameter can contain
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -40,6 +40,24 @@
return Tok.Tok.getIdentifierInfo() != nullptr;
}
+/// With `Left` being '(', check if we're at either `[...](` or
+/// `...]<...>(`.
+static bool isLambdaParameterList(const FormatToken *Left) {
+ if (!Left->Previous)
+return false;
+
+ // Skip <...> if present.
+ if (Left->Previous && Left->Previous->is(tok::greater) &&
+ Left->Previous->MatchingParen &&
+ Left->Previous->MatchingParen->is(TT_TemplateOpener))
+Left = Left->Previous->MatchingParen;
+
+ // Check for `[...](`.
+ return Left->Previous && Left->Previous->is(tok::r_square) &&
+ Left->Previous->MatchingParen &&
+ Left->Previous->MatchingParen->is(TT_LambdaLSquare);
+}
+
/// A parser that gathers additional information about tokens.
///
/// The \c TokenAnnotator tries to match parenthesis and square brakets and
@@ -191,9 +209,7 @@
Left->Previous->is(TT_JsTypeColon)) {
// let x: (SomeType);
Contexts.back().IsExpression = false;
-} else if (Left->Previous && Left->Previous->is(tok::r_square) &&
- Left->Previous->MatchingParen &&
- Left->Previous->MatchingParen->is(TT_LambdaLSquare)) {
+} else if (isLambdaParameterList(Left)) {
// This is a parameter list of a lambda expression.
Contexts.back().IsExpression = false;
} else if (Line.InPPDirective &&
Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -12888,6 +12888,10 @@
" return 1; //\n"
"};");
+ // Lambdas with explicit template argument lists.
+ verifyFormat(
+ "auto L = [] class T, class U>(T &) {};\n");
+
// Multiple lambdas in the same parentheses change indentation rules. These
// lambdas are forced to start on new lines.
verifyFormat("SomeFunction(\n"
@@ -12905,8 +12909,8 @@
"},\n"
"1);\n");
- // A multi-line lambda passed as arg1 forces arg0 to be pushed out, just like the arg0
- // case above.
+ // A multi-line lambda passed as arg1 forces arg0 to be pushed out, just like
+ // the arg0 case above.
auto Style = getGoogleStyle();
Style.BinPackArguments = false;
verifyFormat("SomeFunction(\n"
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -1440,8 +1440,11 @@
case tok::identifier:
case tok::numeric_constant:
case tok::coloncolon:
+case tok::kw_class:
case tok::kw_mutable:
case tok::kw_noexcept:
+case tok::kw_template:
+case tok::kw_typename:
nextToken();
break;
// Specialization of a template with an integer parameter can contain
Index: clang/lib/Format/TokenAnnotator.cpp
