r237104 - Refactor clang-format's formatter.
Manuel Klimek
klimek at google.com
Tue May 12 02:23:57 PDT 2015
Author: klimek
Date: Tue May 12 04:23:57 2015
New Revision: 237104
URL: http://llvm.org/viewvc/llvm-project?rev=237104&view=rev
Log:
Refactor clang-format's formatter.
Summary:
a) Pull out a class LevelIndentTracker whose responsibility is to keep track
of the indent of levels across multiple annotated lines.
b) Put all responsibility for merging lines into the LineJoiner; make the
LineJoiner iterate over the lines so we never operate on a line that might
be merged later; this makes the interface safer to use.
c) Move formatting of the end-of-file whitespace into formatFirstToken.
Fix bugs that became obvious after the refactoring:
1. We would not format lines with offsets correctly inside nested blocks if
only the outer expression was affected:
int x = s({ // clang-format only this line
class X {
public:
// ^ this starts at the non-modified indnent level; previously we would
// not fix this, now we correctly outdent it.
void f();
};
});
2. We would incorrectly align comments across lines that do not have comments
for lines with nested blocks:
int expression; // with comment
int x = s({
int y; // comment
int z; // we would incorrectly align this comment with the comment on
// 'expression'
});
Modified:
cfe/trunk/lib/Format/UnwrappedLineFormatter.cpp
cfe/trunk/lib/Format/UnwrappedLineFormatter.h
cfe/trunk/unittests/Format/FormatTest.cpp
Modified: cfe/trunk/lib/Format/UnwrappedLineFormatter.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/UnwrappedLineFormatter.cpp?rev=237104&r1=237103&r2=237104&view=diff
==============================================================================
--- cfe/trunk/lib/Format/UnwrappedLineFormatter.cpp (original)
+++ cfe/trunk/lib/Format/UnwrappedLineFormatter.cpp Tue May 12 04:23:57 2015
@@ -25,11 +25,134 @@ bool startsExternCBlock(const AnnotatedL
NextNext && NextNext->is(tok::l_brace);
}
+/// \brief Tracks the indent level of \c AnnotatedLines across levels.
+///
+/// \c nextLine must be called for each \c AnnotatedLine, after which \c
+/// getIndent() will return the indent for the last line \c nextLine was called
+/// with.
+/// If the line is not formatted (and thus the indent does not change), calling
+/// \c adjustToUnmodifiedLine after the call to \c nextLine will cause
+/// subsequent lines on the same level to be indented at the same level as the
+/// given line.
+class LevelIndentTracker {
+public:
+ LevelIndentTracker(const FormatStyle &Style,
+ const AdditionalKeywords &Keywords, unsigned StartLevel,
+ int AdditionalIndent)
+ : Style(Style), Keywords(Keywords) {
+ for (unsigned i = 0; i != StartLevel; ++i)
+ IndentForLevel.push_back(Style.IndentWidth * i + AdditionalIndent);
+ }
+
+ /// \brief Returns the indent for the current line.
+ unsigned getIndent() const { return Indent; }
+
+ /// \brief Update the indent state given that \p Line is going to be formatted
+ /// next.
+ void nextLine(const AnnotatedLine &Line) {
+ Offset = getIndentOffset(*Line.First);
+ if (Line.InPPDirective) {
+ Indent = Line.Level * Style.IndentWidth;
+ } else {
+ while (IndentForLevel.size() <= Line.Level)
+ IndentForLevel.push_back(-1);
+ IndentForLevel.resize(Line.Level + 1);
+ Indent = getIndent(IndentForLevel, Line.Level);
+ }
+ if (static_cast<int>(Indent) + Offset >= 0)
+ Indent += Offset;
+ }
+
+ /// \brief Update the level indent to adapt to the given \p Line.
+ ///
+ /// When a line is not formatted, we move the subsequent lines on the same
+ /// level to the same indent.
+ /// Note that \c nextLine must have been called before this method.
+ void adjustToUnmodifiedLine(const AnnotatedLine &Line) {
+ unsigned LevelIndent = Line.First->OriginalColumn;
+ if (static_cast<int>(LevelIndent) - Offset >= 0)
+ LevelIndent -= Offset;
+ if ((Line.First->isNot(tok::comment) || IndentForLevel[Line.Level] == -1) &&
+ !Line.InPPDirective)
+ IndentForLevel[Line.Level] = LevelIndent;
+ }
+
+private:
+ /// \brief Get the offset of the line relatively to the level.
+ ///
+ /// For example, 'public:' labels in classes are offset by 1 or 2
+ /// characters to the left from their level.
+ int getIndentOffset(const FormatToken &RootToken) {
+ if (Style.Language == FormatStyle::LK_Java ||
+ Style.Language == FormatStyle::LK_JavaScript)
+ return 0;
+ if (RootToken.isAccessSpecifier(false) ||
+ RootToken.isObjCAccessSpecifier() ||
+ (RootToken.is(Keywords.kw_signals) && RootToken.Next &&
+ RootToken.Next->is(tok::colon)))
+ return Style.AccessModifierOffset;
+ return 0;
+ }
+
+ /// \brief Get the indent of \p Level from \p IndentForLevel.
+ ///
+ /// \p IndentForLevel must contain the indent for the level \c l
+ /// at \p IndentForLevel[l], or a value < 0 if the indent for
+ /// that level is unknown.
+ unsigned getIndent(ArrayRef<int> IndentForLevel, unsigned Level) {
+ if (IndentForLevel[Level] != -1)
+ return IndentForLevel[Level];
+ if (Level == 0)
+ return 0;
+ return getIndent(IndentForLevel, Level - 1) + Style.IndentWidth;
+ }
+
+ const FormatStyle &Style;
+ const AdditionalKeywords &Keywords;
+
+ /// \brief The indent in characters for each level.
+ std::vector<int> IndentForLevel;
+
+ /// \brief Offset of the current line relative to the indent level.
+ ///
+ /// For example, the 'public' keywords is often indented with a negative
+ /// offset.
+ int Offset = 0;
+
+ /// \brief The current line's indent.
+ unsigned Indent = 0;
+};
+
class LineJoiner {
public:
- LineJoiner(const FormatStyle &Style, const AdditionalKeywords &Keywords)
- : Style(Style), Keywords(Keywords) {}
+ LineJoiner(const FormatStyle &Style, const AdditionalKeywords &Keywords,
+ const SmallVectorImpl<AnnotatedLine *> &Lines)
+ : Style(Style), Keywords(Keywords), End(Lines.end()),
+ Next(Lines.begin()) {}
+
+ /// \brief Returns the next line, merging multiple lines into one if possible.
+ const AnnotatedLine *getNextMergedLine(bool DryRun,
+ LevelIndentTracker &IndentTracker) {
+ if (Next == End)
+ return nullptr;
+ const AnnotatedLine *Current = *Next;
+ IndentTracker.nextLine(*Current);
+ unsigned MergedLines =
+ tryFitMultipleLinesInOne(IndentTracker.getIndent(), Next, End);
+ if (MergedLines > 0 && Style.ColumnLimit == 0)
+ // Disallow line merging if there is a break at the start of one of the
+ // input lines.
+ for (unsigned i = 0; i < MergedLines; ++i)
+ if (Next[i + 1]->First->NewlinesBefore > 0)
+ MergedLines = 0;
+ if (!DryRun)
+ for (unsigned i = 0; i < MergedLines; ++i)
+ join(*Next[i], *Next[i + 1]);
+ Next = Next + MergedLines + 1;
+ return Current;
+ }
+private:
/// \brief Calculates how many lines can be merged into 1 starting at \p I.
unsigned
tryFitMultipleLinesInOne(unsigned Indent,
@@ -119,7 +242,6 @@ public:
return 0;
}
-private:
unsigned
tryMergeSimplePPDirective(SmallVectorImpl<AnnotatedLine *>::const_iterator I,
SmallVectorImpl<AnnotatedLine *>::const_iterator E,
@@ -304,8 +426,26 @@ private:
return false;
}
+ void join(AnnotatedLine &A, const AnnotatedLine &B) {
+ assert(!A.Last->Next);
+ assert(!B.First->Previous);
+ if (B.Affected)
+ A.Affected = true;
+ A.Last->Next = B.First;
+ B.First->Previous = A.Last;
+ B.First->CanBreakBefore = true;
+ unsigned LengthA = A.Last->TotalLength + B.First->SpacesRequiredBefore;
+ for (FormatToken *Tok = B.First; Tok; Tok = Tok->Next) {
+ Tok->TotalLength += LengthA;
+ A.Last = Tok;
+ }
+ }
+
const FormatStyle &Style;
const AdditionalKeywords &Keywords;
+ const SmallVectorImpl<AnnotatedLine*>::const_iterator End;
+
+ SmallVectorImpl<AnnotatedLine*>::const_iterator Next;
};
static void markFinalized(FormatToken *Tok) {
@@ -655,7 +795,7 @@ unsigned
UnwrappedLineFormatter::format(const SmallVectorImpl<AnnotatedLine *> &Lines,
bool DryRun, int AdditionalIndent,
bool FixBadIndentation) {
- LineJoiner Joiner(Style, Keywords);
+ LineJoiner Joiner(Style, Keywords, Lines);
// Try to look up already computed penalty in DryRun-mode.
std::pair<const SmallVectorImpl<AnnotatedLine *> *, unsigned> CacheKey(
@@ -666,125 +806,77 @@ UnwrappedLineFormatter::format(const Sma
assert(!Lines.empty());
unsigned Penalty = 0;
- std::vector<int> IndentForLevel;
- for (unsigned i = 0, e = Lines[0]->Level; i != e; ++i)
- IndentForLevel.push_back(Style.IndentWidth * i + AdditionalIndent);
+ LevelIndentTracker IndentTracker(Style, Keywords, Lines[0]->Level,
+ AdditionalIndent);
const AnnotatedLine *PreviousLine = nullptr;
- for (SmallVectorImpl<AnnotatedLine *>::const_iterator I = Lines.begin(),
- E = Lines.end();
- I != E; ++I) {
- const AnnotatedLine &TheLine = **I;
- const FormatToken *FirstTok = TheLine.First;
- int Offset = getIndentOffset(*FirstTok);
-
- // Determine indent and try to merge multiple unwrapped lines.
- unsigned Indent;
- if (TheLine.InPPDirective) {
- Indent = TheLine.Level * Style.IndentWidth;
- } else {
- while (IndentForLevel.size() <= TheLine.Level)
- IndentForLevel.push_back(-1);
- IndentForLevel.resize(TheLine.Level + 1);
- Indent = getIndent(IndentForLevel, TheLine.Level);
- }
- unsigned LevelIndent = Indent;
- if (static_cast<int>(Indent) + Offset >= 0)
- Indent += Offset;
-
- // Merge multiple lines if possible.
- unsigned MergedLines = Joiner.tryFitMultipleLinesInOne(Indent, I, E);
- if (MergedLines > 0 && Style.ColumnLimit == 0) {
- // Disallow line merging if there is a break at the start of one of the
- // input lines.
- for (unsigned i = 0; i < MergedLines; ++i) {
- if (I[i + 1]->First->NewlinesBefore > 0)
- MergedLines = 0;
- }
- }
- if (!DryRun) {
- for (unsigned i = 0; i < MergedLines; ++i) {
- join(*I[i], *I[i + 1]);
- }
- }
- I += MergedLines;
-
+ const AnnotatedLine *NextLine = nullptr;
+ for (const AnnotatedLine *Line =
+ Joiner.getNextMergedLine(DryRun, IndentTracker);
+ Line; Line = NextLine) {
+ const AnnotatedLine &TheLine = *Line;
+ unsigned Indent = IndentTracker.getIndent();
bool FixIndentation =
- FixBadIndentation && (LevelIndent != FirstTok->OriginalColumn);
+ FixBadIndentation && (Indent != TheLine.First->OriginalColumn);
bool ShouldFormat = TheLine.Affected || FixIndentation;
- if (TheLine.First->is(tok::eof)) {
- if (PreviousLine && PreviousLine->Affected && !DryRun) {
- // Remove the file's trailing whitespace.
- unsigned Newlines = std::min(FirstTok->NewlinesBefore, 1u);
- Whitespaces->replaceWhitespace(*TheLine.First, Newlines,
- /*IndentLevel=*/0, /*Spaces=*/0,
- /*TargetColumn=*/0);
- }
- } else if (TheLine.Type != LT_Invalid && ShouldFormat) {
- if (FirstTok->WhitespaceRange.isValid()) {
- if (!DryRun)
- formatFirstToken(*TheLine.First, PreviousLine, TheLine.Level, Indent,
- TheLine.InPPDirective);
- } else {
- Indent = LevelIndent = FirstTok->OriginalColumn;
- }
+ // We cannot format this line; if the reason is that the line had a
+ // parsing error, remember that.
+ if (ShouldFormat && TheLine.Type == LT_Invalid && IncompleteFormat)
+ *IncompleteFormat = true;
- // If everything fits on a single line, just put it there.
- unsigned ColumnLimit = Style.ColumnLimit;
- if (I + 1 != E) {
- AnnotatedLine *NextLine = I[1];
- if (NextLine->InPPDirective && !NextLine->First->HasUnescapedNewline)
- ColumnLimit = getColumnLimit(TheLine.InPPDirective);
- }
+ if (ShouldFormat && TheLine.Type != LT_Invalid) {
+ if (!DryRun)
+ formatFirstToken(*TheLine.First, PreviousLine, TheLine.Level, Indent,
+ TheLine.InPPDirective);
+
+ NextLine = Joiner.getNextMergedLine(DryRun, IndentTracker);
+ unsigned ColumnLimit = getColumnLimit(TheLine.InPPDirective, NextLine);
+ bool FitsIntoOneLine =
+ TheLine.Last->TotalLength + Indent <= ColumnLimit ||
+ TheLine.Type == LT_ImportStatement;
- if (TheLine.Last->TotalLength + Indent <= ColumnLimit ||
- TheLine.Type == LT_ImportStatement) {
- Penalty += NoLineBreakFormatter(Indenter, Whitespaces, Style, this)
- .formatLine(TheLine, Indent, DryRun);
- } else if (Style.ColumnLimit == 0) {
+ if (Style.ColumnLimit == 0)
NoColumnLimitLineFormatter(Indenter, Whitespaces, Style, this)
.formatLine(TheLine, Indent, DryRun);
- } else {
+ else if (FitsIntoOneLine)
+ Penalty += NoLineBreakFormatter(Indenter, Whitespaces, Style, this)
+ .formatLine(TheLine, Indent, DryRun);
+ else
Penalty += OptimizingLineFormatter(Indenter, Whitespaces, Style, this)
.formatLine(TheLine, Indent, DryRun);
- }
-
- if (!TheLine.InPPDirective)
- IndentForLevel[TheLine.Level] = LevelIndent;
- } else if (TheLine.ChildrenAffected) {
- format(TheLine.Children, DryRun);
} else {
- // Format the first token if necessary, and notify the WhitespaceManager
- // about the unchanged whitespace.
- for (FormatToken *Tok = TheLine.First; Tok; Tok = Tok->Next) {
- if (Tok == TheLine.First && (Tok->NewlinesBefore > 0 || Tok->IsFirst)) {
- unsigned LevelIndent = Tok->OriginalColumn;
- if (!DryRun) {
- // Remove trailing whitespace of the previous line.
- if ((PreviousLine && PreviousLine->Affected) ||
- TheLine.LeadingEmptyLinesAffected) {
- formatFirstToken(*Tok, PreviousLine, TheLine.Level, LevelIndent,
- TheLine.InPPDirective);
- } else {
- Whitespaces->addUntouchableToken(*Tok, TheLine.InPPDirective);
- }
- }
-
- if (static_cast<int>(LevelIndent) - Offset >= 0)
- LevelIndent -= Offset;
- if ((Tok->isNot(tok::comment) ||
- IndentForLevel[TheLine.Level] == -1) &&
- !TheLine.InPPDirective)
- IndentForLevel[TheLine.Level] = LevelIndent;
- } else if (!DryRun) {
+ // If no token in the current line is affected, we still need to format
+ // affected children.
+ if (TheLine.ChildrenAffected)
+ format(TheLine.Children, DryRun);
+
+ // Adapt following lines on the current indent level to the same level
+ // unless the current \c AnnotatedLine is not at the beginning of a line.
+ bool StartsNewLine =
+ TheLine.First->NewlinesBefore > 0 || TheLine.First->IsFirst;
+ if (StartsNewLine)
+ IndentTracker.adjustToUnmodifiedLine(TheLine);
+ if (!DryRun) {
+ bool ReformatLeadingWhitespace =
+ StartsNewLine && ((PreviousLine && PreviousLine->Affected) ||
+ TheLine.LeadingEmptyLinesAffected);
+ // Format the first token.
+ if (ReformatLeadingWhitespace)
+ formatFirstToken(*TheLine.First, PreviousLine, TheLine.Level,
+ TheLine.First->OriginalColumn,
+ TheLine.InPPDirective);
+ else
+ Whitespaces->addUntouchableToken(*TheLine.First,
+ TheLine.InPPDirective);
+
+ // Notify the WhitespaceManager about the unchanged whitespace.
+ for (FormatToken *Tok = TheLine.First->Next; Tok; Tok = Tok->Next)
Whitespaces->addUntouchableToken(*Tok, TheLine.InPPDirective);
- }
}
+ NextLine = Joiner.getNextMergedLine(DryRun, IndentTracker);
}
- if (TheLine.Type == LT_Invalid && ShouldFormat && IncompleteFormat)
- *IncompleteFormat = true;
if (!DryRun)
markFinalized(TheLine.First);
- PreviousLine = *I;
+ PreviousLine = &TheLine;
}
PenaltyCache[CacheKey] = Penalty;
return Penalty;
@@ -795,6 +887,12 @@ void UnwrappedLineFormatter::formatFirst
unsigned IndentLevel,
unsigned Indent,
bool InPPDirective) {
+ if (RootToken.is(tok::eof)) {
+ unsigned Newlines = std::min(RootToken.NewlinesBefore, 1u);
+ Whitespaces->replaceWhitespace(RootToken, Newlines, /*IndentLevel=*/0,
+ /*Spaces=*/0, /*TargetColumn=*/0);
+ return;
+ }
unsigned Newlines =
std::min(RootToken.NewlinesBefore, Style.MaxEmptyLinesToKeep + 1);
// Remove empty lines before "}" where applicable.
@@ -829,33 +927,17 @@ void UnwrappedLineFormatter::formatFirst
!RootToken.HasUnescapedNewline);
}
-/// \brief Get the indent of \p Level from \p IndentForLevel.
-///
-/// \p IndentForLevel must contain the indent for the level \c l
-/// at \p IndentForLevel[l], or a value < 0 if the indent for
-/// that level is unknown.
-unsigned UnwrappedLineFormatter::getIndent(ArrayRef<int> IndentForLevel,
- unsigned Level) {
- if (IndentForLevel[Level] != -1)
- return IndentForLevel[Level];
- if (Level == 0)
- return 0;
- return getIndent(IndentForLevel, Level - 1) + Style.IndentWidth;
-}
-
-void UnwrappedLineFormatter::join(AnnotatedLine &A, const AnnotatedLine &B) {
- assert(!A.Last->Next);
- assert(!B.First->Previous);
- if (B.Affected)
- A.Affected = true;
- A.Last->Next = B.First;
- B.First->Previous = A.Last;
- B.First->CanBreakBefore = true;
- unsigned LengthA = A.Last->TotalLength + B.First->SpacesRequiredBefore;
- for (FormatToken *Tok = B.First; Tok; Tok = Tok->Next) {
- Tok->TotalLength += LengthA;
- A.Last = Tok;
- }
+unsigned
+UnwrappedLineFormatter::getColumnLimit(bool InPPDirective,
+ const AnnotatedLine *NextLine) const {
+ // In preprocessor directives reserve two chars for trailing " \" if the
+ // next line continues the preprocessor directive.
+ bool ContinuesPPDirective =
+ InPPDirective && NextLine && NextLine->InPPDirective &&
+ // If there is an unescaped newline between this line and the next, the
+ // next line starts a new preprocessor directive.
+ !NextLine->First->HasUnescapedNewline;
+ return Style.ColumnLimit - (ContinuesPPDirective ? 2 : 0);
}
} // namespace format
Modified: cfe/trunk/lib/Format/UnwrappedLineFormatter.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/UnwrappedLineFormatter.h?rev=237104&r1=237103&r2=237104&view=diff
==============================================================================
--- cfe/trunk/lib/Format/UnwrappedLineFormatter.h (original)
+++ cfe/trunk/lib/Format/UnwrappedLineFormatter.h Tue May 12 04:23:57 2015
@@ -44,41 +44,15 @@ public:
bool FixBadIndentation = false);
private:
- /// \brief Get the offset of the line relatively to the level.
- ///
- /// For example, 'public:' labels in classes are offset by 1 or 2
- /// characters to the left from their level.
- int getIndentOffset(const FormatToken &RootToken) {
- if (Style.Language == FormatStyle::LK_Java ||
- Style.Language == FormatStyle::LK_JavaScript)
- return 0;
- if (RootToken.isAccessSpecifier(false) ||
- RootToken.isObjCAccessSpecifier() ||
- (RootToken.is(Keywords.kw_signals) && RootToken.Next &&
- RootToken.Next->is(tok::colon)))
- return Style.AccessModifierOffset;
- return 0;
- }
-
/// \brief Add a new line and the required indent before the first Token
/// of the \c UnwrappedLine if there was no structural parsing error.
void formatFirstToken(FormatToken &RootToken,
const AnnotatedLine *PreviousLine, unsigned IndentLevel,
unsigned Indent, bool InPPDirective);
- /// \brief Get the indent of \p Level from \p IndentForLevel.
- ///
- /// \p IndentForLevel must contain the indent for the level \c l
- /// at \p IndentForLevel[l], or a value < 0 if the indent for
- /// that level is unknown.
- unsigned getIndent(ArrayRef<int> IndentForLevel, unsigned Level);
-
- void join(AnnotatedLine &A, const AnnotatedLine &B);
-
- unsigned getColumnLimit(bool InPPDirective) const {
- // In preprocessor directives reserve two chars for trailing " \"
- return Style.ColumnLimit - (InPPDirective ? 2 : 0);
- }
+ /// \brief Returns the column limit for a line, taking into account whether we
+ /// need an escaped newline due to a continued preprocessor directive.
+ unsigned getColumnLimit(bool InPPDirective, const AnnotatedLine *NextLine) const;
// Cache to store the penalty of formatting a vector of AnnotatedLines
// starting from a specific additional offset. Improves performance if there
Modified: cfe/trunk/unittests/Format/FormatTest.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=237104&r1=237103&r2=237104&view=diff
==============================================================================
--- cfe/trunk/unittests/Format/FormatTest.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTest.cpp Tue May 12 04:23:57 2015
@@ -2775,9 +2775,10 @@ TEST_F(FormatTest, MacroDefinitionsWithI
verifyFormat("#d, = };");
verifyFormat("#if \"a");
verifyIncompleteFormat("({\n"
- "#define b }\\\n"
+ "#define b \\\n"
+ " } \\\n"
" a\n"
- "a");
+ "a", getLLVMStyleWithColumns(15));
verifyFormat("#define A \\\n"
" { \\\n"
" {\n"
@@ -3179,6 +3180,30 @@ TEST_F(FormatTest, LayoutBlockInsidePare
" if (a)\n"
" f();\n"
"});");
+ EXPECT_EQ("int longlongname; // comment\n"
+ "int x = f({\n"
+ " int x; // comment\n"
+ " int y; // comment\n"
+ "});",
+ format("int longlongname; // comment\n"
+ "int x = f({\n"
+ " int x; // comment\n"
+ " int y; // comment\n"
+ "});",
+ 65, 0, getLLVMStyle()));
+ EXPECT_EQ("int s = f({\n"
+ " class X {\n"
+ " public:\n"
+ " void f();\n"
+ " };\n"
+ "});",
+ format("int s = f({\n"
+ " class X {\n"
+ " public:\n"
+ " void f();\n"
+ " };\n"
+ "});",
+ 0, 0, getLLVMStyle()));
}
TEST_F(FormatTest, LayoutBlockInsideStatement) {
More information about the cfe-commits
mailing list