llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang-format
Author: Ben Dunkin (bdunkin)
<details>
<summary>Changes</summary>
This change adds an `IgnoreTrailingCommentLength: bool` suboption to the
`AlignConsecutiveXXX` options. It makes those options ignore trailing comments
when calculating whether to include a line in the consecutive group based on
its length. The default value is `false` to make it opt in.
To motivation is to allow more lines to be aligned if alignment is considered
more important than keeping trailing comments under the line length in these
cases, as my organization does.
Example, given this input:
```cpp
struct Test {
int aLongVariableName;
int b; // This is a really long trailing comment that will exceed the line
length if moved
LongTypeName c;
};
```
With a column limit of 80 and without the new option produces:
```cpp
struct Test {
int aLongVariableName;
int b; // This is a really long trailing comment that will exceed the line
// length if moved
LongTypeName c;
};
```
with the new option is produces:
```cpp
struct Test {
int aLongVariableName;
int b; // This is a really long trailing comment that will exceed
the line
// length if moved
LongTypeName c;
};
```
Attribution Note - I have been authorized to contribute this change on behalf
of my company: ArenaNet LLC
---
Patch is 32.91 KiB, truncated to 20.00 KiB below, full version:
https://github.com/llvm/llvm-project/pull/190668.diff
6 Files Affected:
- (modified) clang/docs/ClangFormatStyleOptions.rst (+126)
- (modified) clang/include/clang/Format/Format.h (+19-1)
- (modified) clang/lib/Format/Format.cpp (+12-5)
- (modified) clang/lib/Format/WhitespaceManager.cpp (+15-4)
- (modified) clang/unittests/Format/AlignmentTest.cpp (+223)
- (modified) clang/unittests/Format/ConfigParseTest.cpp (+36-35)
``````````diff
diff --git a/clang/docs/ClangFormatStyleOptions.rst
b/clang/docs/ClangFormatStyleOptions.rst
index 54c085dff2840..02c716e0bfa4e 100644
--- a/clang/docs/ClangFormatStyleOptions.rst
+++ b/clang/docs/ClangFormatStyleOptions.rst
@@ -422,6 +422,24 @@ the configuration (without a prefix: ``Auto``).
a = 2;
bbb >>= 2;
+ * ``bool IgnoreTrailingCommentLength`` Whether to consider trailing comments
going over the column limit a
+ cause to separate alignment groups.
+
+ .. code-block:: c++
+
+ true:
+ /* column limit -> | */
+ int a = 3; // a long trailing comment
+ int b = 4; // short comment 1
+ AReallyLongTypeName c = 5; // short comment 2
+ short comment 2
+
+ false:
+ /* column limit -> | */
+ int a = 3; // a long trailing comment
+ int b = 4; // short comment 1
+ AReallyLongTypeName c = 5; // short comment 2
+
.. _AlignConsecutiveBitFields:
@@ -580,6 +598,24 @@ the configuration (without a prefix: ``Auto``).
a = 2;
bbb >>= 2;
+ * ``bool IgnoreTrailingCommentLength`` Whether to consider trailing comments
going over the column limit a
+ cause to separate alignment groups.
+
+ .. code-block:: c++
+
+ true:
+ /* column limit -> | */
+ int a = 3; // a long trailing comment
+ int b = 4; // short comment 1
+ AReallyLongTypeName c = 5; // short comment 2
+ short comment 2
+
+ false:
+ /* column limit -> | */
+ int a = 3; // a long trailing comment
+ int b = 4; // short comment 1
+ AReallyLongTypeName c = 5; // short comment 2
+
.. _AlignConsecutiveDeclarations:
@@ -738,6 +774,24 @@ the configuration (without a prefix: ``Auto``).
a = 2;
bbb >>= 2;
+ * ``bool IgnoreTrailingCommentLength`` Whether to consider trailing comments
going over the column limit a
+ cause to separate alignment groups.
+
+ .. code-block:: c++
+
+ true:
+ /* column limit -> | */
+ int a = 3; // a long trailing comment
+ int b = 4; // short comment 1
+ AReallyLongTypeName c = 5; // short comment 2
+ short comment 2
+
+ false:
+ /* column limit -> | */
+ int a = 3; // a long trailing comment
+ int b = 4; // short comment 1
+ AReallyLongTypeName c = 5; // short comment 2
+
.. _AlignConsecutiveMacros:
@@ -897,6 +951,24 @@ the configuration (without a prefix: ``Auto``).
a = 2;
bbb >>= 2;
+ * ``bool IgnoreTrailingCommentLength`` Whether to consider trailing comments
going over the column limit a
+ cause to separate alignment groups.
+
+ .. code-block:: c++
+
+ true:
+ /* column limit -> | */
+ int a = 3; // a long trailing comment
+ int b = 4; // short comment 1
+ AReallyLongTypeName c = 5; // short comment 2
+ short comment 2
+
+ false:
+ /* column limit -> | */
+ int a = 3; // a long trailing comment
+ int b = 4; // short comment 1
+ AReallyLongTypeName c = 5; // short comment 2
+
.. _AlignConsecutiveShortCaseStatements:
@@ -1175,6 +1247,24 @@ the configuration (without a prefix: ``Auto``).
a = 2;
bbb >>= 2;
+ * ``bool IgnoreTrailingCommentLength`` Whether to consider trailing comments
going over the column limit a
+ cause to separate alignment groups.
+
+ .. code-block:: c++
+
+ true:
+ /* column limit -> | */
+ int a = 3; // a long trailing comment
+ int b = 4; // short comment 1
+ AReallyLongTypeName c = 5; // short comment 2
+ short comment 2
+
+ false:
+ /* column limit -> | */
+ int a = 3; // a long trailing comment
+ int b = 4; // short comment 1
+ AReallyLongTypeName c = 5; // short comment 2
+
.. _AlignConsecutiveTableGenCondOperatorColons:
@@ -1331,6 +1421,24 @@ the configuration (without a prefix: ``Auto``).
a = 2;
bbb >>= 2;
+ * ``bool IgnoreTrailingCommentLength`` Whether to consider trailing comments
going over the column limit a
+ cause to separate alignment groups.
+
+ .. code-block:: c++
+
+ true:
+ /* column limit -> | */
+ int a = 3; // a long trailing comment
+ int b = 4; // short comment 1
+ AReallyLongTypeName c = 5; // short comment 2
+ short comment 2
+
+ false:
+ /* column limit -> | */
+ int a = 3; // a long trailing comment
+ int b = 4; // short comment 1
+ AReallyLongTypeName c = 5; // short comment 2
+
.. _AlignConsecutiveTableGenDefinitionColons:
@@ -1487,6 +1595,24 @@ the configuration (without a prefix: ``Auto``).
a = 2;
bbb >>= 2;
+ * ``bool IgnoreTrailingCommentLength`` Whether to consider trailing comments
going over the column limit a
+ cause to separate alignment groups.
+
+ .. code-block:: c++
+
+ true:
+ /* column limit -> | */
+ int a = 3; // a long trailing comment
+ int b = 4; // short comment 1
+ AReallyLongTypeName c = 5; // short comment 2
+ short comment 2
+
+ false:
+ /* column limit -> | */
+ int a = 3; // a long trailing comment
+ int b = 4; // short comment 1
+ AReallyLongTypeName c = 5; // short comment 2
+
.. _AlignEscapedNewlines:
diff --git a/clang/include/clang/Format/Format.h
b/clang/include/clang/Format/Format.h
index 48ce5aa2bdfa1..98e29d1feb814 100644
--- a/clang/include/clang/Format/Format.h
+++ b/clang/include/clang/Format/Format.h
@@ -255,13 +255,31 @@ struct FormatStyle {
/// bbb >>= 2;
/// \endcode
bool PadOperators;
+ /// Whether to consider trailing comments going over the column limit a
+ /// cause to separate alignment groups.
+ /// \code
+ /// true:
+ /// /* column limit -> | */
+ /// int a = 3; // a long trailing comment
+ /// int b = 4; // short comment 1
+ /// AReallyLongTypeName c = 5; // short comment 2
+ /// short comment 2
+ ///
+ /// false:
+ /// /* column limit -> | */
+ /// int a = 3; // a long trailing comment
+ /// int b = 4; // short comment 1
+ /// AReallyLongTypeName c = 5; // short comment 2
+ /// \endcode
+ bool IgnoreTrailingCommentLength;
bool operator==(const AlignConsecutiveStyle &R) const {
return Enabled == R.Enabled && AcrossEmptyLines == R.AcrossEmptyLines &&
AcrossComments == R.AcrossComments &&
AlignCompound == R.AlignCompound &&
AlignFunctionDeclarations == R.AlignFunctionDeclarations &&
AlignFunctionPointers == R.AlignFunctionPointers &&
- PadOperators == R.PadOperators;
+ PadOperators == R.PadOperators &&
+ IgnoreTrailingCommentLength == R.IgnoreTrailingCommentLength;
}
bool operator!=(const AlignConsecutiveStyle &R) const {
return !(*this == R);
diff --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp
index 42190604b3881..e2d7ee939dc65 100644
--- a/clang/lib/Format/Format.cpp
+++ b/clang/lib/Format/Format.cpp
@@ -62,25 +62,29 @@ template <> struct
MappingTraits<FormatStyle::AlignConsecutiveStyle> {
{/*Enabled=*/true, /*AcrossEmptyLines=*/false,
/*AcrossComments=*/false, /*AlignCompound=*/false,
/*AlignFunctionDeclarations=*/true,
- /*AlignFunctionPointers=*/false, /*PadOperators=*/true}));
+ /*AlignFunctionPointers=*/false, /*PadOperators=*/true,
+ /*IgnoreTrailingCommentLength=*/false}));
IO.enumCase(Value, "AcrossEmptyLines",
FormatStyle::AlignConsecutiveStyle(
{/*Enabled=*/true, /*AcrossEmptyLines=*/true,
/*AcrossComments=*/false, /*AlignCompound=*/false,
/*AlignFunctionDeclarations=*/true,
- /*AlignFunctionPointers=*/false, /*PadOperators=*/true}));
+ /*AlignFunctionPointers=*/false, /*PadOperators=*/true,
+ /*IgnoreTrailingCommentLength=*/false}));
IO.enumCase(Value, "AcrossComments",
FormatStyle::AlignConsecutiveStyle(
{/*Enabled=*/true, /*AcrossEmptyLines=*/false,
/*AcrossComments=*/true, /*AlignCompound=*/false,
/*AlignFunctionDeclarations=*/true,
- /*AlignFunctionPointers=*/false, /*PadOperators=*/true}));
+ /*AlignFunctionPointers=*/false, /*PadOperators=*/true,
+ /*IgnoreTrailingCommentLength=*/false}));
IO.enumCase(Value, "AcrossEmptyLinesAndComments",
FormatStyle::AlignConsecutiveStyle(
{/*Enabled=*/true, /*AcrossEmptyLines=*/true,
/*AcrossComments=*/true, /*AlignCompound=*/false,
/*AlignFunctionDeclarations=*/true,
- /*AlignFunctionPointers=*/false, /*PadOperators=*/true}));
+ /*AlignFunctionPointers=*/false, /*PadOperators=*/true,
+ /*IgnoreTrailingCommentLength=*/false}));
// For backward compatibility.
IO.enumCase(Value, "true",
@@ -88,7 +92,8 @@ template <> struct
MappingTraits<FormatStyle::AlignConsecutiveStyle> {
{/*Enabled=*/true, /*AcrossEmptyLines=*/false,
/*AcrossComments=*/false, /*AlignCompound=*/false,
/*AlignFunctionDeclarations=*/true,
- /*AlignFunctionPointers=*/false, /*PadOperators=*/true}));
+ /*AlignFunctionPointers=*/false, /*PadOperators=*/true,
+ /*IgnoreTrailingCommentLength=*/false}));
IO.enumCase(Value, "false", FormatStyle::AlignConsecutiveStyle{});
}
@@ -101,6 +106,8 @@ template <> struct
MappingTraits<FormatStyle::AlignConsecutiveStyle> {
Value.AlignFunctionDeclarations);
IO.mapOptional("AlignFunctionPointers", Value.AlignFunctionPointers);
IO.mapOptional("PadOperators", Value.PadOperators);
+ IO.mapOptional("IgnoreTrailingCommentLength",
+ Value.IgnoreTrailingCommentLength);
}
};
diff --git a/clang/lib/Format/WhitespaceManager.cpp
b/clang/lib/Format/WhitespaceManager.cpp
index 93f354a9f7256..459fe44d2ed6a 100644
--- a/clang/lib/Format/WhitespaceManager.cpp
+++ b/clang/lib/Format/WhitespaceManager.cpp
@@ -326,10 +326,12 @@ IncrementChangeSpaces(unsigned Start, int Delta,
// Column - The tokens indexed in Matches are moved to this column.
// RightJustify - Whether it is the token's right end or left end that gets
// moved to that column.
+// ForceAlignTrailingComments - whether trailing comments should be aligned
+// even if they have newlines before them
static void
AlignTokenSequence(const FormatStyle &Style, unsigned Start, unsigned End,
unsigned Column, bool RightJustify,
- ArrayRef<unsigned> Matches,
+ bool ForceAlignTrailingComments, ArrayRef<unsigned> Matches,
SmallVector<WhitespaceManager::Change, 16> &Changes) {
unsigned OriginalMatchColumn = 0;
int Shift = 0;
@@ -382,8 +384,12 @@ AlignTokenSequence(const FormatStyle &Style, unsigned
Start, unsigned End,
(CurrentChange.indentAndNestingLevel() == ScopeStack[0] &&
CurrentChange.IndentedFromColumn >= OriginalMatchColumn));
- if (CurrentChange.NewlinesBefore > 0 && !InsideNestedScope)
+ bool IsForceAlignedTrailingComment =
+ ForceAlignTrailingComments && CurrentChange.IsTrailingComment;
+ if (CurrentChange.NewlinesBefore > 0 && !InsideNestedScope &&
+ !IsForceAlignedTrailingComment) {
Shift = 0;
+ }
// If this is the first matching token to be aligned, remember by how many
// spaces it has to be shifted, so the rest of the changes on the line are
@@ -403,7 +409,7 @@ AlignTokenSequence(const FormatStyle &Style, unsigned
Start, unsigned End,
// not in a scope that should not move.
if ((!Matches.empty() && Matches[0] == i) ||
(ScopeStack.size() == 1u && CurrentChange.NewlinesBefore > 0 &&
- InsideNestedScope)) {
+ (InsideNestedScope || IsForceAlignedTrailingComment))) {
CurrentChange.IndentedFromColumn += Shift;
IncrementChangeSpaces(i, Shift, Changes);
}
@@ -545,7 +551,8 @@ static unsigned AlignTokens(const FormatStyle &Style, F
&&Matches,
auto AlignCurrentSequence = [&] {
if (StartOfSequence > 0 && StartOfSequence < EndOfSequence) {
AlignTokenSequence(Style, StartOfSequence, EndOfSequence,
- WidthLeft + WidthAnchor, RightJustify, MatchedIndices,
+ WidthLeft + WidthAnchor, RightJustify,
+ ACS.IgnoreTrailingCommentLength, MatchedIndices,
Changes);
}
WidthLeft = 0;
@@ -681,6 +688,10 @@ static unsigned AlignTokens(const FormatStyle &Style, F
&&Matches,
MatchingParenToEncounter || Changes[J].IsAligned);
++J) {
const auto &Change = Changes[J];
+
+ if (ACS.IgnoreTrailingCommentLength && Change.IsTrailingComment)
+ continue;
+
const auto *Tok = Change.Tok;
if (Tok->MatchingParen) {
diff --git a/clang/unittests/Format/AlignmentTest.cpp
b/clang/unittests/Format/AlignmentTest.cpp
index e3a3435424914..d7bb4d8a13a79 100644
--- a/clang/unittests/Format/AlignmentTest.cpp
+++ b/clang/unittests/Format/AlignmentTest.cpp
@@ -1913,6 +1913,229 @@ TEST_F(AlignmentTest, ConsecutiveDeclarations) {
"void f2(void);\n"
"size_t f3(void);",
Alignment);
+
+ Style = getLLVMStyleWithColumns(47);
+ Style.AlignConsecutiveDeclarations.Enabled = true;
+ verifyFormat("int a = 3; // a long trailing comment\n"
+ "int b = 4; // short comment 1\n"
+ "AReallyLongTypeName c = 5; // short comment 2\n",
+ Style);
+
+ Style.AlignConsecutiveDeclarations.IgnoreTrailingCommentLength = true;
+ verifyFormat("int a = 3; // a long trailing comment\n"
+ "int b = 4; // short comment 1\n"
+ "AReallyLongTypeName c = 5; // short comment 2\n",
+ Style);
+
+ Style = getLLVMStyleWithColumns(120);
+ Style.IndentWidth = 4;
+ Style.AlignTrailingComments.Kind = FormatStyle::TCAS_Always;
+ Style.AlignConsecutiveDeclarations.Enabled = true;
+ Style.AlignConsecutiveDeclarations.AcrossComments = true;
+ verifyFormat(
+ "struct ATestingTypeABCDEF {\n"
+ " uint8 member1ABCDEFGHI; // Lorem ipsum dolor sit amet, consectetur "
+ "adipiscing elit. Aliquam ullamcorper in leo in\n"
+ " // lobortis. In ac fermentum enim. Duis "
+ "ullamcorper ac lectus suscipit interdum. Phasellus\n"
+ " // id condimentum lorem. Ut ac convallis "
+ "ligula. Maecenas quis interdum nisl. In vel\n"
+ " // molestie eros. Curabitur lobortis dui "
+ "vitae purus interdum\n"
+ " uint8 member2ABCDEFGHIJKLMNOPQRSTUVWXYZABCDEFGHIJ; // Lorem ipsum "
+ "dolor sit amet, consectetur adipiscing\n"
+ " int8 member3ABCDEFGHIJ;\n"
+ " int8 member4ABCDEFGHIJKLMNOPQRSTU; // Lorem ipsum dolor sit amet, "
+ "consectetur amet\n"
+ " uint8 aaa;\n"
+ " int8 bbbbb;\n"
+ " uint8 member7ABCDEFGHIJKLMNO;\n"
+ " ALongTypeNameV1ABCDE *member8ABCDEFGHIJKLMNO;\n"
+ " int8 member9ABCDEF; // Lorem ipsum dolor sit amet, consectetur "
+ "adipiscing elit. Aliquam ullamcorper in leo\n"
+ " uint8 member10ABCDEFGHIJKLMNO; // Lorem ipsum dolor sit amet, "
+ "consectetur elit\n"
+ " uint8 member11ABC;\n"
+ " uint8 member12ABCDEF;\n"
+ " uint8 member13ABC;\n"
+ " int8 member14ABCDEFGH;\n"
+ " AnotherLongTypeNameV1ABCDEFGH *member15ABCDEFGH;\n"
+ " int8 member16ABCDEF;\n"
+ " int8 member17;\n"
+ " int8 member18;\n"
+ " AThirdReallyLongTypeNameV1ABCDEFGHIJK member19ABCDEFGHIJK; //
"
+ "Lorem ipsum dolor sit amet, consectetur elit\n"
+ " TheLastReallyLongTypeNameV1ABCDEFGHIJKLM member20ABCDEFGHIJKLMN; //
"
+ "Lorem ipsum dolor sit amet, consectetur elit\n"
+ " uint8 member21ABCDEFGHIJKLM; //
"
+ "Lorem ipsum dolor sit amet, "
+ "consectetur\n"
+ "};\n",
+ "struct ATestingTypeABCDEF {\n"
+ " uint8 member1ABCDEFGHI; // Lorem ipsum dolor sit amet, consectetur "
+ "adipiscing elit. Aliquam ullamcorper in leo in lobortis. In ac "
+ "fermentum enim. Duis ullamcorper ac lectus suscipit interdum. Phasellus
"
+ "id condimentum lorem. Ut ac convallis ligula. Maecenas quis interdum "
+ "nisl. In vel molestie eros. Curabitur lobortis dui vitae purus "
+ "interdum\n"
+ " uint8 member2ABCDEFGHIJKLMNOPQRSTUVWXYZABCDEFGHIJ; // Lorem ipsum "
+ "dolor sit amet, consectetur adipiscing\n"
+ " int8 member3ABCDEFGHIJ;\n"
+ " int8 member4ABCDEFGHIJKLMNOPQRSTU; // Lorem ipsum dolor sit amet, "
+ "consectetur amet\n"
+ " uint8 aaa;\n"
+ " int8 bbbbb;\n"
+ " uint8 member7ABCDEFGHIJKLMNO;\n"
+ " ALongTypeNameV1ABCDE * member8ABCDEFGHIJKLMNO;\n"
+ " int8 member9ABCDEF; // Lorem ipsum dolor sit amet, consectetur "
+ "adipiscing elit. Aliquam ullamcorper in leo\n"
+ " uint8 member10ABCDEFGHIJKLMNO; // Lorem ipsum dolor sit amet, "
+ "consectetur elit\n"
+ " uint8 member11ABC;\n"
+ " uint8 member12ABCDEF;\n"
+ " uint8 member13ABC;\n"
+ " int8 member14ABCDEFGH;\n"
+ " AnotherLongTypeNameV1ABCDEFGH * member15ABCDEFGH;\n"
+ " int8 member16ABCDEF;\n"
+ " int8 member17;\n"
+ " int8 member18;\n"
+ " AThirdReallyLongTypeNameV1ABCDEFGHIJK member19ABCDEFGHIJK; // Lorem
"
+ "ipsum dolor sit amet, consectetur elit\n"
+ " TheLastReallyLongTypeNameV1ABCDEFGHIJKLM member20ABCDEFGHIJKLMN; //
"
+ "Lorem ipsum dolor sit amet, consectetur elit\n"
+ " uint8 member21ABCDEFGHIJKLM; // Lorem ipsum dolor sit amet, "
+ "consectetur\n"
+ "};\n",
+ Style);
+
+ Style.AlignConsecutiveDeclarations.IgnoreTrailingCommentLength = true;
+ // This one tests proper alignment of the reflowed comment
+ verifyFormat(
+ "struct ATestingTypeABCDEF {\n"
+ " uint8 member1ABCDEFGHI; // Lorem
"
+ "ipsum dolor sit amet, consectetur adipiscing elit. Aliquam ullamcorper "
+ "in leo in\n"
+ " // "
+ "lobortis. In ac fermentum enim. Duis ullamcorper ac lectus suscipit "
+ "interdum. Phasellus\n"
+ " // id "
+ "condimentum lorem. Ut ac convallis ligula. Maecenas quis interdum nisl.
"
+ "In vel\n"
+ " // "
+ "molestie eros. Curabitur lobortis dui vitae purus interdum\n"
+ " uint8 "
+ "member2ABCDEFGHIJKLMNOPQRSTUVWXYZABCDEFGHIJ; // Lorem ipsum dolor sit "
+ "amet, consectetur adipiscing\n"
+ " int8 member3ABCDEFGHIJ;\n"
+ " int8 "
+ "member4ABCDEFGHIJKLMNOPQRSTU; // Lorem ipsum dolor sit amet, "
+ "consectetur amet\n"
+ " uint8 aaa;\n"...
[truncated]
``````````
</details>
https://github.com/llvm/llvm-project/pull/190668
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
