From f876984cc6be6c1c07fbe0df1084c1b755585414 Mon Sep 17 00:00:00 2001 From: Daniel Jasper Date: Mon, 25 Nov 2013 11:08:59 +0000 Subject: [PATCH] clang-format: Refactor calculation of lines intersecting with -lines. No functional changes intended. However, it seems to have found a buggy behavior in one of the tests. I think this structure is generally desirable and it will make a planned bugfix significantly easier. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@195634 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/Format/ContinuationIndenter.cpp | 4 +- lib/Format/Format.cpp | 182 +++++++++++++++++----------- lib/Format/TokenAnnotator.h | 11 +- unittests/Format/FormatTest.cpp | 4 +- 4 files changed, 124 insertions(+), 77 deletions(-) diff --git a/lib/Format/ContinuationIndenter.cpp b/lib/Format/ContinuationIndenter.cpp index 3a49ec43f9..a8891fcc2f 100644 --- a/lib/Format/ContinuationIndenter.cpp +++ b/lib/Format/ContinuationIndenter.cpp @@ -580,8 +580,8 @@ unsigned ContinuationIndenter::moveStateToNextToken(LineState &State, // // instead of: // SomeFunction(a, [] { - // f(); // break - // }); + // f(); // break + // }); for (unsigned i = 0; i != Current.MatchingParen->FakeRParens; ++i) State.Stack.pop_back(); NewIndent = State.Stack.back().LastSpace + Style.IndentWidth; diff --git a/lib/Format/Format.cpp b/lib/Format/Format.cpp index c0a60171f0..1f3eef4759 100644 --- a/lib/Format/Format.cpp +++ b/lib/Format/Format.cpp @@ -553,13 +553,11 @@ private: class UnwrappedLineFormatter { public: - UnwrappedLineFormatter(SourceManager &SourceMgr, - SmallVectorImpl &Ranges, - ContinuationIndenter *Indenter, + UnwrappedLineFormatter(ContinuationIndenter *Indenter, WhitespaceManager *Whitespaces, const FormatStyle &Style) - : SourceMgr(SourceMgr), Ranges(Ranges), Indenter(Indenter), - Whitespaces(Whitespaces), Style(Style), Joiner(Style) {} + : Indenter(Indenter), Whitespaces(Whitespaces), Style(Style), + Joiner(Style) {} unsigned format(const SmallVectorImpl &Lines, bool DryRun, int AdditionalIndent = 0) { @@ -568,9 +566,7 @@ public: std::vector IndentForLevel; for (unsigned i = 0, e = Lines[0]->Level; i != e; ++i) IndentForLevel.push_back(Style.IndentWidth * i + AdditionalIndent); - bool PreviousLineWasTouched = false; const AnnotatedLine *PreviousLine = NULL; - bool FormatPPDirective = false; for (SmallVectorImpl::const_iterator I = Lines.begin(), E = Lines.end(); I != E; ++I) { @@ -578,13 +574,6 @@ public: const FormatToken *FirstTok = TheLine.First; int Offset = getIndentOffset(*FirstTok); - // Check whether this line is part of a formatted preprocessor directive. - if (FirstTok->HasUnescapedNewline) - FormatPPDirective = false; - if (!FormatPPDirective && TheLine.InPPDirective && - (touchesLine(TheLine) || touchesPPDirective(I + 1, E))) - FormatPPDirective = true; - // Determine indent and try to merge multiple unwrapped lines. while (IndentForLevel.size() <= TheLine.Level) IndentForLevel.push_back(-1); @@ -600,16 +589,15 @@ public: } I += MergedLines; - bool WasMoved = PreviousLineWasTouched && FirstTok->NewlinesBefore == 0; if (TheLine.First->is(tok::eof)) { - if (PreviousLineWasTouched && !DryRun) { + 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 && - (WasMoved || FormatPPDirective || touchesLine(TheLine))) { + } else if (TheLine.Type != LT_Invalid && TheLine.Affected) { unsigned LevelIndent = getIndent(IndentForLevel, TheLine.Level); if (FirstTok->WhitespaceRange.isValid()) { if (!DryRun) @@ -640,7 +628,6 @@ public: } IndentForLevel[TheLine.Level] = LevelIndent; - PreviousLineWasTouched = true; } else { // Format the first token if necessary, and notify the WhitespaceManager // about the unchanged whitespace. @@ -649,9 +636,9 @@ public: (Tok->NewlinesBefore > 0 || Tok->IsFirst)) { unsigned LevelIndent = Tok->OriginalColumn; if (!DryRun) { - // Remove trailing whitespace of the previous line if it was - // touched. - if (PreviousLineWasTouched || touchesEmptyLineBefore(TheLine)) { + // Remove trailing whitespace of the previous line if. + if ((PreviousLine && PreviousLine->Affected) || + TheLine.LeadingEmptyLinesAffected) { formatFirstToken(*Tok, PreviousLine, TheLine.Level, LevelIndent, TheLine.InPPDirective); } else { @@ -667,10 +654,6 @@ public: Whitespaces->addUntouchableToken(*Tok, TheLine.InPPDirective); } } - // If we did not reformat this unwrapped line, the column at the end of - // the last token is unchanged - thus, we can calculate the end of the - // last token. - PreviousLineWasTouched = false; } if (!DryRun) { for (FormatToken *Tok = TheLine.First; Tok != NULL; Tok = Tok->Next) { @@ -779,6 +762,8 @@ private: 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; @@ -794,48 +779,6 @@ private: return Style.ColumnLimit - (InPPDirective ? 2 : 0); } - bool touchesRanges(const CharSourceRange &Range) { - for (SmallVectorImpl::const_iterator I = Ranges.begin(), - E = Ranges.end(); - I != E; ++I) { - if (!SourceMgr.isBeforeInTranslationUnit(Range.getEnd(), I->getBegin()) && - !SourceMgr.isBeforeInTranslationUnit(I->getEnd(), Range.getBegin())) - return true; - } - return false; - } - - bool touchesLine(const AnnotatedLine &TheLine) { - const FormatToken *First = TheLine.First; - const FormatToken *Last = TheLine.Last; - CharSourceRange LineRange = CharSourceRange::getCharRange( - First->WhitespaceRange.getBegin().getLocWithOffset( - First->LastNewlineOffset), - Last->getStartOfNonWhitespace().getLocWithOffset( - Last->TokenText.size() - 1)); - return touchesRanges(LineRange); - } - - bool touchesPPDirective(SmallVectorImpl::const_iterator I, - SmallVectorImpl::const_iterator E) { - for (; I != E; ++I) { - if ((*I)->First->HasUnescapedNewline) - return false; - if (touchesLine(**I)) - return true; - } - return false; - } - - bool touchesEmptyLineBefore(const AnnotatedLine &TheLine) { - const FormatToken *First = TheLine.First; - CharSourceRange LineRange = CharSourceRange::getCharRange( - First->WhitespaceRange.getBegin(), - First->WhitespaceRange.getBegin().getLocWithOffset( - First->LastNewlineOffset)); - return touchesRanges(LineRange); - } - /// \brief Analyze the entire solution space starting from \p InitialState. /// /// This implements a variant of Dijkstra's algorithm on the graph that spans @@ -1005,8 +948,6 @@ private: return true; } - SourceManager &SourceMgr; - SmallVectorImpl &Ranges; ContinuationIndenter *Indenter; WhitespaceManager *Whitespaces; FormatStyle Style; @@ -1310,17 +1251,114 @@ public: for (unsigned i = 0, e = AnnotatedLines.size(); i != e; ++i) { Annotator.calculateFormattingInformation(*AnnotatedLines[i]); } + computeAffectedLines(AnnotatedLines.begin(), AnnotatedLines.end()); Annotator.setCommentLineLevels(AnnotatedLines); ContinuationIndenter Indenter(Style, SourceMgr, Whitespaces, Encoding, BinPackInconclusiveFunctions); - UnwrappedLineFormatter Formatter(SourceMgr, Ranges, &Indenter, &Whitespaces, - Style); + UnwrappedLineFormatter Formatter(&Indenter, &Whitespaces, Style); Formatter.format(AnnotatedLines, /*DryRun=*/false); return Whitespaces.generateReplacements(); } private: + // Determines which lines are affected by the SourceRanges given as input. + // Returns \c true if at least one line between I and E was affected. + bool computeAffectedLines(SmallVectorImpl::iterator I, + SmallVectorImpl::iterator E) { + bool SomeLineAffected = false; + bool PreviousLineAffected = false; + while (I != E) { + AnnotatedLine *Line = *I; + Line->LeadingEmptyLinesAffected = affectsLeadingEmptyLines(*Line->First); + + // If a line is part of a preprocessor directive, it needs to be formatted + // if any token within the directive is affected. + if (Line->InPPDirective) { + FormatToken *Last = Line->Last; + SmallVectorImpl::iterator PPEnd = I + 1; + while (PPEnd != E && !(*PPEnd)->First->HasUnescapedNewline) { + Last = (*PPEnd)->Last; + ++PPEnd; + } + + if (affectsTokenRange(*Line->First, *Last, + /*IncludeLeadingNewlines=*/false)) { + SomeLineAffected = true; + markAllAsAffected(I, PPEnd); + } + I = PPEnd; + continue; + } + + bool SomeTokenAffected = false; + for (FormatToken *Tok = Line->First; Tok; Tok = Tok->Next) { + bool IncludeLeadingNewlines = Tok != Line->First; + if (affectsTokenRange(*Tok, *Tok, IncludeLeadingNewlines)) { + SomeTokenAffected = true; + break; + } + } + bool SomeChildAffected = + computeAffectedLines(Line->Children.begin(), Line->Children.end()); + bool LineMoved = PreviousLineAffected && Line->First->NewlinesBefore == 0; + if (SomeTokenAffected || SomeChildAffected || LineMoved) { + Line->Affected = true; + SomeLineAffected = true; + PreviousLineAffected = true; + } else { + PreviousLineAffected = false; + } + + ++I; + } + return SomeLineAffected; + } + + // Marks all lines between I and E as well as all their children as affected. + void markAllAsAffected(SmallVectorImpl::iterator I, + SmallVectorImpl::iterator E) { + while (I != E) { + (*I)->Affected = true; + markAllAsAffected((*I)->Children.begin(), (*I)->Children.end()); + ++I; + } + } + + // Returns true if the range from 'First' to 'Last' intersects with one of the + // input ranges. + bool affectsTokenRange(const FormatToken &First, const FormatToken &Last, + bool IncludeLeadingNewlines) { + SourceLocation Start = First.WhitespaceRange.getBegin(); + if (!IncludeLeadingNewlines) + Start = Start.getLocWithOffset(First.LastNewlineOffset); + SourceLocation End = Last.getStartOfNonWhitespace().getLocWithOffset( + Last.TokenText.size() - 1); + CharSourceRange Range = CharSourceRange::getCharRange(Start, End); + return affectsCharSourceRange(Range); + } + + // Returns true if one of the input ranges intersect the leading empty lines + // before 'Tok'. + bool affectsLeadingEmptyLines(const FormatToken &Tok) { + CharSourceRange EmptyLineRange = CharSourceRange::getCharRange( + Tok.WhitespaceRange.getBegin(), + Tok.WhitespaceRange.getBegin().getLocWithOffset(Tok.LastNewlineOffset)); + return affectsCharSourceRange(EmptyLineRange); + } + + // Returns true if 'Range' intersects with one of the input ranges. + bool affectsCharSourceRange(const CharSourceRange &Range) { + for (SmallVectorImpl::const_iterator I = Ranges.begin(), + E = Ranges.end(); + I != E; ++I) { + if (!SourceMgr.isBeforeInTranslationUnit(Range.getEnd(), I->getBegin()) && + !SourceMgr.isBeforeInTranslationUnit(I->getEnd(), Range.getBegin())) + return true; + } + return false; + } + static bool inputUsesCRLF(StringRef Text) { return Text.count('\r') * 2 > Text.count('\n'); } diff --git a/lib/Format/TokenAnnotator.h b/lib/Format/TokenAnnotator.h index aa49b2a5c0..8bdf758bc1 100644 --- a/lib/Format/TokenAnnotator.h +++ b/lib/Format/TokenAnnotator.h @@ -41,7 +41,8 @@ public: : First(Line.Tokens.front().Tok), Level(Line.Level), InPPDirective(Line.InPPDirective), MustBeDeclaration(Line.MustBeDeclaration), MightBeFunctionDecl(false), - StartsDefinition(false) { + StartsDefinition(false), Affected(false), + LeadingEmptyLinesAffected(false) { assert(!Line.Tokens.empty()); // Calculate Next and Previous for all tokens. Note that we must overwrite @@ -87,6 +88,14 @@ public: bool MightBeFunctionDecl; bool StartsDefinition; + /// \c True if this line should be formatted, i.e. intersects directly or + /// indirectly with one of the input ranges. + bool Affected; + + /// \c True if the leading empty lines of this line intersect with one of the + /// input ranges. + bool LeadingEmptyLinesAffected; + private: // Disallow copying. AnnotatedLine(const AnnotatedLine &) LLVM_DELETED_FUNCTION; diff --git a/unittests/Format/FormatTest.cpp b/unittests/Format/FormatTest.cpp index d61dfc44d9..fc0e935037 100644 --- a/unittests/Format/FormatTest.cpp +++ b/unittests/Format/FormatTest.cpp @@ -187,10 +187,10 @@ TEST_F(FormatTest, FormatsCorrectRegionForLeadingWhitespace) { 26, 0, getLLVMStyleWithColumns(12))); EXPECT_EQ("#define A \\\n" " int a; \\\n" - " int b;", + " int b;", format("#define A \\\n" " int a; \\\n" - " int b;", + " int b;", 25, 0, getLLVMStyleWithColumns(12))); } -- 2.40.0