sstwcw created this revision.
sstwcw added reviewers: curdeius, HazardyKnusperkeks.
sstwcw added projects: clang-format, clang-tools-extra.
sstwcw requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
About the column limit option in the test AlignConsecutiveDeclarations.
Previously, the test worked when the column limit was set to 0.
However, when the column limit is less than the length of a single line,
the formatter is supposed to give up aligning stuff. Previously it
aligned things due to a bug. In the function AlignTokens:
unsigned ChangeMaxColumn = Style.ColumnLimit - LineLengthAfter;
// If we are restricted by the maximum column width, end the sequence.
if (ChangeMinColumn > MaxColumn || ChangeMaxColumn < MinColumn ||
CommasBeforeLastMatch != CommasBeforeMatch) {
AlignCurrentSequence();
StartOfSequence = i;
}
`ChangeMaxColumn` would wrap around 0. `ChangeMaxColumn < MinColumn`
would fail.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D119599
Files:
clang/include/clang/Format/Format.h
clang/lib/Format/Format.cpp
clang/lib/Format/WhitespaceManager.cpp
clang/unittests/Format/FormatTest.cpp
Index: clang/unittests/Format/FormatTest.cpp
===================================================================
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -16392,6 +16392,23 @@
Alignment));
}
+TEST_F(FormatTest, AlignCompoundAssignments) {
+ FormatStyle Alignment = getLLVMStyle();
+ Alignment.AlignConsecutiveAssignments = FormatStyle::ACS_Consecutive;
+ Alignment.AlignCompoundAssignments = true;
+ verifyFormat("aa <= 5;\n"
+ "a &= 5;\n"
+ "bcd *= 5;\n"
+ "ghtyf += 5;\n"
+ "dvfvdb -= 5;\n"
+ "a /= 5;\n"
+ "vdsvsv %= 5;\n"
+ "sfdbddfbdfbb ^= 5;\n"
+ "dvsdsv |= 5;\n"
+ "int dsvvdvsdvvv = 123;",
+ Alignment);
+}
+
TEST_F(FormatTest, AlignConsecutiveAssignments) {
FormatStyle Alignment = getLLVMStyle();
Alignment.AlignConsecutiveMacros = FormatStyle::ACS_Consecutive;
@@ -16410,7 +16427,8 @@
verifyFormat("int a = method();\n"
"int oneTwoThree = 133;",
Alignment);
- verifyFormat("a &= 5;\n"
+ verifyFormat("aa <= 5;\n"
+ "a &= 5;\n"
"bcd *= 5;\n"
"ghtyf += 5;\n"
"dvfvdb -= 5;\n"
@@ -16847,9 +16865,11 @@
"double b();",
Alignment);
unsigned OldColumnLimit = Alignment.ColumnLimit;
- // We need to set ColumnLimit to zero, in order to stress nested alignments,
- // otherwise the function parameters will be re-flowed onto a single line.
- Alignment.ColumnLimit = 0;
+ // We need to set ColumnLimit to a small number, in order to stress
+ // nested alignments, otherwise the function parameters will be
+ // re-flowed onto a single line. It also needs to be wide enough so
+ // that things still get aligned.
+ Alignment.ColumnLimit = 20;
EXPECT_EQ("int a(int x,\n"
" float y);\n"
"double b(int x,\n"
@@ -19248,12 +19268,13 @@
TEST_F(FormatTest, ParsesConfigurationBools) {
FormatStyle Style = {};
Style.Language = FormatStyle::LK_Cpp;
- CHECK_PARSE_BOOL(AlignTrailingComments);
CHECK_PARSE_BOOL(AllowAllArgumentsOnNextLine);
CHECK_PARSE_BOOL(AllowAllParametersOfDeclarationOnNextLine);
+ CHECK_PARSE_BOOL(AlignCompoundAssignments);
CHECK_PARSE_BOOL(AllowShortCaseLabelsOnASingleLine);
CHECK_PARSE_BOOL(AllowShortEnumsOnASingleLine);
CHECK_PARSE_BOOL(AllowShortLoopsOnASingleLine);
+ CHECK_PARSE_BOOL(AlignTrailingComments);
CHECK_PARSE_BOOL(BinPackArguments);
CHECK_PARSE_BOOL(BinPackParameters);
CHECK_PARSE_BOOL(BreakAfterJavaFieldAnnotations);
Index: clang/lib/Format/WhitespaceManager.cpp
===================================================================
--- clang/lib/Format/WhitespaceManager.cpp
+++ clang/lib/Format/WhitespaceManager.cpp
@@ -270,7 +270,7 @@
template <typename F>
static void
AlignTokenSequence(const FormatStyle &Style, unsigned Start, unsigned End,
- unsigned Column, F &&Matches,
+ unsigned Column, unsigned AnchorWidth, F &&Matches,
SmallVector<WhitespaceManager::Change, 16> &Changes) {
bool FoundMatchOnLine = false;
int Shift = 0;
@@ -329,7 +329,9 @@
// shifted by the same amount
if (!FoundMatchOnLine && !SkipMatchCheck && Matches(Changes[i])) {
FoundMatchOnLine = true;
- Shift = Column - Changes[i].StartOfTokenColumn;
+ Shift = Column +
+ (AnchorWidth ? AnchorWidth - Changes[i].TokenLength : 0) -
+ Changes[i].StartOfTokenColumn;
Changes[i].Spaces += Shift;
// FIXME: This is a workaround that should be removed when we fix
// http://llvm.org/PR53699. An assertion later below verifies this.
@@ -460,9 +462,20 @@
static unsigned AlignTokens(
const FormatStyle &Style, F &&Matches,
SmallVector<WhitespaceManager::Change, 16> &Changes, unsigned StartAt,
- const FormatStyle::AlignConsecutiveStyle &ACS = FormatStyle::ACS_None) {
- unsigned MinColumn = 0;
- unsigned MaxColumn = UINT_MAX;
+ const FormatStyle::AlignConsecutiveStyle &ACS = FormatStyle::ACS_None,
+ bool RightJustify = false) {
+ // Widths of the aligned text.
+ // The part to the left of the anchor. For right-justified assignment
+ // operators, this includes the initial space before the sign but not
+ // padding to right-justify the sign.
+ unsigned WidthLeft = 0;
+ // We may align operators of different lengths. Currently the only
+ // case is like `=` and `+=`. We want the `=` signs to align. So we
+ // right-justify the operators.
+ unsigned WidthAnchor = 0;
+ // Width to the right of the anchor when RightJustify is true. Plus
+ // width of the anchor when RightJustify is false.
+ unsigned WidthRight = 0;
// Line number of the start and the end of the current token sequence.
unsigned StartOfSequence = 0;
@@ -495,10 +508,11 @@
// containing any matching token to be aligned and located after such token.
auto AlignCurrentSequence = [&] {
if (StartOfSequence > 0 && StartOfSequence < EndOfSequence)
- AlignTokenSequence(Style, StartOfSequence, EndOfSequence, MinColumn,
- Matches, Changes);
- MinColumn = 0;
- MaxColumn = UINT_MAX;
+ AlignTokenSequence(Style, StartOfSequence, EndOfSequence, WidthLeft,
+ WidthAnchor, Matches, Changes);
+ WidthLeft = 0;
+ WidthAnchor = 0;
+ WidthRight = 0;
StartOfSequence = 0;
EndOfSequence = 0;
};
@@ -563,29 +577,39 @@
if (StartOfSequence == 0)
StartOfSequence = i;
- unsigned ChangeMinColumn = Changes[i].StartOfTokenColumn;
- int LineLengthAfter = Changes[i].TokenLength;
+ unsigned ChangeWidthLeft = Changes[i].StartOfTokenColumn;
+ unsigned ChangeWidthAnchor = 0;
+ unsigned ChangeWidthRight = 0;
+ if (RightJustify)
+ ChangeWidthAnchor = Changes[i].TokenLength;
+ else
+ ChangeWidthRight = Changes[i].TokenLength;
for (unsigned j = i + 1; j != e && Changes[j].NewlinesBefore == 0; ++j) {
- LineLengthAfter += Changes[j].Spaces;
+ ChangeWidthRight += Changes[j].Spaces;
// Changes are generally 1:1 with the tokens, but a change could also be
// inside of a token, in which case it's counted more than once: once for
// the whitespace surrounding the token (!IsInsideToken) and once for
// each whitespace change within it (IsInsideToken).
// Therefore, changes inside of a token should only count the space.
if (!Changes[j].IsInsideToken)
- LineLengthAfter += Changes[j].TokenLength;
+ ChangeWidthRight += Changes[j].TokenLength;
}
- unsigned ChangeMaxColumn = Style.ColumnLimit - LineLengthAfter;
// If we are restricted by the maximum column width, end the sequence.
- if (ChangeMinColumn > MaxColumn || ChangeMaxColumn < MinColumn ||
- CommasBeforeLastMatch != CommasBeforeMatch) {
+ unsigned NewLeft = std::max(ChangeWidthLeft, WidthLeft);
+ unsigned NewAnchor = std::max(ChangeWidthAnchor, WidthAnchor);
+ unsigned NewRight = std::max(ChangeWidthRight, WidthRight);
+ if (Style.ColumnLimit < NewLeft + NewAnchor + NewRight) {
AlignCurrentSequence();
StartOfSequence = i;
+ WidthLeft = ChangeWidthLeft;
+ WidthAnchor = ChangeWidthAnchor;
+ WidthRight = ChangeWidthRight;
+ } else {
+ WidthLeft = NewLeft;
+ WidthAnchor = NewAnchor;
+ WidthRight = NewRight;
}
-
- MinColumn = std::max(MinColumn, ChangeMinColumn);
- MaxColumn = std::min(MaxColumn, ChangeMaxColumn);
}
EndOfSequence = i;
@@ -760,9 +784,12 @@
if (Previous && Previous->is(tok::kw_operator))
return false;
- return C.Tok->is(tok::equal);
+ return Style.AlignCompoundAssignments
+ ? C.Tok->getPrecedence() == prec::Assignment
+ : C.Tok->is(tok::equal);
},
- Changes, /*StartAt=*/0, Style.AlignConsecutiveAssignments);
+ Changes, /*StartAt=*/0, Style.AlignConsecutiveAssignments,
+ /*RightJustify=*/true);
}
void WhitespaceManager::alignConsecutiveBitFields() {
Index: clang/lib/Format/Format.cpp
===================================================================
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -600,13 +600,14 @@
IO.mapOptional("AccessModifierOffset", Style.AccessModifierOffset);
IO.mapOptional("AlignAfterOpenBracket", Style.AlignAfterOpenBracket);
IO.mapOptional("AlignArrayOfStructures", Style.AlignArrayOfStructures);
- IO.mapOptional("AlignConsecutiveMacros", Style.AlignConsecutiveMacros);
+ IO.mapOptional("AlignCompoundAssignments", Style.AlignCompoundAssignments);
IO.mapOptional("AlignConsecutiveAssignments",
Style.AlignConsecutiveAssignments);
IO.mapOptional("AlignConsecutiveBitFields",
Style.AlignConsecutiveBitFields);
IO.mapOptional("AlignConsecutiveDeclarations",
Style.AlignConsecutiveDeclarations);
+ IO.mapOptional("AlignConsecutiveMacros", Style.AlignConsecutiveMacros);
IO.mapOptional("AlignEscapedNewlines", Style.AlignEscapedNewlines);
IO.mapOptional("AlignOperands", Style.AlignOperands);
IO.mapOptional("AlignTrailingComments", Style.AlignTrailingComments);
@@ -1137,6 +1138,7 @@
LLVMStyle.AlignArrayOfStructures = FormatStyle::AIAS_None;
LLVMStyle.AlignOperands = FormatStyle::OAS_Align;
LLVMStyle.AlignTrailingComments = true;
+ LLVMStyle.AlignCompoundAssignments = false;
LLVMStyle.AlignConsecutiveAssignments = FormatStyle::ACS_None;
LLVMStyle.AlignConsecutiveBitFields = FormatStyle::ACS_None;
LLVMStyle.AlignConsecutiveDeclarations = FormatStyle::ACS_None;
Index: clang/include/clang/Format/Format.h
===================================================================
--- clang/include/clang/Format/Format.h
+++ clang/include/clang/Format/Format.h
@@ -148,6 +148,20 @@
ACS_AcrossEmptyLinesAndComments
};
+ /// When aligning assignments, whether compound assignments like
+ /// ``+=``'s are aligned along with ``=``'s.
+ ///
+ /// \code
+ /// true:
+ /// a &= 2;
+ /// bbb = 2;
+ ///
+ /// false:
+ /// a &= 2;
+ /// bbb = 2;
+ /// \endcode
+ bool AlignCompoundAssignments;
+
/// Style of aligning consecutive macro definitions.
///
/// ``Consecutive`` will result in formattings like:
@@ -3919,6 +3933,7 @@
return AccessModifierOffset == R.AccessModifierOffset &&
AlignAfterOpenBracket == R.AlignAfterOpenBracket &&
AlignArrayOfStructures == R.AlignArrayOfStructures &&
+ AlignCompoundAssignments == R.AlignCompoundAssignments &&
AlignConsecutiveAssignments == R.AlignConsecutiveAssignments &&
AlignConsecutiveBitFields == R.AlignConsecutiveBitFields &&
AlignConsecutiveDeclarations == R.AlignConsecutiveDeclarations &&
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
