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 Ranges); + ArrayRef 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 Ranges, - StringRef FileName = ""); + StringRef FileName = "", + 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 &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 Ranges) { + ArrayRef 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 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.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 &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 &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 *, unsigned>, unsigned> PenaltyCache; + + bool *SkippedLines; }; } // end namespace format } // end namespace clang 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 &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(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(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: unittests/Format/FormatTest.cpp =================================================================== --- unittests/Format/FormatTest.cpp +++ unittests/Format/FormatTest.cpp @@ -22,11 +22,15 @@ 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 Ranges(1, tooling::Range(Offset, Length)); - tooling::Replacements Replaces = reformat(Style, Code, Ranges); + bool SkippedLines = false; + tooling::Replacements Replaces = + reformat(Style, Code, Ranges, "", &SkippedLines); + EXPECT_EQ(ExpectSkippedLines, SkippedLines) << Code << "\n\n"; ReplacementCount = Replaces.size(); std::string Result = applyAllReplacements(Code, Replaces); EXPECT_NE("", Result); @@ -35,8 +39,9 @@ } 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,14 +2722,15 @@ } 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 "); 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)); @@ -2730,7 +2739,8 @@ verifyFormat("({\n" "#define b }\\\n" " a\n" - "a"); + "a", + getLLVMStyle(), /*ExpectSkippedLines=*/true); verifyFormat("#define A \\\n" " { \\\n" " {\n" @@ -2738,11 +2748,13 @@ " } \\\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,7 +6004,8 @@ 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;"); } @@ -6000,7 +6014,8 @@ 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] ];");