diff --git a/clang/docs/ClangFormatStyleOptions.rst b/clang/docs/ClangFormatStyleOptions.rst --- a/clang/docs/ClangFormatStyleOptions.rst +++ b/clang/docs/ClangFormatStyleOptions.rst @@ -267,6 +267,24 @@ +**AlignCompoundAssignments** (``Boolean``) + When aligning assignments, whether compound assignments like + ``+=``'s are aligned along with ``=``'s. + + This option only has effect when ``AlignConsecutiveAssignments`` + is not ``None``. + + + .. code-block:: c++ + + true: + a &= 2; + bbb = 2; + + false: + a &= 2; + bbb = 2; + **AlignConsecutiveAssignments** (``AlignConsecutiveStyle``) :versionbadge:`clang-format 3.8` Style of aligning consecutive assignments. diff --git a/clang/include/clang/Format/Format.h b/clang/include/clang/Format/Format.h --- a/clang/include/clang/Format/Format.h +++ b/clang/include/clang/Format/Format.h @@ -148,6 +148,23 @@ ACS_AcrossEmptyLinesAndComments }; + /// When aligning assignments, whether compound assignments like + /// ``+=``'s are aligned along with ``=``'s. + /// + /// This option only has effect when ``AlignConsecutiveAssignments`` + /// is not ``None``. + /// + /// \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 +3936,7 @@ return AccessModifierOffset == R.AccessModifierOffset && AlignAfterOpenBracket == R.AlignAfterOpenBracket && AlignArrayOfStructures == R.AlignArrayOfStructures && + AlignCompoundAssignments == R.AlignCompoundAssignments && AlignConsecutiveAssignments == R.AlignConsecutiveAssignments && AlignConsecutiveBitFields == R.AlignConsecutiveBitFields && AlignConsecutiveDeclarations == R.AlignConsecutiveDeclarations && diff --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp --- a/clang/lib/Format/Format.cpp +++ b/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; diff --git a/clang/lib/Format/WhitespaceManager.cpp b/clang/lib/Format/WhitespaceManager.cpp --- a/clang/lib/Format/WhitespaceManager.cpp +++ b/clang/lib/Format/WhitespaceManager.cpp @@ -267,10 +267,14 @@ } // Align a single sequence of tokens, see AlignTokens below. +// Column - The token for which Matches returns true is moved to this +// column. +// RightJustify - Whether it is the token's right end or left end that +// gets moved to that column. template static void AlignTokenSequence(const FormatStyle &Style, unsigned Start, unsigned End, - unsigned Column, F &&Matches, + unsigned Column, bool RightJustify, F &&Matches, SmallVector &Changes) { bool FoundMatchOnLine = false; int Shift = 0; @@ -329,7 +333,8 @@ // shifted by the same amount if (!FoundMatchOnLine && !SkipMatchCheck && Matches(Changes[i])) { FoundMatchOnLine = true; - Shift = Column - Changes[i].StartOfTokenColumn; + Shift = Column - RightJustify * Changes[i].TokenLength - + 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. @@ -456,13 +461,29 @@ // However, the special exception is that we do NOT skip function parameters // that are split across multiple lines. See the test case in FormatTest.cpp // that mentions "split function parameter alignment" for an example of this. +// When the parameter RightJustify is true, the operator will be +// right-justified. It is used to align compound assignments like `+=` +// and `=`. template static unsigned AlignTokens( const FormatStyle &Style, F &&Matches, SmallVector &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) { + // We arrange each line in 3 parts. The operator to be aligned (the + // anchor), and text to its left and right. In the aligned text the + // width of each part will be the maximum of that over the block that + // has been aligned. + // Maximum widths of each part so far. + // The part to the left of the anchor. Including the space that exists + // on its left from the start. Not including the padding added on the + // left to right-justify the anchor. + unsigned WidthLeft = 0; + // The operator to be aligned when right-justifying it. 0 otherwise. + unsigned WidthAnchor = 0; + // Width to the right of the anchor. 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 +516,12 @@ // 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, RightJustify, Matches, + Changes); + WidthLeft = 0; + WidthAnchor = 0; + WidthRight = 0; StartOfSequence = 0; EndOfSequence = 0; }; @@ -563,29 +586,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 +793,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() { diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp --- a/clang/unittests/Format/FormatTest.cpp +++ b/clang/unittests/Format/FormatTest.cpp @@ -16392,6 +16392,122 @@ 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); + 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); + verifyFormat("a &= 5;\n" + "bcd *= 5;\n" + "ghtyf >>= 5;\n" + "dvfvdb -= 5;\n" + "a /= 5;\n" + "aa <= 5;\n" + "vdsvsv %= 5;\n" + "sfdbddfbdfbb ^= 5;\n" + "dvsdsv <<= 5;\n" + "int dsvvdvsdvvv = 123;", + Alignment); + Alignment.AlignConsecutiveAssignments = FormatStyle::ACS_Consecutive; + EXPECT_EQ("int a += 5;\n" + "int one = 1;\n" + "\n" + "int oneTwoThree = 123;\n", + format("int a += 5;\n" + "int one = 1;\n" + "\n" + "int oneTwoThree = 123;\n", + Alignment)); + EXPECT_EQ("int a += 5;\n" + "int one = 1;\n" + "//\n" + "int oneTwoThree = 123;\n", + format("int a += 5;\n" + "int one = 1;\n" + "//\n" + "int oneTwoThree = 123;\n", + Alignment)); + Alignment.AlignConsecutiveAssignments = FormatStyle::ACS_AcrossEmptyLines; + EXPECT_EQ("int a += 5;\n" + "int one = 1;\n" + "\n" + "int oneTwoThree = 123;\n", + format("int a += 5;\n" + "int one = 1;\n" + "\n" + "int oneTwoThree = 123;\n", + Alignment)); + EXPECT_EQ("int a += 5;\n" + "int one = 1;\n" + "//\n" + "int oneTwoThree = 123;\n", + format("int a += 5;\n" + "int one = 1;\n" + "//\n" + "int oneTwoThree = 123;\n", + Alignment)); + Alignment.AlignConsecutiveAssignments = FormatStyle::ACS_AcrossComments; + EXPECT_EQ("int a += 5;\n" + "int one = 1;\n" + "\n" + "int oneTwoThree = 123;\n", + format("int a += 5;\n" + "int one = 1;\n" + "\n" + "int oneTwoThree = 123;\n", + Alignment)); + EXPECT_EQ("int a += 5;\n" + "int one = 1;\n" + "//\n" + "int oneTwoThree = 123;\n", + format("int a += 5;\n" + "int one = 1;\n" + "//\n" + "int oneTwoThree = 123;\n", + Alignment)); + Alignment.AlignConsecutiveAssignments = + FormatStyle::ACS_AcrossEmptyLinesAndComments; + EXPECT_EQ("int a += 5;\n" + "int one >>= 1;\n" + "\n" + "int oneTwoThree = 123;\n", + format("int a += 5;\n" + "int one >>= 1;\n" + "\n" + "int oneTwoThree = 123;\n", + Alignment)); + EXPECT_EQ("int a += 5;\n" + "int one = 1;\n" + "//\n" + "int oneTwoThree <<= 123;\n", + format("int a += 5;\n" + "int one = 1;\n" + "//\n" + "int oneTwoThree <<= 123;\n", + Alignment)); +} + TEST_F(FormatTest, AlignConsecutiveAssignments) { FormatStyle Alignment = getLLVMStyle(); Alignment.AlignConsecutiveMacros = FormatStyle::ACS_Consecutive; @@ -16410,7 +16526,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 +16964,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,6 +19367,7 @@ TEST_F(FormatTest, ParsesConfigurationBools) { FormatStyle Style = {}; Style.Language = FormatStyle::LK_Cpp; + CHECK_PARSE_BOOL(AlignCompoundAssignments); CHECK_PARSE_BOOL(AlignTrailingComments); CHECK_PARSE_BOOL(AllowAllArgumentsOnNextLine); CHECK_PARSE_BOOL(AllowAllParametersOfDeclarationOnNextLine);