[cfe-commits] r172604 - in /cfe/trunk/lib/Format: Format.cpp UnwrappedLineParser.h
Daniel Jasper
djasper at google.com
Mon Jan 21 06:22:03 PST 2013
Fixed in r173043. Sorry for the trouble..
On Sun, Jan 20, 2013 at 4:07 PM, NAKAMURA Takumi <geek4civic at gmail.com>wrote:
> Daniel,
>
> It causes an error in valgrind on
> FormatTest.DoesNotTouchUnwrappedLinesWithErrors.
> http://lab.llvm.org:8011/builders/clang-x86_64-linux-vg/builds/643
>
> In nextTwoLinesFitInto(),
> (I + 2)->Last->TotalLength is uninitialized.
>
> Please take a look.
>
> ...Takumi
>
> 2013/1/16 Daniel Jasper <djasper at google.com>:
> > Author: djasper
> > Date: Wed Jan 16 04:41:46 2013
> > New Revision: 172604
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=172604&view=rev
> > Log:
> > Calculate the total length of a line up to each token up front.
> >
> > This makes the tedious fitsIntoLimit() method unnecessary and I can
> > replace one hack (constructor initializers) by a slightly better hack.
> >
> > Furthermore, this will enable calculating whether a certain part of a
> > line fits into the limit for future modifications.
> >
> > Modified:
> > cfe/trunk/lib/Format/Format.cpp
> > cfe/trunk/lib/Format/UnwrappedLineParser.h
> >
> > Modified: cfe/trunk/lib/Format/Format.cpp
> > URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/Format.cpp?rev=172604&r1=172603&r2=172604&view=diff
> >
> ==============================================================================
> > --- cfe/trunk/lib/Format/Format.cpp (original)
> > +++ cfe/trunk/lib/Format/Format.cpp Wed Jan 16 04:41:46 2013
> > @@ -86,6 +86,9 @@
> >
> > bool ClosesTemplateDeclaration;
> >
> > + /// \brief The total length of the line up to and including this
> token.
> > + unsigned TotalLength;
> > +
> > std::vector<AnnotatedToken> Children;
> > AnnotatedToken *Parent;
> > };
> > @@ -207,31 +210,6 @@
> > NewLineText +
> std::string(Spaces, ' ')));
> > }
> >
> > -/// \brief Calculates whether the (remaining) \c AnnotatedLine starting
> with
> > -/// \p RootToken fits into \p Limit columns on a single line.
> > -///
> > -/// If true, sets \p Length to the required length.
> > -static bool fitsIntoLimit(const AnnotatedToken &RootToken, unsigned
> Limit,
> > - unsigned *Length = 0) {
> > - unsigned Columns = RootToken.FormatTok.TokenLength;
> > - if (Columns > Limit) return false;
> > - const AnnotatedToken *Tok = &RootToken;
> > - while (!Tok->Children.empty()) {
> > - Tok = &Tok->Children[0];
> > - Columns += (Tok->SpaceRequiredBefore ? 1 : 0) +
> Tok->FormatTok.TokenLength;
> > - // A special case for the colon of a constructor initializer as
> this only
> > - // needs to be put on a new line if the line needs to be split.
> > - if (Columns > Limit ||
> > - (Tok->MustBreakBefore && Tok->Type != TT_CtorInitializerColon))
> {
> > - // FIXME: Remove this hack.
> > - return false;
> > - }
> > - }
> > - if (Length != 0)
> > - *Length = Columns;
> > - return true;
> > -}
> > -
> > /// \brief Returns if a token is an Objective-C selector name.
> > ///
> > /// For example, "bar" is a selector name in [foo bar:(4 + 5)].
> > @@ -245,11 +223,10 @@
> > public:
> > UnwrappedLineFormatter(const FormatStyle &Style, SourceManager
> &SourceMgr,
> > const AnnotatedLine &Line, unsigned
> FirstIndent,
> > - bool FitsOnALine, const AnnotatedToken
> &RootToken,
> > + const AnnotatedToken &RootToken,
> > tooling::Replacements &Replaces, bool
> StructuralError)
> > : Style(Style), SourceMgr(SourceMgr), Line(Line),
> > - FirstIndent(FirstIndent), FitsOnALine(FitsOnALine),
> > - RootToken(RootToken), Replaces(Replaces) {
> > + FirstIndent(FirstIndent), RootToken(RootToken),
> Replaces(Replaces) {
> > Parameters.PenaltyIndentLevel = 15;
> > Parameters.PenaltyLevelDecrease = 30;
> > Parameters.PenaltyExcessCharacter = 1000000;
> > @@ -280,7 +257,7 @@
> > // cannot continue an top-level implicit string literal on the
> next
> > // line.
> > return 0;
> > - if (FitsOnALine) {
> > + if (Line.Last->TotalLength <= getColumnLimit() - FirstIndent) {
> > addTokenToState(false, false, State);
> > } else {
> > unsigned NoBreak = calcPenalty(State, false, UINT_MAX);
> > @@ -289,8 +266,7 @@
> > if (State.NextToken != NULL &&
> > State.NextToken->Parent->Type == TT_CtorInitializerColon) {
> > if (Style.ConstructorInitializerAllOnOneLineOrOnePerLine &&
> > - !fitsIntoLimit(*State.NextToken,
> > - getColumnLimit() - State.Column - 1))
> > + Line.Last->TotalLength > getColumnLimit() - State.Column
> - 1)
> > State.Stack.back().BreakAfterComma = true;
> > }
> > }
> > @@ -607,6 +583,8 @@
> > State.NextToken->Type != TT_LineComment &&
> > State.Stack.back().BreakAfterComma)
> > return UINT_MAX;
> > + if (!NewLine && State.NextToken->Type == TT_CtorInitializerColon)
> > + return UINT_MAX;
> >
> > unsigned CurrentPenalty = 0;
> > if (NewLine) {
> > @@ -658,7 +636,6 @@
> > SourceManager &SourceMgr;
> > const AnnotatedLine &Line;
> > const unsigned FirstIndent;
> > - const bool FitsOnALine;
> > const AnnotatedToken &RootToken;
> > tooling::Replacements &Replaces;
> >
> > @@ -974,8 +951,7 @@
> > } else {
> > if (Current.Type == TT_LineComment) {
> > Current.MustBreakBefore = Current.FormatTok.NewlinesBefore > 0;
> > - } else if (Current.Type == TT_CtorInitializerColon ||
> > - Current.Parent->Type == TT_LineComment ||
> > + } else if (Current.Parent->Type == TT_LineComment ||
> > (Current.is(tok::string_literal) &&
> > Current.Parent->is(tok::string_literal))) {
> > Current.MustBreakBefore = true;
> > @@ -984,6 +960,12 @@
> > }
> > }
> > Current.CanBreakBefore = Current.MustBreakBefore ||
> canBreakBefore(Current);
> > + if (Current.MustBreakBefore)
> > + Current.TotalLength = Current.Parent->TotalLength +
> Style.ColumnLimit;
> > + else
> > + Current.TotalLength = Current.Parent->TotalLength +
> > + Current.FormatTok.TokenLength +
> > + (Current.SpaceRequiredBefore ? 1 : 0);
> > if (!Current.Children.empty())
> > calculateExtraInformation(Current.Children[0]);
> > }
> > @@ -1007,6 +989,7 @@
> > Line.First.MustBreakBefore = Line.First.FormatTok.MustBreakBefore;
> > Line.First.CanBreakBefore = Line.First.MustBreakBefore;
> >
> > + Line.First.TotalLength = Line.First.FormatTok.TokenLength;
> > if (!Line.First.Children.empty())
> > calculateExtraInformation(Line.First.Children[0]);
> > }
> > @@ -1450,9 +1433,9 @@
> > unsigned Indent = formatFirstToken(TheLine.First, TheLine.Level,
> > TheLine.InPPDirective,
> > PreviousEndOfLineColumn);
> > - bool FitsOnALine = tryFitMultipleLinesInOne(Indent, I, E);
> > + tryFitMultipleLinesInOne(Indent, I, E);
> > UnwrappedLineFormatter Formatter(Style, SourceMgr, TheLine,
> Indent,
> > - FitsOnALine, TheLine.First,
> Replaces,
> > + TheLine.First, Replaces,
> > StructuralError);
> > PreviousEndOfLineColumn = Formatter.format();
> > } else {
> > @@ -1477,27 +1460,24 @@
> > /// if possible; note that \c I will be incremented when lines are
> merged.
> > ///
> > /// Returns whether the resulting \c Line can fit in a single line.
> > - bool tryFitMultipleLinesInOne(unsigned Indent,
> > + void tryFitMultipleLinesInOne(unsigned Indent,
> > std::vector<AnnotatedLine>::iterator &I,
> > std::vector<AnnotatedLine>::iterator E)
> {
> > unsigned Limit = Style.ColumnLimit - (I->InPPDirective ? 1 : 0) -
> Indent;
> >
> > + // We can never merge stuff if there are trailing line comments.
> > + if (I->Last->Type == TT_LineComment)
> > + return;
> > +
> > // Check whether the UnwrappedLine can be put onto a single line. If
> > // so, this is bound to be the optimal solution (by definition) and
> we
> > // don't need to analyze the entire solution space.
> > - unsigned Length = 0;
> > - if (!fitsIntoLimit(I->First, Limit, &Length))
> > - return false;
> > -
> > - // We can never merge stuff if there are trailing line comments.
> > - if (I->Last->Type == TT_LineComment)
> > - return true;
> > + if (I->Last->TotalLength >= Limit)
> > + return;
> > + Limit -= I->Last->TotalLength + 1; // One space.
> >
> > - if (Limit == Length)
> > - return true; // Couldn't fit a space.
> > - Limit -= Length + 1; // One space.
> > if (I + 1 == E)
> > - return true;
> > + return;
> >
> > if (I->Last->is(tok::l_brace)) {
> > tryMergeSimpleBlock(I, E, Limit);
> > @@ -1507,7 +1487,7 @@
> > I->First.FormatTok.IsFirst)) {
> > tryMergeSimplePPDirective(I, E, Limit);
> > }
> > - return true;
> > + return;
> > }
> >
> > void tryMergeSimplePPDirective(std::vector<AnnotatedLine>::iterator
> &I,
> > @@ -1519,7 +1499,8 @@
> > if (I + 2 != E && (I + 2)->InPPDirective &&
> > !(I + 2)->First.FormatTok.HasUnescapedNewline)
> > return;
> > - if (!fitsIntoLimit((I + 1)->First, Limit)) return;
> > + if ((I + 1)->Last->TotalLength > Limit)
> > + return;
> > join(Line, *(++I));
> > }
> >
> > @@ -1531,7 +1512,7 @@
> > AnnotatedLine &Line = *I;
> > if (Line.Last->isNot(tok::r_paren))
> > return;
> > - if (!fitsIntoLimit((I + 1)->First, Limit))
> > + if ((I + 1)->Last->TotalLength > Limit)
> > return;
> > if ((I + 1)->First.is(tok::kw_if) || (I + 1)->First.Type ==
> TT_LineComment)
> > return;
> > @@ -1594,12 +1575,7 @@
> >
> > bool nextTwoLinesFitInto(std::vector<AnnotatedLine>::iterator I,
> > unsigned Limit) {
> > - unsigned LengthLine1 = 0;
> > - unsigned LengthLine2 = 0;
> > - if (!fitsIntoLimit((I + 1)->First, Limit, &LengthLine1) ||
> > - !fitsIntoLimit((I + 2)->First, Limit, &LengthLine2))
> > - return false;
> > - return LengthLine1 + LengthLine2 + 1 <= Limit; // One space.
> > + return (I + 1)->Last->TotalLength + 1 + (I + 2)->Last->TotalLength
> <= Limit;
> > }
> >
> > void join(AnnotatedLine &A, const AnnotatedLine &B) {
> >
> > Modified: cfe/trunk/lib/Format/UnwrappedLineParser.h
> > URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/UnwrappedLineParser.h?rev=172604&r1=172603&r2=172604&view=diff
> >
> ==============================================================================
> > --- cfe/trunk/lib/Format/UnwrappedLineParser.h (original)
> > +++ cfe/trunk/lib/Format/UnwrappedLineParser.h Wed Jan 16 04:41:46 2013
> > @@ -88,6 +88,7 @@
> > UnwrappedLine() : Level(0), InPPDirective(false) {
> > }
> >
> > + // FIXME: Don't use std::list here.
> > /// \brief The \c Tokens comprising this \c UnwrappedLine.
> > std::list<FormatToken> Tokens;
> >
> >
> >
> > _______________________________________________
> > cfe-commits mailing list
> > cfe-commits at cs.uiuc.edu
> > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130121/d5721496/attachment.html>
More information about the cfe-commits
mailing list