r254406 - This fixes https://llvm.org/bugs/show_bug.cgi?id=25329, as well as
Daniel Jasper via cfe-commits
cfe-commits at lists.llvm.org
Tue Dec 1 04:00:44 PST 2015
Author: djasper
Date: Tue Dec 1 06:00:43 2015
New Revision: 254406
URL: http://llvm.org/viewvc/llvm-project?rev=254406&view=rev
Log:
This fixes https://llvm.org/bugs/show_bug.cgi?id=25329, as well as
misalignments like the following:
int a, b = 2;
int c = 3;
Patch by Beren Minor, thanks!
Modified:
cfe/trunk/lib/Format/WhitespaceManager.cpp
cfe/trunk/lib/Format/WhitespaceManager.h
cfe/trunk/unittests/Format/FormatTest.cpp
Modified: cfe/trunk/lib/Format/WhitespaceManager.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/WhitespaceManager.cpp?rev=254406&r1=254405&r2=254406&view=diff
==============================================================================
--- cfe/trunk/lib/Format/WhitespaceManager.cpp (original)
+++ cfe/trunk/lib/Format/WhitespaceManager.cpp Tue Dec 1 06:00:43 2015
@@ -148,125 +148,24 @@ void WhitespaceManager::calculateLineBre
}
}
-// 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 <typename F>
+static void
+AlignTokenSequence(unsigned Start, unsigned End, unsigned Column, F &&Matches,
+ SmallVector<WhitespaceManager::Change, 16> &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 @@ void WhitespaceManager::alignConsecutive
}
}
-// 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 <typename F>
+static void AlignTokens(const FormatStyle &Style, F &&Matches,
+ SmallVector<WhitespaceManager::Change, 16> &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 @@ void WhitespaceManager::alignConsecutive
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::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;
+ }
+
+ 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;
}
- 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;
- }
-
- MinColumn = std::max(MinColumn, ChangeMinColumn);
- MaxColumn = std::min(MaxColumn, ChangeMaxColumn);
- }
+ 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;
+
+ // 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);
+}
- assert(Shift >= 0);
- Changes[i].StartOfTokenColumn += Shift;
- if (i + 1 != Changes.size())
- Changes[i + 1].PreviousEndOfTokenColumn += Shift;
- }
+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() {
Modified: cfe/trunk/lib/Format/WhitespaceManager.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/WhitespaceManager.h?rev=254406&r1=254405&r2=254406&view=diff
==============================================================================
--- cfe/trunk/lib/Format/WhitespaceManager.h (original)
+++ cfe/trunk/lib/Format/WhitespaceManager.h Tue Dec 1 06:00:43 2015
@@ -81,7 +81,6 @@ public:
/// \brief Returns all the \c Replacements created during formatting.
const tooling::Replacements &generateReplacements();
-private:
/// \brief Represents a change before a token, a break inside a token,
/// or the layout of an unchanged token (or whitespace within).
struct Change {
@@ -160,6 +159,7 @@ private:
int IndentationOffset;
};
+private:
/// \brief Calculate \c IsTrailingComment, \c TokenLength for the last tokens
/// or token parts in a line and \c PreviousEndOfTokenColumn and
/// \c EscapedNewlineColumn for the first tokens or token parts in a line.
@@ -168,20 +168,9 @@ private:
/// \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();
Modified: cfe/trunk/unittests/Format/FormatTest.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=254406&r1=254405&r2=254406&view=diff
==============================================================================
--- cfe/trunk/unittests/Format/FormatTest.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTest.cpp Tue Dec 1 06:00:43 2015
@@ -8659,7 +8659,7 @@ TEST_F(FormatTest, AlignConsecutiveAssig
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 @@ TEST_F(FormatTest, AlignConsecutiveAssig
" loooooooooooooooooooooongParameterB);\n"
"int j = 2;",
Alignment);
+
+ verifyFormat("template <typename T, typename T_0 = very_long_type_name_0,\n"
+ " typename B = very_long_type_name_1,\n"
+ " typename T_2 = very_long_type_name_2>\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 @@ TEST_F(FormatTest, AlignConsecutiveDecla
"int myvar = 1;",
Alignment);
Alignment.ColumnLimit = 80;
+ Alignment.AlignConsecutiveAssignments = false;
+
+ verifyFormat(
+ "template <typename LongTemplate, typename VeryLongTemplateTypeName,\n"
+ " typename LongType, typename B>\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) {
More information about the cfe-commits
mailing list