[clang] [clang-format] Allow open brace with trailing comment (no line break) (PR #89956)
via cfe-commits
cfe-commits at lists.llvm.org
Fri May 31 10:28:19 PDT 2024
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang-format
Author: None (GertyP)
<details>
<summary>Changes</summary>
clang-format always forcibly line-breaks between an initialiser open brace and a trailing comment, which separates the comment from the very thing it might directly relate to (the braced object). E.g. for things like nested braced blocks of a multi-dimensional array initialisation, keeping trailing comments next to their open braces can be a desirable formatting style. I.e. the comment is about the object associated with the brace, not the the first element in its initialisation.
This relates to the issue mentioned in #<!-- -->85083 and does just about the minimum to fix only this case, through use of `AlignTrailingComments: Kind: Leave`.
It could be argued that this 'keep trailing comment next to open brace' behaviour could also apply under other trailing comments 'kinds' or even that this may not perfectly fit within the scope of `AlignTrailingComments` and instead might warrant an entirely separate style option. I do wonder if this could be enough of an edge-case (i.e. no one's asked for this control so far and no other controls seem to explicitly define any initialiser-open-brace-with-trailing-comment behaviour) that this fairly minimal change is acceptable under the umbrella of the `AlignTrailingComments : Kind : Leave` control or whether opinions are that this behaviour actually really belongs under some other control or even an entirely new option. If the latter, I think this would then require quite a few more changes, which is why I'm initially proposing this under the former, simpler approach but I'm interested in thoughts on this.
---
Full diff: https://github.com/llvm/llvm-project/pull/89956.diff
3 Files Affected:
- (modified) clang/lib/Format/ContinuationIndenter.cpp (+2-1)
- (modified) clang/lib/Format/TokenAnnotator.cpp (+24-4)
- (modified) clang/unittests/Format/FormatTestComments.cpp (+31)
``````````diff
diff --git a/clang/lib/Format/ContinuationIndenter.cpp b/clang/lib/Format/ContinuationIndenter.cpp
index ad0e2c3c620c3..dbc02aec0e0fc 100644
--- a/clang/lib/Format/ContinuationIndenter.cpp
+++ b/clang/lib/Format/ContinuationIndenter.cpp
@@ -831,7 +831,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))) {
+ (Style.AlignTrailingComments.Kind != FormatStyle::TCAS_Leave &&
+ Previous.isOneOf(BK_BracedInit, TT_VerilogMultiLineListLParen)))) {
CurrentState.Indent = State.Column + Spaces;
CurrentState.IsAligned = true;
}
diff --git a/clang/lib/Format/TokenAnnotator.cpp b/clang/lib/Format/TokenAnnotator.cpp
index cdfb4256e41d9..2b13954df89ca 100644
--- a/clang/lib/Format/TokenAnnotator.cpp
+++ b/clang/lib/Format/TokenAnnotator.cpp
@@ -5588,6 +5588,23 @@ bool TokenAnnotator::mustBreakBefore(const AnnotatedLine &Line,
}
if (BeforeClosingBrace && (BeforeClosingBrace->is(tok::comma) ||
BeforeClosingBrace->isTrailingComment())) {
+ // Except let's not always break after the open brace/parenthesis/array.
+ // A style option allowing keeping trailing comments together with the
+ // open token can be desirable. E.g -
+ // int a[2][2] = {
+ // { // [0][...]
+ // 0, // [0][0]
+ // 1, // [0][1]
+ // },
+ // { // [1][...]
+ // 2, // [1][0]
+ // 3, // [1][1]
+ // }
+ // };
+ if (Right.isTrailingComment() &&
+ Style.AlignTrailingComments.Kind == FormatStyle::TCAS_Leave) {
+ return false;
+ }
return true;
}
}
@@ -5997,10 +6014,13 @@ bool TokenAnnotator::canBreakBefore(const AnnotatedLine &Line,
if (Right.isTrailingComment()) {
// We rely on MustBreakBefore being set correctly here as we should not
// change the "binding" behavior of a comment.
- // The first comment in a braced lists is always interpreted as belonging to
- // the first list element. Otherwise, it should be placed outside of the
- // list.
- return Left.is(BK_BracedInit) ||
+ // The first comment in a braced lists is usually interpreted as belonging
+ // to the first list element, unless the style is to leave trailing comments
+ // alone. Otherwise, it should be placed outside of the list.
+ bool AfterBracedInitAndBrakeable =
+ Left.is(BK_BracedInit) &&
+ Style.AlignTrailingComments.Kind != FormatStyle::TCAS_Leave;
+ return AfterBracedInitAndBrakeable ||
(Left.is(TT_CtorInitializerColon) && Right.NewlinesBefore > 0 &&
Style.BreakConstructorInitializers == FormatStyle::BCIS_AfterColon);
}
diff --git a/clang/unittests/Format/FormatTestComments.cpp b/clang/unittests/Format/FormatTestComments.cpp
index d2baace6a7d80..04ca2f92a9579 100644
--- a/clang/unittests/Format/FormatTestComments.cpp
+++ b/clang/unittests/Format/FormatTestComments.cpp
@@ -1429,6 +1429,37 @@ TEST_F(FormatTestComments, CommentsInStaticInitializers) {
" 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, // comment\n"
" 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, // comment\n"
" 0x00, 0x00, 0x00, 0x00}; // comment");
+
+ // The usual 'open brace with trailing comment' behaviour is to forcibly
+ // break the trailing comment onto onto a new line -
+ FormatStyle Style = getLLVMStyle();
+ Style.AlignAfterOpenBracket = FormatStyle::BAS_BlockIndent;
+ StringRef Input = "int a[2][2] = {\n"
+ " { // a\n"
+ " 0, // x\n"
+ " 1,\n"
+ " },\n"
+ " {\n"
+ " 2,\n"
+ " 3, // y\n"
+ " }\n"
+ "};";
+ verifyFormat("int a[2][2] = {\n"
+ " {\n"
+ " // a\n"
+ " 0, // x\n"
+ " 1,\n"
+ " },\n"
+ " {\n"
+ " 2,\n"
+ " 3, // y\n"
+ " }\n"
+ "};",
+ Input, Style);
+ // But, especially for nested, multi-dimensional initialization, allowing
+ // open braces with trailing comments can be desirable -
+ Style.AlignTrailingComments.Kind = FormatStyle::TCAS_Leave;
+ verifyNoChange(Input, Style);
}
TEST_F(FormatTestComments, LineCommentsAfterRightBrace) {
``````````
</details>
https://github.com/llvm/llvm-project/pull/89956
More information about the cfe-commits
mailing list