[cfe-commits] r172604 - in /cfe/trunk/lib/Format: Format.cpp UnwrappedLineParser.h
NAKAMURA Takumi
geek4civic at gmail.com
Sun Jan 20 07:07:53 PST 2013
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
More information about the cfe-commits
mailing list