Index: lib/Format/WhitespaceManager.h =================================================================== --- lib/Format/WhitespaceManager.h +++ lib/Format/WhitespaceManager.h @@ -168,20 +168,9 @@ /// \brief Align consecutive assignments over all \c Changes. void alignConsecutiveAssignments(); - /// \brief Align consecutive assignments from change \p Start to change \p End - /// at - /// the specified \p Column. - void alignConsecutiveAssignments(unsigned Start, unsigned End, - unsigned Column); - /// \brief Align consecutive declarations over all \c Changes. void alignConsecutiveDeclarations(); - /// \brief Align consecutive declarations from change \p Start to change \p - /// End at the specified \p Column. - void alignConsecutiveDeclarations(unsigned Start, unsigned End, - unsigned Column); - /// \brief Align trailing comments over all \c Changes. void alignTrailingComments(); Index: lib/Format/WhitespaceManager.cpp =================================================================== --- lib/Format/WhitespaceManager.cpp +++ lib/Format/WhitespaceManager.cpp @@ -148,125 +148,24 @@ } } -// Walk through all of the changes and find sequences of "=" to align. To do -// so, keep track of the lines and whether or not an "=" was found on align. If -// a "=" is found on a line, extend the current sequence. If the current line -// cannot be part of a sequence, e.g. because there is an empty line before it -// or it contains non-assignments, finalize the previous sequence. -// -// FIXME: The code between assignment and declaration alignment is mostly -// duplicated and would benefit from factorization. -void WhitespaceManager::alignConsecutiveAssignments() { - if (!Style.AlignConsecutiveAssignments) - return; - - unsigned MinColumn = 0; - unsigned MaxColumn = UINT_MAX; - unsigned StartOfSequence = 0; - unsigned EndOfSequence = 0; - bool FoundAssignmentOnLine = false; - bool FoundLeftBraceOnLine = false; - bool FoundLeftParenOnLine = false; - - // Aligns a sequence of assignment tokens, on the MinColumn column. - // - // Sequences start from the first assignment token to align, and end at the - // first token of the first line that doesn't need to be aligned. - // - // We need to adjust the StartOfTokenColumn of each Change that is on a line - // containing any assignment to be aligned and located after such assignment - auto AlignSequence = [&] { - if (StartOfSequence > 0 && StartOfSequence < EndOfSequence) - alignConsecutiveAssignments(StartOfSequence, EndOfSequence, MinColumn); - MinColumn = 0; - MaxColumn = UINT_MAX; - StartOfSequence = 0; - EndOfSequence = 0; - }; - - for (unsigned i = 0, e = Changes.size(); i != e; ++i) { - if (Changes[i].NewlinesBefore != 0) { - EndOfSequence = i; - // If there is a blank line, if the last line didn't contain any - // assignment, or if we found an open brace or paren, the sequence ends - // here. - if (Changes[i].NewlinesBefore > 1 || !FoundAssignmentOnLine || - FoundLeftBraceOnLine || FoundLeftParenOnLine) { - // NB: In the latter case, the sequence should end at the beggining of - // the previous line, but it doesn't really matter as there is no - // assignment on it - AlignSequence(); - } - - FoundAssignmentOnLine = false; - FoundLeftBraceOnLine = false; - FoundLeftParenOnLine = false; - } - - // If there is more than one "=" per line, or if the "=" appears first on - // the line of if it appears last, end the sequence - if (Changes[i].Kind == tok::equal && - (FoundAssignmentOnLine || Changes[i].NewlinesBefore > 0 || - Changes[i + 1].NewlinesBefore > 0)) { - AlignSequence(); - } else if (Changes[i].Kind == tok::r_brace) { - if (!FoundLeftBraceOnLine) - AlignSequence(); - FoundLeftBraceOnLine = false; - } else if (Changes[i].Kind == tok::l_brace) { - FoundLeftBraceOnLine = true; - if (!FoundAssignmentOnLine) - AlignSequence(); - } else if (Changes[i].Kind == tok::r_paren) { - if (!FoundLeftParenOnLine) - AlignSequence(); - FoundLeftParenOnLine = false; - } else if (Changes[i].Kind == tok::l_paren) { - FoundLeftParenOnLine = true; - if (!FoundAssignmentOnLine) - AlignSequence(); - } else if (!FoundAssignmentOnLine && !FoundLeftBraceOnLine && - !FoundLeftParenOnLine && Changes[i].Kind == tok::equal) { - FoundAssignmentOnLine = true; - if (StartOfSequence == 0) - StartOfSequence = i; - - unsigned ChangeMinColumn = Changes[i].StartOfTokenColumn; - int LineLengthAfter = -Changes[i].Spaces; - for (unsigned j = i; j != e && Changes[j].NewlinesBefore == 0; ++j) - LineLengthAfter += Changes[j].Spaces + Changes[j].TokenLength; - unsigned ChangeMaxColumn = Style.ColumnLimit - LineLengthAfter; - - if (ChangeMinColumn > MaxColumn || ChangeMaxColumn < MinColumn) { - AlignSequence(); - StartOfSequence = i; - } - - MinColumn = std::max(MinColumn, ChangeMinColumn); - MaxColumn = std::min(MaxColumn, ChangeMaxColumn); - } - } - - EndOfSequence = Changes.size(); - AlignSequence(); -} - -void WhitespaceManager::alignConsecutiveAssignments(unsigned Start, - unsigned End, - unsigned Column) { - bool FoundAssignmentOnLine = false; +// Align a single sequence of tokens, see AlignTokens below. +template +static void +AlignTokenSequence(unsigned Start, unsigned End, unsigned Column, F &&Matches, + SmallVector &Changes) { + bool FoundMatchOnLine = false; int Shift = 0; for (unsigned i = Start; i != End; ++i) { if (Changes[i].NewlinesBefore > 0) { - FoundAssignmentOnLine = false; + FoundMatchOnLine = false; Shift = 0; } - // If this is the first assignment to be aligned, remember by how many + // 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 // shifted by the same amount - if (!FoundAssignmentOnLine && Changes[i].Kind == tok::equal) { - FoundAssignmentOnLine = true; + if (!FoundMatchOnLine && Matches(Changes[i])) { + FoundMatchOnLine = true; Shift = Column - Changes[i].StartOfTokenColumn; Changes[i].Spaces += Shift; } @@ -278,30 +177,51 @@ } } -// Walk through all of the changes and find sequences of declaration names to -// align. To do so, keep track of the lines and whether or not a name was found -// on align. If a name is found on a line, extend the current sequence. If the -// current line cannot be part of a sequence, e.g. because there is an empty -// line before it or it contains non-declarations, finalize the previous -// sequence. -// -// FIXME: The code between assignment and declaration alignment is mostly -// duplicated and would benefit from factorization. -void WhitespaceManager::alignConsecutiveDeclarations() { - if (!Style.AlignConsecutiveDeclarations) - return; - +// Walk through all of the changes and find sequences of matching tokens to +// align. To do so, keep track of the lines and whether or not a matching token +// was found on a line. If a matching token is found, extend the current +// sequence. If the current line cannot be part of a sequence, e.g. because +// there is an empty line before it or it contains only non-matching tokens, +// finalize the previous sequence. +template +static void AlignTokens(const FormatStyle &Style, F &&Matches, + SmallVector &Changes) { unsigned MinColumn = 0; unsigned MaxColumn = UINT_MAX; + + // Line number of the start and the end of the current token sequence. unsigned StartOfSequence = 0; unsigned EndOfSequence = 0; - bool FoundDeclarationOnLine = false; - bool FoundLeftBraceOnLine = false; - bool FoundLeftParenOnLine = false; - auto AlignSequence = [&] { + // Keep track of the nesting level of matching tokens, i.e. the number of + // surrounding (), [], or {}. We will only align a sequence of matching + // token that share the same scope depth. + // + // FIXME: This could use FormatToken::NestingLevel information, but there is + // an outstanding issue wrt the brace scopes. + unsigned NestingLevelOfLastMatch = 0; + unsigned NestingLevel = 0; + + // Keep track of the number of commas before the matching tokens, we will only + // align a sequence of matching tokens if they are preceded by the same number + // of commas. + unsigned CommasBeforeLastMatch = 0; + unsigned CommasBeforeMatch = 0; + + // Whether a matching token has been found on the current line. + bool FoundMatchOnLine = false; + + // Aligns a sequence of matching tokens, on the MinColumn column. + // + // Sequences start from the first matching token to align, and end at the + // first token of the first line that doesn't need to be aligned. + // + // We need to adjust the StartOfTokenColumn of each Change that is on a line + // containing any matching token to be aligned and located after such token. + auto AlignCurrentSequence = [&] { if (StartOfSequence > 0 && StartOfSequence < EndOfSequence) - alignConsecutiveDeclarations(StartOfSequence, EndOfSequence, MinColumn); + AlignTokenSequence(StartOfSequence, EndOfSequence, MinColumn, Matches, + Changes); MinColumn = 0; MaxColumn = UINT_MAX; StartOfSequence = 0; @@ -310,79 +230,101 @@ for (unsigned i = 0, e = Changes.size(); i != e; ++i) { if (Changes[i].NewlinesBefore != 0) { + CommasBeforeMatch = 0; EndOfSequence = i; - if (Changes[i].NewlinesBefore > 1 || !FoundDeclarationOnLine || - FoundLeftBraceOnLine || FoundLeftParenOnLine) - AlignSequence(); - FoundDeclarationOnLine = false; - FoundLeftBraceOnLine = false; - FoundLeftParenOnLine = false; + // If there is a blank line, or if the last line didn't contain any + // matching token, the sequence ends here. + if (Changes[i].NewlinesBefore > 1 || !FoundMatchOnLine) + AlignCurrentSequence(); + + FoundMatchOnLine = false; } - if (Changes[i].Kind == tok::r_brace) { - if (!FoundLeftBraceOnLine) - AlignSequence(); - FoundLeftBraceOnLine = false; - } else if (Changes[i].Kind == tok::l_brace) { - FoundLeftBraceOnLine = true; - if (!FoundDeclarationOnLine) - AlignSequence(); - } else if (Changes[i].Kind == tok::r_paren) { - if (!FoundLeftParenOnLine) - AlignSequence(); - FoundLeftParenOnLine = false; - } else if (Changes[i].Kind == tok::l_paren) { - FoundLeftParenOnLine = true; - if (!FoundDeclarationOnLine) - AlignSequence(); - } else if (!FoundDeclarationOnLine && !FoundLeftBraceOnLine && - !FoundLeftParenOnLine && Changes[i].IsStartOfDeclName) { - FoundDeclarationOnLine = true; - if (StartOfSequence == 0) - StartOfSequence = i; - - unsigned ChangeMinColumn = Changes[i].StartOfTokenColumn; - int LineLengthAfter = -Changes[i].Spaces; - for (unsigned j = i; j != e && Changes[j].NewlinesBefore == 0; ++j) - LineLengthAfter += Changes[j].Spaces + Changes[j].TokenLength; - unsigned ChangeMaxColumn = Style.ColumnLimit - LineLengthAfter; - - if (ChangeMinColumn > MaxColumn || ChangeMaxColumn < MinColumn) { - AlignSequence(); - StartOfSequence = i; - } + if (Changes[i].Kind == tok::comma) { + ++CommasBeforeMatch; + } else if (Changes[i].Kind == tok::r_brace || + Changes[i].Kind == tok::r_paren || + Changes[i].Kind == tok::r_square) { + --NestingLevel; + } else if (Changes[i].Kind == tok::l_brace || + Changes[i].Kind == tok::l_paren || + Changes[i].Kind == tok::l_square) { + // We want sequences to skip over child scopes if possible, but not the + // other way around. + NestingLevelOfLastMatch = std::min(NestingLevelOfLastMatch, NestingLevel); + ++NestingLevel; + } - MinColumn = std::max(MinColumn, ChangeMinColumn); - MaxColumn = std::min(MaxColumn, ChangeMaxColumn); + if (!Matches(Changes[i])) + continue; + + // If there is more than one matching token per line, or if the number of + // preceding commas, or the scope depth, do not match anymore, end the + // sequence. + if (FoundMatchOnLine || CommasBeforeMatch != CommasBeforeLastMatch || + NestingLevel != NestingLevelOfLastMatch) + AlignCurrentSequence(); + + CommasBeforeLastMatch = CommasBeforeMatch; + NestingLevelOfLastMatch = NestingLevel; + FoundMatchOnLine = true; + + if (StartOfSequence == 0) + StartOfSequence = i; + + unsigned ChangeMinColumn = Changes[i].StartOfTokenColumn; + int LineLengthAfter = -Changes[i].Spaces; + for (unsigned j = i; j != e && Changes[j].NewlinesBefore == 0; ++j) + LineLengthAfter += Changes[j].Spaces + 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) { + AlignCurrentSequence(); + StartOfSequence = i; } + + MinColumn = std::max(MinColumn, ChangeMinColumn); + MaxColumn = std::min(MaxColumn, ChangeMaxColumn); } EndOfSequence = Changes.size(); - AlignSequence(); + AlignCurrentSequence(); } -void WhitespaceManager::alignConsecutiveDeclarations(unsigned Start, - unsigned End, - unsigned Column) { - bool FoundDeclarationOnLine = false; - int Shift = 0; - for (unsigned i = Start; i != End; ++i) { - if (Changes[i].NewlinesBefore != 0) { - FoundDeclarationOnLine = false; - Shift = 0; - } +void WhitespaceManager::alignConsecutiveAssignments() { + if (!Style.AlignConsecutiveAssignments) + return; - if (!FoundDeclarationOnLine && Changes[i].IsStartOfDeclName) { - FoundDeclarationOnLine = true; - Shift = Column - Changes[i].StartOfTokenColumn; - Changes[i].Spaces += Shift; - } + AlignTokens(Style, + [&](const Change &C) { + // Do not align on equal signs that are first on a line. + if (C.NewlinesBefore > 0) + return false; - assert(Shift >= 0); - Changes[i].StartOfTokenColumn += Shift; - if (i + 1 != Changes.size()) - Changes[i + 1].PreviousEndOfTokenColumn += Shift; - } + // Do not align on equal signs that are last on a line. + if (&C != &Changes.back() && (&C + 1)->NewlinesBefore > 0) + return false; + + return C.Kind == tok::equal; + }, + Changes); +} + +void WhitespaceManager::alignConsecutiveDeclarations() { + if (!Style.AlignConsecutiveDeclarations) + return; + + // FIXME: Currently we don't handle properly the PointerAlignment: Right + // The * and & are not aligned and are left dangling. Something has to be done + // about it, but it raises the question of alignment of code like: + // const char* const* v1; + // float const* v2; + // SomeVeryLongType const& v3; + + AlignTokens(Style, [](Change const &C) { return C.IsStartOfDeclName; }, + Changes); } void WhitespaceManager::alignTrailingComments() { Index: unittests/Format/FormatTest.cpp =================================================================== --- unittests/Format/FormatTest.cpp +++ unittests/Format/FormatTest.cpp @@ -8659,7 +8659,7 @@ Alignment); verifyFormat("class C {\n" "public:\n" - " int i = 1;\n" + " int i = 1;\n" " virtual void f() = 0;\n" "};", Alignment); @@ -8708,6 +8708,19 @@ " loooooooooooooooooooooongParameterB);\n" "int j = 2;", Alignment); + + verifyFormat("template \n" + "auto foo() {}\n", + Alignment); + verifyFormat("int a, b = 1;\n" + "int c = 2;\n" + "int dd = 3;\n", + Alignment); + verifyFormat("int aa = ((1 > 2) ? 3 : 4);\n" + "float b[1][] = {{3.f}};\n", + Alignment); } TEST_F(FormatTest, AlignConsecutiveDeclarations) { @@ -8908,6 +8921,47 @@ "int myvar = 1;", Alignment); Alignment.ColumnLimit = 80; + Alignment.AlignConsecutiveAssignments = false; + + verifyFormat( + "template \n" + "auto foo() {}\n", + Alignment); + verifyFormat("float a, b = 1;\n" + "int c = 2;\n" + "int dd = 3;\n", + Alignment); + verifyFormat("int aa = ((1 > 2) ? 3 : 4);\n" + "float b[1][] = {{3.f}};\n", + Alignment); + Alignment.AlignConsecutiveAssignments = true; + verifyFormat("float a, b = 1;\n" + "int c = 2;\n" + "int dd = 3;\n", + Alignment); + verifyFormat("int aa = ((1 > 2) ? 3 : 4);\n" + "float b[1][] = {{3.f}};\n", + Alignment); + Alignment.AlignConsecutiveAssignments = false; + + Alignment.ColumnLimit = 30; + Alignment.BinPackParameters = false; + verifyFormat("void foo(float a,\n" + " float b,\n" + " int c,\n" + " uint32_t *d) {\n" + " int * e = 0;\n" + " float f = 0;\n" + " double g = 0;\n" + "}\n" + "void bar(ino_t a,\n" + " int b,\n" + " uint32_t *c,\n" + " bool d) {}\n", + Alignment); + Alignment.BinPackParameters = true; + Alignment.ColumnLimit = 80; } TEST_F(FormatTest, LinuxBraceBreaking) {