Hi djasper,
http://reviews.llvm.org/D9532
Files:
include/clang/Format/Format.h
lib/Format/Format.cpp
lib/Format/UnwrappedLineFormatter.cpp
lib/Format/UnwrappedLineFormatter.h
unittests/Format/FormatTest.cpp
EMAIL PREFERENCES
http://reviews.llvm.org/settings/panel/emailpreferences/
Index: include/clang/Format/Format.h
===================================================================
--- include/clang/Format/Format.h
+++ include/clang/Format/Format.h
@@ -546,16 +546,22 @@
///
/// Returns the \c Replacements necessary to make all \p Ranges comply with
/// \p Style.
+///
+/// If \c SkippedLines is non-null, its value will be set to true if any
+/// of the affected lines were not formatted due to a non-recoverable syntax
+/// error.
tooling::Replacements reformat(const FormatStyle &Style,
SourceManager &SourceMgr, FileID ID,
- ArrayRef<CharSourceRange> Ranges);
+ ArrayRef<CharSourceRange> Ranges,
+ bool *SkippedLines = nullptr);
/// \brief Reformats the given \p Ranges in \p Code.
///
-/// Otherwise identical to the reformat() function consuming a \c Lexer.
+/// Otherwise identical to the reformat() function using a file ID.
tooling::Replacements reformat(const FormatStyle &Style, StringRef Code,
ArrayRef<tooling::Range> Ranges,
- StringRef FileName = "<stdin>");
+ StringRef FileName = "<stdin>",
+ bool *SkippedLines = nullptr);
/// \brief Returns the \c LangOpts that the formatter expects you to set.
///
Index: lib/Format/Format.cpp
===================================================================
--- lib/Format/Format.cpp
+++ lib/Format/Format.cpp
@@ -1219,7 +1219,7 @@
<< "\n");
}
- tooling::Replacements format() {
+ tooling::Replacements format(bool *SkippedLines) {
tooling::Replacements Result;
FormatTokenLexer Tokens(SourceMgr, ID, Style, Encoding);
@@ -1234,7 +1234,8 @@
for (unsigned i = 0, e = UnwrappedLines[Run].size(); i != e; ++i) {
AnnotatedLines.push_back(new AnnotatedLine(UnwrappedLines[Run][i]));
}
- tooling::Replacements RunResult = format(AnnotatedLines, Tokens);
+ tooling::Replacements RunResult =
+ format(AnnotatedLines, Tokens, SkippedLines);
DEBUG({
llvm::dbgs() << "Replacements for run " << Run << ":\n";
for (tooling::Replacements::iterator I = RunResult.begin(),
@@ -1253,7 +1254,7 @@
}
tooling::Replacements format(SmallVectorImpl<AnnotatedLine *> &AnnotatedLines,
- FormatTokenLexer &Tokens) {
+ FormatTokenLexer &Tokens, bool *SkippedLines) {
TokenAnnotator Annotator(Style, Tokens.getKeywords());
for (unsigned i = 0, e = AnnotatedLines.size(); i != e; ++i) {
Annotator.annotate(*AnnotatedLines[i]);
@@ -1269,7 +1270,7 @@
Whitespaces, Encoding,
BinPackInconclusiveFunctions);
UnwrappedLineFormatter Formatter(&Indenter, &Whitespaces, Style,
- Tokens.getKeywords());
+ Tokens.getKeywords(), SkippedLines);
Formatter.format(AnnotatedLines, /*DryRun=*/false);
return Whitespaces.generateReplacements();
}
@@ -1489,16 +1490,18 @@
tooling::Replacements reformat(const FormatStyle &Style,
SourceManager &SourceMgr, FileID ID,
- ArrayRef<CharSourceRange> Ranges) {
+ ArrayRef<CharSourceRange> Ranges,
+ bool *SkippedLines) {
if (Style.DisableFormat)
return tooling::Replacements();
Formatter formatter(Style, SourceMgr, ID, Ranges);
- return formatter.format();
+ return formatter.format(SkippedLines);
}
tooling::Replacements reformat(const FormatStyle &Style, StringRef Code,
ArrayRef<tooling::Range> Ranges,
- StringRef FileName) {
+ StringRef FileName,
+ bool *SkippedLines) {
if (Style.DisableFormat)
return tooling::Replacements();
@@ -1521,7 +1524,7 @@
SourceLocation End = Start.getLocWithOffset(Range.getLength());
CharRanges.push_back(CharSourceRange::getCharRange(Start, End));
}
- return reformat(Style, SourceMgr, ID, CharRanges);
+ return reformat(Style, SourceMgr, ID, CharRanges, SkippedLines);
}
LangOptions getFormattingLangOpts(const FormatStyle &Style) {
Index: lib/Format/UnwrappedLineFormatter.cpp
===================================================================
--- lib/Format/UnwrappedLineFormatter.cpp
+++ lib/Format/UnwrappedLineFormatter.cpp
@@ -401,91 +401,111 @@
}
I += MergedLines;
- bool FixIndentation =
- FixBadIndentation && (LevelIndent != FirstTok->OriginalColumn);
- 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 &&
- (TheLine.Affected || FixIndentation)) {
- if (FirstTok->WhitespaceRange.isValid()) {
- if (!DryRun)
- formatFirstToken(*TheLine.First, PreviousLine, TheLine.Level, Indent,
- TheLine.InPPDirective);
- } else {
- Indent = LevelIndent = FirstTok->OriginalColumn;
- }
+ Penalty += formatLine(TheLine, PreviousLine, I + 1 != E ? I[1] : nullptr,
+ LevelIndent, Indent, IndentForLevel, DryRun,
+ FixBadIndentation);
+ if (!DryRun)
+ markFinalized(TheLine.First);
+ PreviousLine = *I;
+ }
+ PenaltyCache[CacheKey] = Penalty;
+ return Penalty;
+}
- // 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);
- }
+unsigned UnwrappedLineFormatter::formatLine(
+ const AnnotatedLine &TheLine, const AnnotatedLine *PreviousLine,
+ const AnnotatedLine *NextLine, unsigned LevelIndent, unsigned &Indent,
+ std::vector<int> &IndentForLevel, bool DryRun, bool FixBadIndentation) {
+ const FormatToken *FirstTok = TheLine.First;
+ int Offset = getIndentOffset(*FirstTok);
+ bool FixIndentation =
+ FixBadIndentation && (LevelIndent != FirstTok->OriginalColumn);
+ 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);
+ }
+ return 0;
+ }
- if (TheLine.Last->TotalLength + Indent <= ColumnLimit ||
- TheLine.Type == LT_ImportStatement) {
- LineState State = Indenter->getInitialState(Indent, &TheLine, DryRun);
- while (State.NextToken) {
- formatChildren(State, /*Newline=*/false, DryRun, Penalty);
- Indenter->addTokenToState(State, /*Newline=*/false, DryRun);
- }
- } else if (Style.ColumnLimit == 0) {
- NoColumnLimitFormatter Formatter(Indenter, this);
- if (!DryRun)
- Formatter.format(Indent, &TheLine);
- } else {
- Penalty += format(TheLine, Indent, DryRun);
- }
+ bool ShouldFormat = TheLine.Affected || FixIndentation;
+ if (TheLine.Type != LT_Invalid && ShouldFormat) {
+ unsigned Penalty = 0;
+ if (FirstTok->WhitespaceRange.isValid()) {
+ if (!DryRun)
+ formatFirstToken(*TheLine.First, PreviousLine, TheLine.Level, Indent,
+ TheLine.InPPDirective);
+ } else {
+ Indent = LevelIndent = FirstTok->OriginalColumn;
+ }
- if (!TheLine.InPPDirective)
- IndentForLevel[TheLine.Level] = LevelIndent;
- } else if (TheLine.ChildrenAffected) {
- format(TheLine.Children, DryRun);
+ // If everything fits on a single line, just put it there.
+ unsigned ColumnLimit = Style.ColumnLimit;
+ if (NextLine && NextLine->InPPDirective && !NextLine->First->HasUnescapedNewline) {
+ ColumnLimit = getColumnLimit(TheLine.InPPDirective);
+ }
+
+ if (TheLine.Last->TotalLength + Indent <= ColumnLimit ||
+ TheLine.Type == LT_ImportStatement) {
+ LineState State = Indenter->getInitialState(Indent, &TheLine, DryRun);
+ while (State.NextToken) {
+ formatChildren(State, /*Newline=*/false, DryRun, Penalty);
+ Indenter->addTokenToState(State, /*Newline=*/false, DryRun);
+ }
+ } else if (Style.ColumnLimit == 0) {
+ NoColumnLimitFormatter Formatter(Indenter, this);
+ if (!DryRun)
+ Formatter.format(Indent, &TheLine);
} 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) {
+ Penalty = formatLine(TheLine, Indent, DryRun);
+ }
+
+ if (!TheLine.InPPDirective)
+ IndentForLevel[TheLine.Level] = LevelIndent;
+ return Penalty;
+
+ } else if (ShouldFormat && SkippedLines) {
+ *SkippedLines = true;
+ }
+
+ if (TheLine.ChildrenAffected) {
+ format(TheLine.Children, DryRun);
+ return 0;
+ }
+
+ // 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) {
+ Whitespaces->addUntouchableToken(*Tok, TheLine.InPPDirective);
}
- if (!DryRun)
- markFinalized(TheLine.First);
- PreviousLine = *I;
}
- PenaltyCache[CacheKey] = Penalty;
- return Penalty;
+ return 0;
}
-unsigned UnwrappedLineFormatter::format(const AnnotatedLine &Line,
- unsigned FirstIndent, bool DryRun) {
+unsigned UnwrappedLineFormatter::formatLine(const AnnotatedLine &Line,
+ unsigned FirstIndent, bool DryRun) {
LineState State = Indenter->getInitialState(FirstIndent, &Line, DryRun);
// If the ObjC method declaration does not fit on a line, we should format
@@ -728,7 +748,7 @@
/*Newlines=*/0, /*IndentLevel=*/0, /*Spaces=*/1,
/*StartOfTokenColumn=*/State.Column, State.Line->InPPDirective);
}
- Penalty += format(*Previous.Children[0], State.Column + 1, DryRun);
+ Penalty += formatLine(*Previous.Children[0], State.Column + 1, DryRun);
State.Column += 1 + Previous.Children[0]->Last->TotalLength;
return true;
Index: lib/Format/UnwrappedLineFormatter.h
===================================================================
--- lib/Format/UnwrappedLineFormatter.h
+++ lib/Format/UnwrappedLineFormatter.h
@@ -33,9 +33,10 @@
UnwrappedLineFormatter(ContinuationIndenter *Indenter,
WhitespaceManager *Whitespaces,
const FormatStyle &Style,
- const AdditionalKeywords &Keywords)
+ const AdditionalKeywords &Keywords,
+ bool *SkippedLines)
: Indenter(Indenter), Whitespaces(Whitespaces), Style(Style),
- Keywords(Keywords) {}
+ Keywords(Keywords), SkippedLines(SkippedLines) {}
unsigned format(const SmallVectorImpl<AnnotatedLine *> &Lines, bool DryRun,
int AdditionalIndent = 0, bool FixBadIndentation = false);
@@ -67,9 +68,22 @@
private:
/// \brief Formats an \c AnnotatedLine and returns the penalty.
///
+ /// This function will decide whether a line should be formatted,
+ /// and what the appropriate indent is; if it decides the line
+ /// should be formated, it will do so.
+ ///
+ /// \c Indent and \c IndentForLevel will be updated.
+ unsigned formatLine(const AnnotatedLine &TheLine,
+ const AnnotatedLine *PreviousLine,
+ const AnnotatedLine *NextLine, unsigned LevelIndent,
+ unsigned &Indent, std::vector<int> &IndentForLevel,
+ bool DryRun, bool FixBadIndentation);
+
+ /// \brief Formats an \c AnnotatedLine and returns the penalty.
+ ///
/// If \p DryRun is \c false, directly applies the changes.
- unsigned format(const AnnotatedLine &Line, unsigned FirstIndent,
- bool DryRun);
+ unsigned formatLine(const AnnotatedLine &Line, unsigned FirstIndent,
+ bool DryRun);
/// \brief An edge in the solution space from \c Previous->State to \c State,
/// inserting a newline dependent on the \c NewLine.
@@ -167,6 +181,8 @@
// are many nested blocks.
std::map<std::pair<const SmallVectorImpl<AnnotatedLine *> *, unsigned>,
unsigned> PenaltyCache;
+
+ bool *SkippedLines;
};
} // end namespace format
} // end namespace clang
Index: unittests/Format/FormatTest.cpp
===================================================================
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -22,21 +22,26 @@
class FormatTest : public ::testing::Test {
protected:
std::string format(llvm::StringRef Code, unsigned Offset, unsigned Length,
- const FormatStyle &Style) {
+ const FormatStyle &Style,
+ bool ExpectSkippedLines = false) {
DEBUG(llvm::errs() << "---\n");
DEBUG(llvm::errs() << Code << "\n\n");
std::vector<tooling::Range> Ranges(1, tooling::Range(Offset, Length));
- tooling::Replacements Replaces = reformat(Style, Code, Ranges);
+ bool SkippedLines = false;
+ tooling::Replacements Replaces =
+ reformat(Style, Code, Ranges, "<stdin>", &SkippedLines);
+ EXPECT_EQ(ExpectSkippedLines, SkippedLines) << Code << "\n\n";
ReplacementCount = Replaces.size();
std::string Result = applyAllReplacements(Code, Replaces);
EXPECT_NE("", Result);
DEBUG(llvm::errs() << "\n" << Result << "\n\n");
return Result;
}
std::string format(llvm::StringRef Code,
- const FormatStyle &Style = getLLVMStyle()) {
- return format(Code, 0, Code.size(), Style);
+ const FormatStyle &Style = getLLVMStyle(),
+ bool ExpectSkippedLines = false) {
+ return format(Code, 0, Code.size(), Style, ExpectSkippedLines);
}
FormatStyle getLLVMStyleWithColumns(unsigned ColumnLimit) {
@@ -52,8 +57,10 @@
}
void verifyFormat(llvm::StringRef Code,
- const FormatStyle &Style = getLLVMStyle()) {
- EXPECT_EQ(Code.str(), format(test::messUp(Code), Style));
+ const FormatStyle &Style = getLLVMStyle(),
+ bool ExpectSkippedLines = false) {
+ EXPECT_EQ(Code.str(),
+ format(test::messUp(Code), Style, ExpectSkippedLines));
}
void verifyGoogleFormat(llvm::StringRef Code) {
@@ -67,8 +74,9 @@
/// \brief Verify that clang-format does not crash on the given input.
void verifyNoCrash(llvm::StringRef Code,
- const FormatStyle &Style = getLLVMStyle()) {
- format(Code, Style);
+ const FormatStyle &Style = getLLVMStyle(),
+ bool ExpectSkippedLines = false) {
+ format(Code, Style, ExpectSkippedLines);
}
int ReplacementCount;
@@ -2311,7 +2319,7 @@
"};\n");
// Incomplete try-catch blocks.
- verifyFormat("try {} catch (");
+ verifyFormat("try {} catch (", getLLVMStyle(), /*ExpectSkippedLines=*/true);
}
TEST_F(FormatTest, FormatSEHTryCatch) {
@@ -2714,35 +2722,39 @@
}
TEST_F(FormatTest, MacroDefinitionsWithIncompleteCode) {
- verifyFormat("#define A :");
+ verifyFormat("#define A :", getLLVMStyle(), /*ExpectSkippedLines=*/true);
verifyFormat("#define SOMECASES \\\n"
" case 1: \\\n"
" case 2\n",
getLLVMStyleWithColumns(20));
verifyFormat("#define A template <typename T>");
verifyFormat("#define STR(x) #x\n"
- "f(STR(this_is_a_string_literal{));");
+ "f(STR(this_is_a_string_literal{));",
+ getLLVMStyle(), /*ExpectSkippedLines=*/true);
verifyFormat("#pragma omp threadprivate( \\\n"
" y)), // expected-warning",
getLLVMStyleWithColumns(28));
verifyFormat("#d, = };");
verifyFormat("#if \"a");
verifyFormat("({\n"
"#define b }\\\n"
" a\n"
- "a");
+ "a",
+ getLLVMStyle(), /*ExpectSkippedLines=*/true);
verifyFormat("#define A \\\n"
" { \\\n"
" {\n"
"#define B \\\n"
" } \\\n"
" }",
getLLVMStyleWithColumns(15));
-
- verifyNoCrash("#if a\na(\n#else\n#endif\n{a");
+ verifyNoCrash("#if a\na(\n#else\n#endif\n{a", getLLVMStyle(),
+ /*ExpectSkippedLines=*/true);
verifyNoCrash("a={0,1\n#if a\n#else\n;\n#endif\n}");
- verifyNoCrash("#if a\na(\n#else\n#endif\n) a {a,b,c,d,f,g};");
- verifyNoCrash("#ifdef A\n a(\n #else\n #endif\n) = []() { \n)}");
+ verifyNoCrash("#if a\na(\n#else\n#endif\n) a {a,b,c,d,f,g};", getLLVMStyle(),
+ /*ExpectSkippedLines=*/true);
+ verifyNoCrash("#ifdef A\n a(\n #else\n #endif\n) = []() { \n)}",
+ getLLVMStyle(), /*ExpectSkippedLines=*/true);
}
TEST_F(FormatTest, MacrosWithoutTrailingSemicolon) {
@@ -3067,7 +3079,8 @@
"#if A\n"
" );\n"
"#else\n"
- "#endif");
+ "#endif",
+ getLLVMStyle(), /*ExpectSkippedLines=*/true);
}
TEST_F(FormatTest, GraciouslyHandleIncorrectPreprocessorConditions) {
@@ -5991,16 +6004,18 @@
TEST_F(FormatTest, IncorrectCodeMissingParens) {
verifyFormat("if {\n foo;\n foo();\n}");
verifyFormat("switch {\n foo;\n foo();\n}");
- verifyFormat("for {\n foo;\n foo();\n}");
+ verifyFormat("for {\n foo;\n foo();\n}", getLLVMStyle(),
+ /*ExpectSkippedLines=*/true);
verifyFormat("while {\n foo;\n foo();\n}");
verifyFormat("do {\n foo;\n foo();\n} while;");
}
TEST_F(FormatTest, DoesNotTouchUnwrappedLinesWithErrors) {
verifyFormat("namespace {\n"
"class Foo { Foo (\n"
"};\n"
- "} // comment");
+ "} // comment",
+ getLLVMStyle(), /*ExpectSkippedLines=*/true);
}
TEST_F(FormatTest, IncorrectCodeErrorDetection) {
@@ -7272,8 +7287,10 @@
"}");
verifyFormat("@{1 > 2 ? @\"one\" : @\"two\" : 1 > 2 ? @1 : @2}");
- verifyFormat("[self setDict:@{}");
- verifyFormat("[self setDict:@{@1 : @2}");
+ verifyFormat("[self setDict:@{}", getLLVMStyle(),
+ /*ExpectSkippedLines=*/true);
+ verifyFormat("[self setDict:@{@1 : @2}", getLLVMStyle(),
+ /*ExpectSkippedLines=*/true);
verifyFormat("NSLog(@\"%@\", @{@1 : @2, @2 : @3}[@1]);");
verifyFormat(
"NSDictionary *masses = @{@\"H\" : @1.0078, @\"He\" : @4.0026};");
@@ -7316,7 +7333,7 @@
}
TEST_F(FormatTest, ObjCArrayLiterals) {
- verifyFormat("@[");
+ verifyFormat("@[", getLLVMStyle(), /*ExpectSkippedLines=*/true);
verifyFormat("@[]");
verifyFormat(
"NSArray *array = @[ @\" Hey \", NSApp, [NSNumber numberWithInt:42] ];");
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
