[clang] 37c2233 - [clang-format] [PR50702] Lamdba processing does not respect AfterClass and AfterNamespace
via cfe-commits
cfe-commits at lists.llvm.org
Sat Jun 26 05:34:37 PDT 2021
Author: mydeveloperday
Date: 2021-06-26T13:34:07+01:00
New Revision: 37c2233097ac44697b87228d86eef1fce10ea5c1
URL: https://github.com/llvm/llvm-project/commit/37c2233097ac44697b87228d86eef1fce10ea5c1
DIFF: https://github.com/llvm/llvm-project/commit/37c2233097ac44697b87228d86eef1fce10ea5c1.diff
LOG: [clang-format] [PR50702] Lamdba processing does not respect AfterClass and AfterNamespace
https://bugs.llvm.org/show_bug.cgi?id=50702
I believe {D44609} may be too aggressive with brace wrapping rules which doesn't always apply to Lamdbas
The introduction of BeforeLambdaBody and AllowShortLambdasOnASingleLine has impact on brace handling on other block types, which I suspect we didn't see before as people may not be using the BeforeLambdaBody style
>From what I can tell this can be seen by the unit test I change as its not honouring the orginal LLVM brace wrapping style for the `Fct()` function
I added a unit test from PR50702 and have removed some of the code (which has zero impact on the unit test, which kind of suggests its unnecessary), some additional attempt has been made to try and ensure we'll only break on what is actually a LamdbaLBrace
Reviewed By: HazardyKnusperkeks
Differential Revision: https://reviews.llvm.org/D104222
Added:
Modified:
clang/lib/Format/TokenAnnotator.cpp
clang/unittests/Format/FormatTest.cpp
Removed:
################################################################################
diff --git a/clang/lib/Format/TokenAnnotator.cpp b/clang/lib/Format/TokenAnnotator.cpp
index bfa49de7d74f..5f973950b8d7 100644
--- a/clang/lib/Format/TokenAnnotator.cpp
+++ b/clang/lib/Format/TokenAnnotator.cpp
@@ -3579,42 +3579,11 @@ isItAnEmptyLambdaAllowed(const FormatToken &Tok,
return Tok.Children.empty() && ShortLambdaOption != FormatStyle::SLS_None;
}
-static bool
-isItAInlineLambdaAllowed(const FormatToken &Tok,
- FormatStyle::ShortLambdaStyle ShortLambdaOption) {
- return (ShortLambdaOption == FormatStyle::SLS_Inline &&
- IsFunctionArgument(Tok)) ||
- (ShortLambdaOption == FormatStyle::SLS_All);
-}
-
-static bool isOneChildWithoutMustBreakBefore(const FormatToken &Tok) {
- if (Tok.Children.size() != 1)
- return false;
- FormatToken *curElt = Tok.Children[0]->First;
- while (curElt) {
- if (curElt->MustBreakBefore)
- return false;
- curElt = curElt->Next;
- }
- return true;
-}
static bool isAllmanLambdaBrace(const FormatToken &Tok) {
return (Tok.is(tok::l_brace) && Tok.is(BK_Block) &&
!Tok.isOneOf(TT_ObjCBlockLBrace, TT_DictLiteral));
}
-static bool isAllmanBraceIncludedBreakableLambda(
- const FormatToken &Tok, FormatStyle::ShortLambdaStyle ShortLambdaOption) {
- if (!isAllmanLambdaBrace(Tok))
- return false;
-
- if (isItAnEmptyLambdaAllowed(Tok, ShortLambdaOption))
- return false;
-
- return !isItAInlineLambdaAllowed(Tok, ShortLambdaOption) ||
- !isOneChildWithoutMustBreakBefore(Tok);
-}
-
bool TokenAnnotator::mustBreakBefore(const AnnotatedLine &Line,
const FormatToken &Right) {
const FormatToken &Left = *Right.Previous;
@@ -3776,13 +3745,6 @@ bool TokenAnnotator::mustBreakBefore(const AnnotatedLine &Line,
if (Right.is(TT_InlineASMBrace))
return Right.HasUnescapedNewline;
- auto ShortLambdaOption = Style.AllowShortLambdasOnASingleLine;
- if (Style.BraceWrapping.BeforeLambdaBody &&
- (isAllmanBraceIncludedBreakableLambda(Left, ShortLambdaOption) ||
- isAllmanBraceIncludedBreakableLambda(Right, ShortLambdaOption))) {
- return true;
- }
-
if (isAllmanBrace(Left) || isAllmanBrace(Right))
return (Line.startsWith(tok::kw_enum) && Style.BraceWrapping.AfterEnum) ||
(Line.startsWith(tok::kw_typedef, tok::kw_enum) &&
@@ -3805,6 +3767,11 @@ bool TokenAnnotator::mustBreakBefore(const AnnotatedLine &Line,
return true;
}
+ if (Style.BraceWrapping.BeforeLambdaBody && Right.is(TT_LambdaLBrace) &&
+ Left.isOneOf(tok::star, tok::amp, tok::ampamp, TT_TemplateCloser)) {
+ return true;
+ }
+
// Put multiple Java annotation on a new line.
if ((Style.Language == FormatStyle::LK_Java ||
Style.Language == FormatStyle::LK_JavaScript) &&
@@ -4209,7 +4176,7 @@ bool TokenAnnotator::canBreakBefore(const AnnotatedLine &Line,
return false;
auto ShortLambdaOption = Style.AllowShortLambdasOnASingleLine;
- if (Style.BraceWrapping.BeforeLambdaBody) {
+ if (Style.BraceWrapping.BeforeLambdaBody && Right.is(TT_LambdaLBrace)) {
if (isAllmanLambdaBrace(Left))
return !isItAnEmptyLambdaAllowed(Left, ShortLambdaOption);
if (isAllmanLambdaBrace(Right))
@@ -4221,7 +4188,6 @@ bool TokenAnnotator::canBreakBefore(const AnnotatedLine &Line,
Right.isMemberAccess() ||
Right.isOneOf(TT_TrailingReturnArrow, TT_LambdaArrow, tok::lessless,
tok::colon, tok::l_square, tok::at) ||
- (Style.BraceWrapping.BeforeLambdaBody && Right.is(TT_LambdaLBrace)) ||
(Left.is(tok::r_paren) &&
Right.isOneOf(tok::identifier, tok::kw_const)) ||
(Left.is(tok::l_paren) && !Right.is(tok::r_paren)) ||
diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp
index e084d06b2aa6..e5fa17270cdf 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -19596,8 +19596,7 @@ TEST_F(FormatTest, FormatsLambdas) {
" });\n"
" });",
LLVMWithBeforeLambdaBody);
- verifyFormat("void Fct()\n"
- "{\n"
+ verifyFormat("void Fct() {\n"
" return {[]()\n"
" {\n"
" return 17;\n"
@@ -19802,6 +19801,35 @@ TEST_F(FormatTest, FormatsLambdas) {
" });",
LLVMWithBeforeLambdaBody);
+ LLVMWithBeforeLambdaBody.AllowShortLambdasOnASingleLine =
+ FormatStyle::ShortLambdaStyle::SLS_None;
+
+ verifyFormat("auto select = [this]() -> const Library::Object *\n"
+ "{\n"
+ " return MyAssignment::SelectFromList(this);\n"
+ "};\n",
+ LLVMWithBeforeLambdaBody);
+
+ verifyFormat("auto select = [this]() -> const Library::Object &\n"
+ "{\n"
+ " return MyAssignment::SelectFromList(this);\n"
+ "};\n",
+ LLVMWithBeforeLambdaBody);
+
+ verifyFormat("auto select = [this]() -> std::unique_ptr<Object>\n"
+ "{\n"
+ " return MyAssignment::SelectFromList(this);\n"
+ "};\n",
+ LLVMWithBeforeLambdaBody);
+
+ verifyFormat("namespace test {\n"
+ "class Test {\n"
+ "public:\n"
+ " Test() = default;\n"
+ "};\n"
+ "} // namespace test",
+ LLVMWithBeforeLambdaBody);
+
// Lambdas with
diff erent indentation styles.
Style = getLLVMStyleWithColumns(100);
EXPECT_EQ("SomeResult doSomething(SomeObject promise) {\n"
More information about the cfe-commits
mailing list