https://github.com/gedare updated
https://github.com/llvm/llvm-project/pull/93140
>From 6f35c68cff7381453a0bd6a491fee1db6784f42d Mon Sep 17 00:00:00 2001
From: Gedare Bloom <ged...@rtems.org>
Date: Tue, 21 May 2024 22:21:59 -0600
Subject: [PATCH 1/5] [clang-format] Improve BlockIndent at ColumnLimit
Fixes #55731
Fixes #73584
The reported formatting problems were related to ignoring deep nesting
of "simple" functions (causing #54808) and to allowing the trailing
annotation to become separated from the closing parens, which allowed a
break to occur between the closing parens and the trailing annotation.
The fix for the nesting of "simple" functions is to detect them more
carefully. "Simple" was defined in a comment as being a single
non-expression argument. I tried to stay as close to the original intent
of the implementation while fixing the various bad formatting reports.
In the process of fixing these bugs, some latent bugs were discovered
related to how JavaScript Template Strings are handled. Those are also
fixed here.
---
clang/lib/Format/ContinuationIndenter.cpp | 47 +++++++++++++++++++++--
clang/lib/Format/TokenAnnotator.cpp | 6 +++
clang/unittests/Format/FormatTest.cpp | 22 +++++++++++
3 files changed, 72 insertions(+), 3 deletions(-)
diff --git a/clang/lib/Format/ContinuationIndenter.cpp
b/clang/lib/Format/ContinuationIndenter.cpp
index b07360425ca6e..516cfd8b1af61 100644
--- a/clang/lib/Format/ContinuationIndenter.cpp
+++ b/clang/lib/Format/ContinuationIndenter.cpp
@@ -803,6 +803,46 @@ void ContinuationIndenter::addTokenOnCurrentLine(LineState
&State, bool DryRun,
return !Tok.Previous->isOneOf(TT_CastRParen, tok::kw_for, tok::kw_while,
tok::kw_switch);
};
+ // Detecting functions is brittle. It would be better if we could annotate
+ // the LParen type of functions/calls.
+ const auto IsFunctionDeclParen = [&](const FormatToken &Tok) {
+ return Tok.is(tok::l_paren) && Tok.Previous &&
+ (Tok.Previous->is(TT_FunctionDeclarationName) ||
+ (Tok.Previous->Previous &&
+ Tok.Previous->Previous->is(tok::coloncolon) &&
+ Tok.Previous->Previous->Previous &&
+
Tok.Previous->Previous->Previous->is(TT_FunctionDeclarationName)));
+ };
+ const auto IsFunctionCallParen = [&](const FormatToken &Tok) {
+ return Tok.is(tok::l_paren) && Tok.ParameterCount > 0 && Tok.Previous &&
+ Tok.Previous->is(tok::identifier);
+ };
+ const auto IsInTemplateString = [&](const FormatToken &Tok) {
+ if (!Style.isJavaScript())
+ return false;
+ for (const FormatToken *Prev = &Tok; Prev; Prev = Prev->Previous) {
+ if (Prev->is(TT_TemplateString) && Prev->opensScope())
+ return true;
+ if (Prev->is(TT_TemplateString) && Prev->closesScope())
+ break;
+ }
+ return false;
+ };
+ const auto IsNotSimpleFunction = [&](const FormatToken &Tok) {
+ const auto *Previous = Tok.Previous;
+ const auto *Next = Tok.Next;
+ if (Tok.FakeLParens.size() > 0 && Tok.FakeLParens.back() > prec::Unknown)
+ return true;
+ if (Previous &&
+ (IsFunctionDeclParen(*Previous) || IsFunctionCallParen(*Previous))) {
+ if (!IsOpeningBracket(Tok) && Next && !Next->isMemberAccess() &&
+ !IsInTemplateString(Tok) && !IsFunctionDeclParen(*Next) &&
+ !IsFunctionCallParen(*Next)) {
+ return true;
+ }
+ }
+ return false;
+ };
if ((Style.AlignAfterOpenBracket == FormatStyle::BAS_AlwaysBreak ||
Style.AlignAfterOpenBracket == FormatStyle::BAS_BlockIndent) &&
IsOpeningBracket(Previous) && State.Column > getNewLineColumn(State) &&
@@ -813,10 +853,10 @@ void
ContinuationIndenter::addTokenOnCurrentLine(LineState &State, bool DryRun,
// caaaaaaaaaaaall(
// caaaaaaaaaaaall(
// caaaaaaaaaaaaaaaaaaaaaaall(aaaaaaaaaaaaaa,
aaaaaaaaa))));
- Current.FakeLParens.size() > 0 &&
- Current.FakeLParens.back() > prec::Unknown) {
+ IsNotSimpleFunction(Current)) {
CurrentState.NoLineBreak = true;
}
+
if (Previous.is(TT_TemplateString) && Previous.opensScope())
CurrentState.NoLineBreak = true;
@@ -831,7 +871,8 @@ void ContinuationIndenter::addTokenOnCurrentLine(LineState
&State, bool DryRun,
Previous.isNot(TT_TableGenDAGArgOpenerToBreak) &&
!(Current.MacroParent && Previous.MacroParent) &&
(Current.isNot(TT_LineComment) ||
- Previous.isOneOf(BK_BracedInit, TT_VerilogMultiLineListLParen))) {
+ Previous.isOneOf(BK_BracedInit, TT_VerilogMultiLineListLParen)) &&
+ !IsInTemplateString(Current)) {
CurrentState.Indent = State.Column + Spaces;
CurrentState.IsAligned = true;
}
diff --git a/clang/lib/Format/TokenAnnotator.cpp
b/clang/lib/Format/TokenAnnotator.cpp
index 1fd309afd697e..fe8a126e25476 100644
--- a/clang/lib/Format/TokenAnnotator.cpp
+++ b/clang/lib/Format/TokenAnnotator.cpp
@@ -6195,6 +6195,12 @@ bool TokenAnnotator::canBreakBefore(const AnnotatedLine
&Line,
return !(Previous && (Previous->is(tok::kw_for) || Previous->isIf()));
}
+ if (Left.isOneOf(tok::r_paren, TT_TrailingAnnotation) &&
+ Right.is(TT_TrailingAnnotation) &&
+ Style.AlignAfterOpenBracket == FormatStyle::BAS_BlockIndent) {
+ return false;
+ }
+
// Allow breaking after a trailing annotation, e.g. after a method
// declaration.
if (Left.is(TT_TrailingAnnotation)) {
diff --git a/clang/unittests/Format/FormatTest.cpp
b/clang/unittests/Format/FormatTest.cpp
index 283843ad7ab47..46acc1dc813eb 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -9337,6 +9337,28 @@ TEST_F(FormatTest, AlignsAfterOpenBracket) {
" aaaaaaaaaaaaaaaa\n"
");",
Style);
+ verifyFormat(
+ "bool aaaaaaaaaaaaaaaaaaaaaaaaaaa(\n"
+ " const bool &aaaaaaaaa, const void *aaaaaaaaaa\n"
+ ") const {\n"
+ " return true;\n"
+ "}",
+ Style);
+ verifyFormat(
+ "bool aaaaaaaaaaaaaaaaaaaaaaaa(\n"
+ " const bool &aaaaaaaaaa, const void *aaaaaaaaaa\n"
+ ") const;",
+ Style);
+ verifyFormat(
+ "void aaaaaaaaa(\n"
+ " int aaaaaa, int bbbbbb, int cccccc, int dddddddddd\n"
+ ") const noexcept -> std::vector<of_very_long_type>;",
+ Style);
+ verifyFormat(
+ "aaaaaaaaaaaaaaaaaaa(\n"
+ " \"a aaaaaaa aaaaaaaaaaaaaaaaa aaaaaaaaaaaaaaa aaaaaaaaaaaaa\"\n"
+ ");",
+ Style);
}
TEST_F(FormatTest, ParenthesesAndOperandAlignment) {
>From ae059541605114803882d0979ee42b18b4c06b41 Mon Sep 17 00:00:00 2001
From: Gedare Bloom <ged...@rtems.org>
Date: Thu, 23 May 2024 00:05:33 -0600
Subject: [PATCH 2/5] FormatTest.cpp: fix format
---
clang/unittests/Format/FormatTest.cpp | 31 ++++++++++++---------------
1 file changed, 14 insertions(+), 17 deletions(-)
diff --git a/clang/unittests/Format/FormatTest.cpp
b/clang/unittests/Format/FormatTest.cpp
index 46acc1dc813eb..668c3eba9d2cd 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -9337,23 +9337,20 @@ TEST_F(FormatTest, AlignsAfterOpenBracket) {
" aaaaaaaaaaaaaaaa\n"
");",
Style);
- verifyFormat(
- "bool aaaaaaaaaaaaaaaaaaaaaaaaaaa(\n"
- " const bool &aaaaaaaaa, const void *aaaaaaaaaa\n"
- ") const {\n"
- " return true;\n"
- "}",
- Style);
- verifyFormat(
- "bool aaaaaaaaaaaaaaaaaaaaaaaa(\n"
- " const bool &aaaaaaaaaa, const void *aaaaaaaaaa\n"
- ") const;",
- Style);
- verifyFormat(
- "void aaaaaaaaa(\n"
- " int aaaaaa, int bbbbbb, int cccccc, int dddddddddd\n"
- ") const noexcept -> std::vector<of_very_long_type>;",
- Style);
+ verifyFormat("bool aaaaaaaaaaaaaaaaaaaaaaaaaaa(\n"
+ " const bool &aaaaaaaaa, const void *aaaaaaaaaa\n"
+ ") const {\n"
+ " return true;\n"
+ "}",
+ Style);
+ verifyFormat("bool aaaaaaaaaaaaaaaaaaaaaaaa(\n"
+ " const bool &aaaaaaaaaa, const void *aaaaaaaaaa\n"
+ ") const;",
+ Style);
+ verifyFormat("void aaaaaaaaa(\n"
+ " int aaaaaa, int bbbbbb, int cccccc, int dddddddddd\n"
+ ") const noexcept -> std::vector<of_very_long_type>;",
+ Style);
verifyFormat(
"aaaaaaaaaaaaaaaaaaa(\n"
" \"a aaaaaaa aaaaaaaaaaaaaaaaa aaaaaaaaaaaaaaa aaaaaaaaaaaaa\"\n"
>From 562def1936201ee6c9a4628fa15704c5213b716b Mon Sep 17 00:00:00 2001
From: Gedare Bloom <ged...@rtems.org>
Date: Fri, 31 May 2024 15:03:04 -0600
Subject: [PATCH 3/5] handle lambdas
---
clang/lib/Format/ContinuationIndenter.cpp | 26 +++++++++++++++++------
clang/unittests/Format/FormatTest.cpp | 7 ++++++
2 files changed, 27 insertions(+), 6 deletions(-)
diff --git a/clang/lib/Format/ContinuationIndenter.cpp
b/clang/lib/Format/ContinuationIndenter.cpp
index 516cfd8b1af61..84bb968348da7 100644
--- a/clang/lib/Format/ContinuationIndenter.cpp
+++ b/clang/lib/Format/ContinuationIndenter.cpp
@@ -813,6 +813,20 @@ void ContinuationIndenter::addTokenOnCurrentLine(LineState
&State, bool DryRun,
Tok.Previous->Previous->Previous &&
Tok.Previous->Previous->Previous->is(TT_FunctionDeclarationName)));
};
+ const auto IsLambdaParameterList = [](const FormatToken *Left) {
+ // adapted from TokenAnnotator.cpp:isLambdaParameterList()
+ // 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);
+ };
const auto IsFunctionCallParen = [&](const FormatToken &Tok) {
return Tok.is(tok::l_paren) && Tok.ParameterCount > 0 && Tok.Previous &&
Tok.Previous->is(tok::identifier);
@@ -828,18 +842,18 @@ void
ContinuationIndenter::addTokenOnCurrentLine(LineState &State, bool DryRun,
}
return false;
};
+ // Identifies simple (no expression) one-argument function calls.
const auto IsNotSimpleFunction = [&](const FormatToken &Tok) {
const auto *Previous = Tok.Previous;
const auto *Next = Tok.Next;
if (Tok.FakeLParens.size() > 0 && Tok.FakeLParens.back() > prec::Unknown)
return true;
if (Previous &&
- (IsFunctionDeclParen(*Previous) || IsFunctionCallParen(*Previous))) {
- if (!IsOpeningBracket(Tok) && Next && !Next->isMemberAccess() &&
- !IsInTemplateString(Tok) && !IsFunctionDeclParen(*Next) &&
- !IsFunctionCallParen(*Next)) {
- return true;
- }
+ (IsFunctionDeclParen(*Previous) || IsFunctionCallParen(*Previous) ||
+ IsLambdaParameterList(Previous))) {
+ return !IsOpeningBracket(Tok) && Next && !Next->isMemberAccess() &&
+ !IsInTemplateString(Tok) && !IsFunctionDeclParen(*Next) &&
+ !IsFunctionCallParen(*Next);
}
return false;
};
diff --git a/clang/unittests/Format/FormatTest.cpp
b/clang/unittests/Format/FormatTest.cpp
index 668c3eba9d2cd..5117cfa90fb2d 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -9356,6 +9356,13 @@ TEST_F(FormatTest, AlignsAfterOpenBracket) {
" \"a aaaaaaa aaaaaaaaaaaaaaaaa aaaaaaaaaaaaaaa aaaaaaaaaaaaa\"\n"
");",
Style);
+ verifyFormat(
+ "auto lambda =\n"
+ " [&b](\n"
+ " auto "
+ "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\n"
+ " ) {};",
+ Style);
}
TEST_F(FormatTest, ParenthesesAndOperandAlignment) {
>From e367b78a0e4bbc9f7dfeeef551b3ad67d3e27873 Mon Sep 17 00:00:00 2001
From: Gedare Bloom <ged...@rtems.org>
Date: Thu, 20 Jun 2024 12:49:11 -0600
Subject: [PATCH 4/5] Address review comments
---
clang/lib/Format/ContinuationIndenter.cpp | 25 ++++++++++-------------
1 file changed, 11 insertions(+), 14 deletions(-)
diff --git a/clang/lib/Format/ContinuationIndenter.cpp
b/clang/lib/Format/ContinuationIndenter.cpp
index 84bb968348da7..62437e20d5c09 100644
--- a/clang/lib/Format/ContinuationIndenter.cpp
+++ b/clang/lib/Format/ContinuationIndenter.cpp
@@ -805,29 +805,26 @@ void
ContinuationIndenter::addTokenOnCurrentLine(LineState &State, bool DryRun,
};
// Detecting functions is brittle. It would be better if we could annotate
// the LParen type of functions/calls.
- const auto IsFunctionDeclParen = [&](const FormatToken &Tok) {
+ auto IsFunctionDeclParen = [&](const FormatToken &Tok) {
return Tok.is(tok::l_paren) && Tok.Previous &&
(Tok.Previous->is(TT_FunctionDeclarationName) ||
- (Tok.Previous->Previous &&
- Tok.Previous->Previous->is(tok::coloncolon) &&
- Tok.Previous->Previous->Previous &&
-
Tok.Previous->Previous->Previous->is(TT_FunctionDeclarationName)));
+ Tok.endsSequence(TT_FunctionDeclarationName, tok::coloncolon));
};
- const auto IsLambdaParameterList = [](const FormatToken *Left) {
+ auto IsLambdaParameterList = [](const FormatToken *Tok) {
// adapted from TokenAnnotator.cpp:isLambdaParameterList()
// Skip <...> if present.
- if (Left->Previous && Left->Previous->is(tok::greater) &&
- Left->Previous->MatchingParen &&
- Left->Previous->MatchingParen->is(TT_TemplateOpener)) {
- Left = Left->Previous->MatchingParen;
+ if (Tok->Previous && Tok->Previous->is(tok::greater) &&
+ Tok->Previous->MatchingParen &&
+ Tok->Previous->MatchingParen->is(TT_TemplateOpener)) {
+ Tok = Tok->Previous->MatchingParen;
}
// Check for `[...]`.
- return Left->Previous && Left->Previous->is(tok::r_square) &&
- Left->Previous->MatchingParen &&
- Left->Previous->MatchingParen->is(TT_LambdaLSquare);
+ return Tok->Previous && Tok->Previous->is(tok::r_square) &&
+ Tok->Previous->MatchingParen &&
+ Tok->Previous->MatchingParen->is(TT_LambdaLSquare);
};
- const auto IsFunctionCallParen = [&](const FormatToken &Tok) {
+ auto IsFunctionCallParen = [&](const FormatToken &Tok) {
return Tok.is(tok::l_paren) && Tok.ParameterCount > 0 && Tok.Previous &&
Tok.Previous->is(tok::identifier);
};
>From 5a5077beb221e42b82d2d7a409c94647ae12da92 Mon Sep 17 00:00:00 2001
From: Gedare Bloom <ged...@rtems.org>
Date: Wed, 10 Jul 2024 14:13:47 -0600
Subject: [PATCH 5/5] Use TT_FunctionDeclarationLParen
---
clang/lib/Format/ContinuationIndenter.cpp | 14 ++++----------
clang/unittests/Format/FormatTest.cpp | 2 +-
2 files changed, 5 insertions(+), 11 deletions(-)
diff --git a/clang/lib/Format/ContinuationIndenter.cpp
b/clang/lib/Format/ContinuationIndenter.cpp
index 62437e20d5c09..01730e6838c8b 100644
--- a/clang/lib/Format/ContinuationIndenter.cpp
+++ b/clang/lib/Format/ContinuationIndenter.cpp
@@ -803,13 +803,6 @@ void ContinuationIndenter::addTokenOnCurrentLine(LineState
&State, bool DryRun,
return !Tok.Previous->isOneOf(TT_CastRParen, tok::kw_for, tok::kw_while,
tok::kw_switch);
};
- // Detecting functions is brittle. It would be better if we could annotate
- // the LParen type of functions/calls.
- auto IsFunctionDeclParen = [&](const FormatToken &Tok) {
- return Tok.is(tok::l_paren) && Tok.Previous &&
- (Tok.Previous->is(TT_FunctionDeclarationName) ||
- Tok.endsSequence(TT_FunctionDeclarationName, tok::coloncolon));
- };
auto IsLambdaParameterList = [](const FormatToken *Tok) {
// adapted from TokenAnnotator.cpp:isLambdaParameterList()
// Skip <...> if present.
@@ -846,10 +839,11 @@ void
ContinuationIndenter::addTokenOnCurrentLine(LineState &State, bool DryRun,
if (Tok.FakeLParens.size() > 0 && Tok.FakeLParens.back() > prec::Unknown)
return true;
if (Previous &&
- (IsFunctionDeclParen(*Previous) || IsFunctionCallParen(*Previous) ||
- IsLambdaParameterList(Previous))) {
+ (Previous->is(TT_FunctionDeclarationLParen) ||
+ IsFunctionCallParen(*Previous) || IsLambdaParameterList(Previous))) {
return !IsOpeningBracket(Tok) && Next && !Next->isMemberAccess() &&
- !IsInTemplateString(Tok) && !IsFunctionDeclParen(*Next) &&
+ !IsInTemplateString(Tok) &&
+ !Next->is(TT_FunctionDeclarationLParen) &&
!IsFunctionCallParen(*Next);
}
return false;
diff --git a/clang/unittests/Format/FormatTest.cpp
b/clang/unittests/Format/FormatTest.cpp
index 5117cfa90fb2d..c1e4515bc074c 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -9352,7 +9352,7 @@ TEST_F(FormatTest, AlignsAfterOpenBracket) {
") const noexcept -> std::vector<of_very_long_type>;",
Style);
verifyFormat(
- "aaaaaaaaaaaaaaaaaaa(\n"
+ "x = aaaaaaaaaaaaaaa(\n"
" \"a aaaaaaa aaaaaaaaaaaaaaaaa aaaaaaaaaaaaaaa aaaaaaaaaaaaa\"\n"
");",
Style);
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
