r190038 - clang-format: Enable formatting of nested blocks.
Manuel Klimek
klimek at google.com
Thu Sep 5 03:33:00 PDT 2013
On Thu, Sep 5, 2013 at 11:29 AM, Daniel Jasper <djasper at google.com> wrote:
> Author: djasper
> Date: Thu Sep 5 04:29:45 2013
> New Revision: 190038
>
> URL: http://llvm.org/viewvc/llvm-project?rev=190038&view=rev
> Log:
> clang-format: Enable formatting of nested blocks.
>
> Among other things, this enables (better) formatting lambdas and
> constructs like:
> MACRO({
> long_statement();
> long_statement_2();
> },
> {
> long_statement();
> long_statement_2();
> },
> { short_statement(); }, "");
>
> This fixes llvm.org/PR15381.
>
> Modified:
> cfe/trunk/lib/Format/ContinuationIndenter.cpp
> cfe/trunk/lib/Format/ContinuationIndenter.h
> cfe/trunk/lib/Format/Format.cpp
> cfe/trunk/lib/Format/FormatToken.h
> cfe/trunk/lib/Format/TokenAnnotator.cpp
> cfe/trunk/lib/Format/TokenAnnotator.h
> cfe/trunk/lib/Format/UnwrappedLineParser.cpp
> cfe/trunk/lib/Format/UnwrappedLineParser.h
> cfe/trunk/unittests/Format/FormatTest.cpp
>
> Modified: cfe/trunk/lib/Format/ContinuationIndenter.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/ContinuationIndenter.cpp?rev=190038&r1=190037&r2=190038&view=diff
>
> ==============================================================================
> --- cfe/trunk/lib/Format/ContinuationIndenter.cpp (original)
> +++ cfe/trunk/lib/Format/ContinuationIndenter.cpp Thu Sep 5 04:29:45 2013
> @@ -55,20 +55,20 @@ static bool startsSegmentOfBuilderTypeCa
>
> ContinuationIndenter::ContinuationIndenter(const FormatStyle &Style,
> SourceManager &SourceMgr,
> - const AnnotatedLine &Line,
> - unsigned FirstIndent,
> WhitespaceManager &Whitespaces,
> encoding::Encoding Encoding,
> bool
> BinPackInconclusiveFunctions)
> - : Style(Style), SourceMgr(SourceMgr), Line(Line),
> FirstIndent(FirstIndent),
> - Whitespaces(Whitespaces), Encoding(Encoding),
> + : Style(Style), SourceMgr(SourceMgr), Whitespaces(Whitespaces),
> + Encoding(Encoding),
> BinPackInconclusiveFunctions(BinPackInconclusiveFunctions) {}
>
> -LineState ContinuationIndenter::getInitialState() {
> - // Initialize state dependent on indent.
> +LineState ContinuationIndenter::getInitialState(unsigned FirstIndent,
> + const AnnotatedLine
> *Line) {
> LineState State;
> + State.FirstIndent = FirstIndent;
> State.Column = FirstIndent;
> - State.NextToken = Line.First;
> + State.Line = Line;
> + State.NextToken = Line->First;
> State.Stack.push_back(ParenState(FirstIndent, FirstIndent,
> /*AvoidBinPacking=*/false,
> /*NoLineBreak=*/false));
> @@ -95,7 +95,7 @@ bool ContinuationIndenter::canBreak(cons
> // The opening "{" of a braced list has to be on the same line as the
> first
> // element if it is nested in another braced init list or function call.
> if (!Current.MustBreakBefore && Previous.is(tok::l_brace) &&
> - Previous.Previous &&
> + Previous.BlockKind == BK_BracedInit && Previous.Previous &&
> Previous.Previous->isOneOf(tok::l_brace, tok::l_paren, tok::comma))
> return false;
> // This prevents breaks like:
> @@ -183,8 +183,8 @@ bool ContinuationIndenter::mustBreak(con
> return true;
>
> if ((Current.Type == TT_StartOfName || Current.is(tok::kw_operator)) &&
> - Line.MightBeFunctionDecl && State.Stack.back().BreakBeforeParameter
> &&
> - State.ParenLevel == 0)
> + State.Line->MightBeFunctionDecl &&
> + State.Stack.back().BreakBeforeParameter && State.ParenLevel == 0)
> return true;
> if (startsSegmentOfBuilderTypeCall(Current) &&
> (State.Stack.back().CallContinuation != 0 ||
> @@ -234,10 +234,7 @@ unsigned ContinuationIndenter::addTokenT
> Penalty += Style.PenaltyBreakFirstLessLess;
>
> if (Current.is(tok::r_brace)) {
> - if (Current.BlockKind == BK_BracedInit)
> - State.Column = State.Stack[State.Stack.size() - 2].LastSpace;
> - else
> - State.Column = FirstIndent;
> + State.Column = State.Stack[State.Stack.size() - 2].LastSpace;
> } else if (Current.is(tok::string_literal) &&
> State.StartOfStringLiteral != 0) {
> State.Column = State.StartOfStringLiteral;
> @@ -261,7 +258,7 @@ unsigned ContinuationIndenter::addTokenT
> Current.is(tok::kw_operator)) &&
> State.ParenLevel == 0 &&
> (!Style.IndentFunctionDeclarationAfterType ||
> - Line.StartsDefinition))) {
> + State.Line->StartsDefinition))) {
> State.Column = State.Stack.back().Indent;
> } else if (Current.Type == TT_ObjCSelectorName) {
> if (State.Stack.back().ColonPos > Current.CodePointCount) {
> @@ -280,14 +277,15 @@ unsigned ContinuationIndenter::addTokenT
> Previous.Type == TT_ObjCMethodExpr) {
> State.Column = ContinuationIndent;
> } else if (Current.Type == TT_CtorInitializerColon) {
> - State.Column = FirstIndent +
> Style.ConstructorInitializerIndentWidth;
> + State.Column =
> + State.FirstIndent + Style.ConstructorInitializerIndentWidth;
> } else if (Current.Type == TT_CtorInitializerComma) {
> State.Column = State.Stack.back().Indent;
> } else {
> State.Column = State.Stack.back().Indent;
> // Ensure that we fall back to indenting 4 spaces instead of just
> // flushing continuations left.
> - if (State.Column == FirstIndent)
> + if (State.Column == State.FirstIndent)
> State.Column += 4;
> }
>
> @@ -306,7 +304,7 @@ unsigned ContinuationIndenter::addTokenT
> NewLines = std::max(NewLines, std::min(Current.NewlinesBefore,
> Style.MaxEmptyLinesToKeep
> + 1));
> Whitespaces.replaceWhitespace(Current, NewLines, State.Column,
> - State.Column, Line.InPPDirective);
> + State.Column,
> State.Line->InPPDirective);
> }
>
> if (!Current.isTrailingComment())
> @@ -337,13 +335,13 @@ unsigned ContinuationIndenter::addTokenT
> if (!(Previous.isOneOf(tok::l_paren, tok::l_brace) ||
> Previous.Type == TT_BinaryOperator) ||
> (!Style.AllowAllParametersOfDeclarationOnNextLine &&
> - Line.MustBeDeclaration))
> + State.Line->MustBeDeclaration))
> State.Stack.back().BreakBeforeParameter = true;
> }
>
> } else {
> if (Current.is(tok::equal) &&
> - (Line.First->is(tok::kw_for) || State.ParenLevel == 0) &&
> + (State.Line->First->is(tok::kw_for) || State.ParenLevel == 0) &&
> State.Stack.back().VariablePos == 0) {
> State.Stack.back().VariablePos = State.Column;
> // Move over * and & if they are bound to the variable name.
> @@ -403,21 +401,18 @@ unsigned ContinuationIndenter::addTokenT
> else if (Previous.Type == TT_InheritanceColon)
> State.Stack.back().Indent = State.Column;
> else if (Previous.opensScope()) {
> - // If a function has multiple parameters (including a single
> parameter
> - // that is a binary expression) or a trailing call, indent all
> - // parameters from the opening parenthesis. This avoids confusing
> - // indents like:
> - // OuterFunction(InnerFunctionCall(
> - // ParameterToInnerFunction),
> - // SecondParameterToOuterFunction);
> + // If a function has a trailing call, indent all parameters from the
> + // opening parenthesis. This avoids confusing indents like:
> + // OuterFunction(InnerFunctionCall( // break
> + // ParameterToInnerFunction)) // break
> + // .SecondInnerFunctionCall();
> bool HasTrailingCall = false;
> if (Previous.MatchingParen) {
> const FormatToken *Next =
> Previous.MatchingParen->getNextNonComment();
> HasTrailingCall = Next && Next->isMemberAccess();
> }
> - if (startsBinaryExpression(Current) ||
> - (HasTrailingCall &&
> - State.Stack[State.Stack.size() - 2].CallContinuation == 0))
> + if (HasTrailingCall &&
> + State.Stack[State.Stack.size() - 2].CallContinuation == 0)
> State.Stack.back().LastSpace = State.Column;
> }
> }
> @@ -434,7 +429,7 @@ unsigned ContinuationIndenter::moveState
> State.Stack.back().AvoidBinPacking = true;
> if (Current.is(tok::lessless) && State.Stack.back().FirstLessLess == 0)
> State.Stack.back().FirstLessLess = State.Column;
> - if (Current.is(tok::l_square) &&
> + if (Current.is(tok::l_square) && Current.Type != TT_LambdaLSquare &&
> State.Stack.back().StartOfArraySubscripts == 0)
> State.Stack.back().StartOfArraySubscripts = State.Column;
> if (Current.is(tok::question))
> @@ -485,6 +480,14 @@ unsigned ContinuationIndenter::moveState
> NewParenState.Indent =
> std::max(std::max(State.Column, NewParenState.Indent),
> State.Stack.back().LastSpace);
> + // Do not indent relative to the fake parentheses inserted for "." or
> "->".
> + // This is a special case to make the following to statements
> consistent:
> + // OuterFunction(InnerFunctionCall( // break
> + // ParameterToInnerFunction));
> + // OuterFunction(SomeObject.InnerFunctionCall( // break
> + // ParameterToInnerFunction));
> + if (*I > prec::Unknown)
> + NewParenState.LastSpace = std::max(NewParenState.LastSpace,
> State.Column);
>
> // Always indent conditional expressions. Never indent expression
> where
> // the 'operator' is ',', ';' or an assignment (i.e. *I <=
> @@ -504,17 +507,22 @@ unsigned ContinuationIndenter::moveState
> // prepare for the following tokens.
> if (Current.opensScope()) {
> unsigned NewIndent;
> - unsigned LastSpace = State.Stack.back().LastSpace;
> bool AvoidBinPacking;
> if (Current.is(tok::l_brace)) {
> - NewIndent =
> - LastSpace + (Style.Cpp11BracedListStyle ? 4 :
> Style.IndentWidth);
> + if (Current.MatchingParen && Current.BlockKind == BK_Block) {
> + for (unsigned i = 0; i != Current.MatchingParen->FakeRParens; ++i)
> + State.Stack.pop_back();
>
Can you please add a comment why this is ok / necessary here :)
> + NewIndent = State.Stack.back().LastSpace;
> + } else {
> + NewIndent = State.Stack.back().LastSpace +
> + (Style.Cpp11BracedListStyle ? 4 : Style.IndentWidth);
> + }
> const FormatToken *NextNoComment = Current.getNextNonComment();
> AvoidBinPacking = NextNoComment &&
> NextNoComment->Type ==
> TT_DesignatedInitializerPeriod;
> } else {
> - NewIndent =
> - 4 + std::max(LastSpace, State.Stack.back().StartOfFunctionCall);
> + NewIndent = 4 + std::max(State.Stack.back().LastSpace,
> + State.Stack.back().StartOfFunctionCall);
> AvoidBinPacking = !Style.BinPackParameters ||
> (Style.ExperimentalAutoDetectBinPacking &&
> (Current.PackingKind == PPK_OnePerLine ||
> @@ -522,7 +530,8 @@ unsigned ContinuationIndenter::moveState
> Current.PackingKind == PPK_Inconclusive)));
> }
>
> - State.Stack.push_back(ParenState(NewIndent, LastSpace,
> AvoidBinPacking,
> + State.Stack.push_back(ParenState(NewIndent,
> State.Stack.back().LastSpace,
> + AvoidBinPacking,
> State.Stack.back().NoLineBreak));
> ++State.ParenLevel;
> }
> @@ -531,7 +540,8 @@ unsigned ContinuationIndenter::moveState
> // one line and put one per line if they don't.
> if (Current.is(tok::l_square) && Current.Type == TT_ObjCMethodExpr &&
> Current.MatchingParen != NULL) {
> - if (getLengthToMatchingParen(Current) + State.Column >
> getColumnLimit())
> + if (getLengthToMatchingParen(Current) + State.Column >
> + getColumnLimit(State))
> State.Stack.back().BreakBeforeParameter = true;
> }
>
> @@ -539,7 +549,7 @@ unsigned ContinuationIndenter::moveState
> // stacks.
> if (State.Stack.size() > 1 &&
> (Current.isOneOf(tok::r_paren, tok::r_square) ||
> - (Current.is(tok::r_brace) && State.NextToken != Line.First) ||
> + (Current.is(tok::r_brace) && State.NextToken != State.Line->First)
> ||
> State.NextToken->Type == TT_TemplateCloser)) {
> State.Stack.pop_back();
> --State.ParenLevel;
> @@ -552,10 +562,13 @@ unsigned ContinuationIndenter::moveState
> }
>
> // Remove scopes created by fake parenthesis.
> - for (unsigned i = 0, e = Current.FakeRParens; i != e; ++i) {
> - unsigned VariablePos = State.Stack.back().VariablePos;
> - State.Stack.pop_back();
> - State.Stack.back().VariablePos = VariablePos;
> + if (Current.isNot(tok::r_brace) ||
> + (Current.MatchingParen && Current.MatchingParen->BlockKind !=
> BK_Block)) {
> + for (unsigned i = 0, e = Current.FakeRParens; i != e; ++i) {
> + unsigned VariablePos = State.Stack.back().VariablePos;
> + State.Stack.pop_back();
> + State.Stack.back().VariablePos = VariablePos;
> + }
> }
>
> if (Current.is(tok::string_literal) && State.StartOfStringLiteral == 0)
> {
> @@ -568,6 +581,10 @@ unsigned ContinuationIndenter::moveState
> State.Column += Current.CodePointCount;
> State.NextToken = State.NextToken->Next;
> unsigned Penalty = breakProtrudingToken(Current, State, DryRun);
> + if (State.Column > getColumnLimit(State)) {
> + unsigned ExcessCharacters = State.Column - getColumnLimit(State);
> + Penalty += Style.PenaltyExcessCharacter * ExcessCharacters;
> + }
>
> // If the previous has a special role, let it consume tokens as
> appropriate.
> // It is necessary to start at the previous token for the only
> implemented
> @@ -593,8 +610,8 @@ ContinuationIndenter::addMultilineString
> // for all other lines is constant, and we ignore it.
> State.Column = Current.CodePointsInLastLine;
>
> - if (ColumnsUsed > getColumnLimit())
> - return Style.PenaltyExcessCharacter * (ColumnsUsed -
> getColumnLimit());
> + if (ColumnsUsed > getColumnLimit(State))
> + return Style.PenaltyExcessCharacter * (ColumnsUsed -
> getColumnLimit(State));
> return 0;
> }
>
> @@ -623,15 +640,15 @@ unsigned ContinuationIndenter::breakProt
> if (Current.IsUnterminatedLiteral)
> return 0;
>
> - Token.reset(new BreakableStringLiteral(Current, StartColumn,
> - Line.InPPDirective, Encoding));
> + Token.reset(new BreakableStringLiteral(
> + Current, StartColumn, State.Line->InPPDirective, Encoding));
> } else if (Current.Type == TT_BlockComment &&
> Current.isTrailingComment()) {
> unsigned OriginalStartColumn =
>
> SourceMgr.getSpellingColumnNumber(Current.getStartOfNonWhitespace()) -
> 1;
> Token.reset(new BreakableBlockComment(
> Style, Current, StartColumn, OriginalStartColumn,
> !Current.Previous,
> - Line.InPPDirective, Encoding));
> + State.Line->InPPDirective, Encoding));
> } else if (Current.Type == TT_LineComment &&
> (Current.Previous == NULL ||
> Current.Previous->Type != TT_ImplicitStringLiteral)) {
> @@ -648,14 +665,15 @@ unsigned ContinuationIndenter::breakProt
> }
>
> Token.reset(new BreakableLineComment(Current, StartColumn,
> - Line.InPPDirective, Encoding));
> + State.Line->InPPDirective,
> Encoding));
> } else {
> return 0;
> }
> - if (Current.UnbreakableTailLength >= getColumnLimit())
> + if (Current.UnbreakableTailLength >= getColumnLimit(State))
> return 0;
>
> - unsigned RemainingSpace = getColumnLimit() -
> Current.UnbreakableTailLength;
> + unsigned RemainingSpace =
> + getColumnLimit(State) - Current.UnbreakableTailLength;
> bool BreakInserted = false;
> unsigned Penalty = 0;
> unsigned RemainingTokenColumns = 0;
> @@ -668,7 +686,7 @@ unsigned ContinuationIndenter::breakProt
> Token->getLineLengthAfterSplit(LineIndex, TailOffset,
> StringRef::npos);
> while (RemainingTokenColumns > RemainingSpace) {
> BreakableToken::Split Split =
> - Token->getSplit(LineIndex, TailOffset, getColumnLimit());
> + Token->getSplit(LineIndex, TailOffset, getColumnLimit(State));
> if (Split.first == StringRef::npos) {
> // The last line's penalty is handled in addNextStateToQueue().
> if (LineIndex < EndIndex - 1)
> @@ -685,9 +703,9 @@ unsigned ContinuationIndenter::breakProt
> Penalty += Current.SplitPenalty;
> unsigned ColumnsUsed =
> Token->getLineLengthAfterSplit(LineIndex, TailOffset,
> Split.first);
> - if (ColumnsUsed > getColumnLimit()) {
> - Penalty +=
> - Style.PenaltyExcessCharacter * (ColumnsUsed -
> getColumnLimit());
> + if (ColumnsUsed > getColumnLimit(State)) {
> + Penalty += Style.PenaltyExcessCharacter *
> + (ColumnsUsed - getColumnLimit(State));
> }
> TailOffset += Split.first + Split.second;
> RemainingTokenColumns = NewRemainingTokenColumns;
> @@ -714,9 +732,9 @@ unsigned ContinuationIndenter::breakProt
> return Penalty;
> }
>
> -unsigned ContinuationIndenter::getColumnLimit() const {
> +unsigned ContinuationIndenter::getColumnLimit(const LineState &State)
> const {
> // In preprocessor directives reserve two chars for trailing " \"
> - return Style.ColumnLimit - (Line.InPPDirective ? 2 : 0);
> + return Style.ColumnLimit - (State.Line->InPPDirective ? 2 : 0);
> }
>
> bool ContinuationIndenter::NextIsMultilineString(const LineState &State) {
>
> Modified: cfe/trunk/lib/Format/ContinuationIndenter.h
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/ContinuationIndenter.h?rev=190038&r1=190037&r2=190038&view=diff
>
> ==============================================================================
> --- cfe/trunk/lib/Format/ContinuationIndenter.h (original)
> +++ cfe/trunk/lib/Format/ContinuationIndenter.h Thu Sep 5 04:29:45 2013
> @@ -35,14 +35,13 @@ public:
> /// \brief Constructs a \c ContinuationIndenter to format \p Line
> starting in
> /// column \p FirstIndent.
> ContinuationIndenter(const FormatStyle &Style, SourceManager &SourceMgr,
> - const AnnotatedLine &Line, unsigned FirstIndent,
> WhitespaceManager &Whitespaces,
> encoding::Encoding Encoding,
> bool BinPackInconclusiveFunctions);
>
> - /// \brief Get the initial state, i.e. the state after placing the
> line's
> - /// first token.
> - LineState getInitialState();
> + /// \brief Get the initial state, i.e. the state after placing \p Line's
> + /// first token at \p FirstIndent.
> + LineState getInitialState(unsigned FirstIndent, const AnnotatedLine
> *Line);
>
> // FIXME: canBreak and mustBreak aren't strictly indentation-related.
> Find a
> // better home.
> @@ -65,7 +64,7 @@ public:
>
> /// \brief Get the column limit for this line. This is the style's
> column
> /// limit, potentially reduced for preprocessor definitions.
> - unsigned getColumnLimit() const;
> + unsigned getColumnLimit(const LineState &State) const;
>
> private:
> /// \brief Mark the next token as consumed in \p State and modify its
> stacks
> @@ -101,8 +100,6 @@ private:
>
> FormatStyle Style;
> SourceManager &SourceMgr;
> - const AnnotatedLine &Line;
> - const unsigned FirstIndent;
> WhitespaceManager &Whitespaces;
> encoding::Encoding Encoding;
> bool BinPackInconclusiveFunctions;
> @@ -271,6 +268,14 @@ struct LineState {
> /// FIXME: Come up with a better algorithm instead.
> bool IgnoreStackForComparison;
>
> + /// \brief The indent of the first token.
> + unsigned FirstIndent;
> +
> + /// \brief The line that is being formatted.
> + ///
> + /// Does not need to be considered for memoization because it doesn't
> change.
> + const AnnotatedLine *Line;
> +
> /// \brief Comparison operator to be able to used \c LineState in \c
> map.
> bool operator<(const LineState &Other) const {
> if (NextToken != Other.NextToken)
>
> Modified: cfe/trunk/lib/Format/Format.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/Format.cpp?rev=190038&r1=190037&r2=190038&view=diff
>
> ==============================================================================
> --- cfe/trunk/lib/Format/Format.cpp (original)
> +++ cfe/trunk/lib/Format/Format.cpp Thu Sep 5 04:29:45 2013
> @@ -324,8 +324,8 @@ public:
>
> /// \brief Formats the line starting at \p State, simply keeping all of
> the
> /// input's line breaking decisions.
> - void format() {
> - LineState State = Indenter->getInitialState();
> + void format(unsigned FirstIndent, const AnnotatedLine *Line) {
> + LineState State = Indenter->getInitialState(FirstIndent, Line);
> while (State.NextToken != NULL) {
> bool Newline =
> Indenter->mustBreak(State) ||
> @@ -341,12 +341,16 @@ private:
> class UnwrappedLineFormatter {
> public:
> UnwrappedLineFormatter(ContinuationIndenter *Indenter,
> + WhitespaceManager *Whitespaces,
> const FormatStyle &Style, const AnnotatedLine
> &Line)
> - : Indenter(Indenter), Style(Style), Line(Line), Count(0) {}
> + : Indenter(Indenter), Whitespaces(Whitespaces), Style(Style),
> Line(Line),
> + Count(0) {}
>
> - /// \brief Formats an \c UnwrappedLine.
> - void format() {
> - LineState State = Indenter->getInitialState();
> + /// \brief Formats an \c UnwrappedLine and returns the penalty.
> + ///
> + /// If \p DryRun is \c false, directly applies the changes.
> + unsigned format(unsigned FirstIndent, bool DryRun = false) {
> + LineState State = Indenter->getInitialState(FirstIndent, &Line);
>
> // If the ObjC method declaration does not fit on a line, we should
> format
> // it with one arg per line.
> @@ -354,7 +358,7 @@ public:
> State.Stack.back().BreakBeforeParameter = true;
>
> // Find best solution in solution space.
> - analyzeSolutionSpace(State);
> + return analyzeSolutionSpace(State, DryRun);
> }
>
> private:
> @@ -388,8 +392,10 @@ private:
> /// This implements a variant of Dijkstra's algorithm on the graph that
> spans
> /// the solution space (\c LineStates are the nodes). The algorithm
> tries to
> /// find the shortest path (the one with lowest penalty) from \p
> InitialState
> - /// to a state where all tokens are placed.
> - void analyzeSolutionSpace(LineState &InitialState) {
> + /// to a state where all tokens are placed. Returns the penalty.
> + ///
> + /// If \p DryRun is \c false, directly applies the changes.
> + unsigned analyzeSolutionSpace(LineState &InitialState, bool DryRun =
> false) {
> std::set<LineState> Seen;
>
> // Insert start element into queue.
> @@ -398,9 +404,11 @@ private:
> Queue.push(QueueItem(OrderedPenalty(0, Count), Node));
> ++Count;
>
> + unsigned Penalty = 0;
> +
> // While not empty, take first element and follow edges.
> while (!Queue.empty()) {
> - unsigned Penalty = Queue.top().first.first;
> + Penalty = Queue.top().first.first;
> StateNode *Node = Queue.top().second;
> if (Node->State.NextToken == NULL) {
> DEBUG(llvm::dbgs() << "\n---\nPenalty for line: " << Penalty <<
> "\n");
> @@ -424,12 +432,16 @@ private:
> if (Queue.empty())
> // We were unable to find a solution, do nothing.
> // FIXME: Add diagnostic?
> - return;
> + return 0;
>
> // Reconstruct the solution.
> - reconstructPath(InitialState, Queue.top().second);
> + if (!DryRun)
> + reconstructPath(InitialState, Queue.top().second);
> +
> DEBUG(llvm::dbgs() << "Total number of analyzed states: " << Count <<
> "\n");
> DEBUG(llvm::dbgs() << "---\n");
> +
> + return Penalty;
> }
>
> void reconstructPath(LineState &State, StateNode *Current) {
> @@ -441,8 +453,10 @@ private:
> }
> for (std::deque<StateNode *>::iterator I = Path.begin(), E =
> Path.end();
> I != E; ++I) {
> - unsigned Penalty = Indenter->addTokenToState(State, (*I)->NewLine,
> false);
> - (void)Penalty;
> + unsigned Penalty = 0;
> + formatChildren(State, (*I)->NewLine, /*DryRun=*/false, Penalty);
> + Penalty += Indenter->addTokenToState(State, (*I)->NewLine, false);
> +
> DEBUG({
> if ((*I)->NewLine) {
> llvm::dbgs() << "Penalty for placing "
> @@ -466,18 +480,80 @@ private:
>
> StateNode *Node = new (Allocator.Allocate())
> StateNode(PreviousNode->State, NewLine, PreviousNode);
> + if (!formatChildren(Node->State, NewLine, /*DryRun=*/true, Penalty))
> + return;
> +
> Penalty += Indenter->addTokenToState(Node->State, NewLine, true);
> - if (Node->State.Column > Indenter->getColumnLimit()) {
> - unsigned ExcessCharacters =
> - Node->State.Column - Indenter->getColumnLimit();
> - Penalty += Style.PenaltyExcessCharacter * ExcessCharacters;
> - }
>
> Queue.push(QueueItem(OrderedPenalty(Penalty, Count), Node));
> ++Count;
> }
>
> + /// \brief Format all children of \p Tok assuming the parent is
> indented to
> + /// \p ParentIndent.
>
ParentIndent is not a parameter...
> + ///
> + /// Returns \c true if all children could be placed successfully and
> adapts
> + /// \p Penalty as well as \p State. If \p DryRun is false, also directly
> + /// creates changes using \c Whitespaces.
> + ///
> + /// The crucial idea here is that children always get formatted upon
> + /// encountering the closing brace right after the nested block. Now,
> if we
> + /// are currently trying to keep the "}" on the same line (i.e. \p
> NewLine is
> + /// \c false), the entire block has to be kept on the same line (which
> is only
> + /// possible if it fits on the line, only contains a single statement,
> etc.
> + ///
> + /// If \p NewLine is true, we format the nested block on separate
> lines, i.e.
> + /// break after the "{", format all lines with correct indentation and
> the put
> + /// the closing "}" on yet another new line.
> + ///
> + /// This enables us to keep the simple structure of the
> + /// \c UnwrappedLineFormatter, where we only have two options for each
> token:
> + /// break or don't break.
> + bool formatChildren(LineState &State, bool NewLine, bool DryRun,
> + unsigned &Penalty) {
> + const FormatToken &LBrace = *State.NextToken->Previous;
> + if (LBrace.isNot(tok::l_brace) || LBrace.BlockKind != BK_Block ||
> + LBrace.Children.size() == 0)
> + return true; // The previous token does not open a block. Nothing
> to do.
>
Why's that not an assert?
> +
> + if (NewLine) {
> + unsigned ParentIndent = State.Stack.back().Indent;
> + for (SmallVector<AnnotatedLine *, 1>::const_iterator
> + I = LBrace.Children.begin(),
> + E = LBrace.Children.end();
> + I != E; ++I) {
> + unsigned Indent =
> + ParentIndent + ((*I)->Level - Line.Level) * Style.IndentWidth;
> + if (!DryRun)
> + Whitespaces->replaceWhitespace(
> + *(*I)->First, /*Newlines=*/1, /*Spaces=*/Indent,
> + /*StartOfTokenColumn=*/Indent, Line.InPPDirective);
> + UnwrappedLineFormatter Formatter(Indenter, Whitespaces, Style,
> **I);
> + Penalty += Formatter.format(Indent, DryRun);
> + }
> + return true;
> + }
> +
> + if (LBrace.Children.size() > 1)
> + return false; // Cannot merge multiple statements into a single
> line.
> +
> + // We can't put the closing "}" on a line with a trailing comment.
> + if (LBrace.Children[0]->Last->isTrailingComment())
> + return false;
> +
> + if (!DryRun) {
> + Whitespaces->replaceWhitespace(*LBrace.Children[0]->First,
> + /*Newlines=*/0, /*Spaces=*/1,
> + /*StartOfTokenColumn=*/State.Column,
> + State.Line->InPPDirective);
> + }
> +
> + State.Column += 1 + LBrace.Children[0]->Last->TotalLength;
> + return true;
> + }
> +
> ContinuationIndenter *Indenter;
> + WhitespaceManager *Whitespaces;
> FormatStyle Style;
> const AnnotatedLine &Line;
>
> @@ -654,7 +730,12 @@ public:
> << "\n");
> }
>
> - virtual ~Formatter() {}
> + virtual ~Formatter() {
> + for (unsigned i = 0, e = AnnotatedLines.size(); i != e; ++i) {
> + delete AnnotatedLines[i];
> + }
> + AnnotatedLines.clear();
>
I would not do the clear(), as the object is not reachable afterwards...
> + }
>
> tooling::Replacements format() {
> FormatTokenLexer Tokens(Lex, SourceMgr, Style, Encoding);
> @@ -663,23 +744,23 @@ public:
> bool StructuralError = Parser.parse();
> TokenAnnotator Annotator(Style, Tokens.getIdentTable().get("in"));
> for (unsigned i = 0, e = AnnotatedLines.size(); i != e; ++i) {
> - Annotator.annotate(AnnotatedLines[i]);
> + Annotator.annotate(*AnnotatedLines[i]);
> }
> deriveLocalStyle();
> for (unsigned i = 0, e = AnnotatedLines.size(); i != e; ++i) {
> - Annotator.calculateFormattingInformation(AnnotatedLines[i]);
> + Annotator.calculateFormattingInformation(*AnnotatedLines[i]);
> }
>
> // Adapt level to the next line if this is a comment.
> // FIXME: Can/should this be done in the UnwrappedLineParser?
> const AnnotatedLine *NextNonCommentLine = NULL;
> for (unsigned i = AnnotatedLines.size() - 1; i > 0; --i) {
> - if (NextNonCommentLine && AnnotatedLines[i].First->is(tok::comment)
> &&
> - !AnnotatedLines[i].First->Next)
> - AnnotatedLines[i].Level = NextNonCommentLine->Level;
> + if (NextNonCommentLine &&
> AnnotatedLines[i]->First->is(tok::comment) &&
> + !AnnotatedLines[i]->First->Next)
> + AnnotatedLines[i]->Level = NextNonCommentLine->Level;
> else
> - NextNonCommentLine = AnnotatedLines[i].First->isNot(tok::r_brace)
> - ? &AnnotatedLines[i]
> + NextNonCommentLine = AnnotatedLines[i]->First->isNot(tok::r_brace)
> + ? AnnotatedLines[i]
> : NULL;
> }
>
> @@ -687,10 +768,10 @@ public:
> bool PreviousLineWasTouched = false;
> const FormatToken *PreviousLineLastToken = 0;
> bool FormatPPDirective = false;
> - for (std::vector<AnnotatedLine>::iterator I = AnnotatedLines.begin(),
> - E = AnnotatedLines.end();
> + for (std::vector<AnnotatedLine *>::iterator I =
> AnnotatedLines.begin(),
> + E = AnnotatedLines.end();
> I != E; ++I) {
> - const AnnotatedLine &TheLine = *I;
> + const AnnotatedLine &TheLine = **I;
> const FormatToken *FirstTok = TheLine.First;
> int Offset = getIndentOffset(*TheLine.First);
>
> @@ -729,26 +810,27 @@ public:
> } else {
> Indent = LevelIndent = FirstTok->OriginalColumn;
> }
> - ContinuationIndenter Indenter(Style, SourceMgr, TheLine, Indent,
> - Whitespaces, Encoding,
> + ContinuationIndenter Indenter(Style, SourceMgr, Whitespaces,
> Encoding,
> BinPackInconclusiveFunctions);
>
> // If everything fits on a single line, just put it there.
> unsigned ColumnLimit = Style.ColumnLimit;
> - if ((I + 1) != E && (I + 1)->InPPDirective &&
> - !(I + 1)->First->HasUnescapedNewline)
> - ColumnLimit = Indenter.getColumnLimit();
> + AnnotatedLine *NextLine = *(I + 1);
> + if ((I + 1) != E && NextLine->InPPDirective &&
> + !NextLine->First->HasUnescapedNewline)
> + ColumnLimit = getColumnLimit(TheLine.InPPDirective);
>
> - if (I->Last->TotalLength + Indent <= ColumnLimit) {
> - LineState State = Indenter.getInitialState();
> + if (TheLine.Last->TotalLength + Indent <= ColumnLimit) {
> + LineState State = Indenter.getInitialState(Indent, &TheLine);
> while (State.NextToken != NULL)
> Indenter.addTokenToState(State, false, false);
> } else if (Style.ColumnLimit == 0) {
> NoColumnLimitFormatter Formatter(&Indenter);
> - Formatter.format();
> + Formatter.format(Indent, &TheLine);
> } else {
> - UnwrappedLineFormatter Formatter(&Indenter, Style, TheLine);
> - Formatter.format();
> + UnwrappedLineFormatter Formatter(&Indenter, &Whitespaces, Style,
> + TheLine);
> + Formatter.format(Indent);
> }
>
> IndentForLevel[TheLine.Level] = LevelIndent;
> @@ -783,7 +865,7 @@ public:
> // last token.
> PreviousLineWasTouched = false;
> }
> - PreviousLineLastToken = I->Last;
> + PreviousLineLastToken = TheLine.Last;
> }
> return Whitespaces.generateReplacements();
> }
> @@ -796,9 +878,9 @@ private:
> bool HasBinPackedFunction = false;
> bool HasOnePerLineFunction = false;
> for (unsigned i = 0, e = AnnotatedLines.size(); i != e; ++i) {
> - if (!AnnotatedLines[i].First->Next)
> + if (!AnnotatedLines[i]->First->Next)
> continue;
> - FormatToken *Tok = AnnotatedLines[i].First->Next;
> + FormatToken *Tok = AnnotatedLines[i]->First->Next;
> while (Tok->Next) {
> if (Tok->Type == TT_PointerOrReference) {
> bool SpacesBefore =
> @@ -866,10 +948,11 @@ private:
> /// This will change \c Line and \c AnnotatedLine to contain the merged
> line,
> /// if possible; note that \c I will be incremented when lines are
> merged.
> void tryFitMultipleLinesInOne(unsigned Indent,
> - std::vector<AnnotatedLine>::iterator &I,
> - std::vector<AnnotatedLine>::iterator E) {
> + std::vector<AnnotatedLine *>::iterator &I,
> + std::vector<AnnotatedLine *>::iterator E)
> {
> // We can never merge stuff if there are trailing line comments.
> - if (I->Last->Type == TT_LineComment)
> + AnnotatedLine *TheLine = *I;
> + if (TheLine->Last->Type == TT_LineComment)
> return;
>
> if (Indent > Style.ColumnLimit)
> @@ -878,70 +961,72 @@ private:
> unsigned Limit = Style.ColumnLimit - Indent;
> // If we already exceed the column limit, we set 'Limit' to 0. The
> different
> // tryMerge..() functions can then decide whether to still do merging.
> - Limit = I->Last->TotalLength > Limit ? 0 : Limit -
> I->Last->TotalLength;
> + Limit = TheLine->Last->TotalLength > Limit
> + ? 0
> + : Limit - TheLine->Last->TotalLength;
>
> - if (I + 1 == E || (I + 1)->Type == LT_Invalid)
> + if (I + 1 == E || (*(I + 1))->Type == LT_Invalid)
> return;
>
> - if (I->Last->is(tok::l_brace)) {
> + if (TheLine->Last->is(tok::l_brace)) {
> tryMergeSimpleBlock(I, E, Limit);
> } else if (Style.AllowShortIfStatementsOnASingleLine &&
> - I->First->is(tok::kw_if)) {
> + TheLine->First->is(tok::kw_if)) {
> tryMergeSimpleControlStatement(I, E, Limit);
> } else if (Style.AllowShortLoopsOnASingleLine &&
> - I->First->isOneOf(tok::kw_for, tok::kw_while)) {
> + TheLine->First->isOneOf(tok::kw_for, tok::kw_while)) {
> tryMergeSimpleControlStatement(I, E, Limit);
> - } else if (I->InPPDirective &&
> - (I->First->HasUnescapedNewline || I->First->IsFirst)) {
> + } else if (TheLine->InPPDirective &&
> (TheLine->First->HasUnescapedNewline ||
> + TheLine->First->IsFirst)) {
> tryMergeSimplePPDirective(I, E, Limit);
> }
> }
>
> - void tryMergeSimplePPDirective(std::vector<AnnotatedLine>::iterator &I,
> - std::vector<AnnotatedLine>::iterator E,
> + void tryMergeSimplePPDirective(std::vector<AnnotatedLine *>::iterator
> &I,
> + std::vector<AnnotatedLine *>::iterator E,
> unsigned Limit) {
> if (Limit == 0)
> return;
> - AnnotatedLine &Line = *I;
> - if (!(I + 1)->InPPDirective || (I + 1)->First->HasUnescapedNewline)
> + AnnotatedLine &Line = **I;
> + if (!(*(I + 1))->InPPDirective || (*(I +
> 1))->First->HasUnescapedNewline)
> return;
> - if (I + 2 != E && (I + 2)->InPPDirective &&
> - !(I + 2)->First->HasUnescapedNewline)
> + if (I + 2 != E && (*(I + 2))->InPPDirective &&
> + !(*(I + 2))->First->HasUnescapedNewline)
> return;
> - if (1 + (I + 1)->Last->TotalLength > Limit)
> + if (1 + (*(I + 1))->Last->TotalLength > Limit)
> return;
> - join(Line, *(++I));
> + join(Line, **(++I));
> }
>
> - void
> tryMergeSimpleControlStatement(std::vector<AnnotatedLine>::iterator &I,
> -
> std::vector<AnnotatedLine>::iterator E,
> + void tryMergeSimpleControlStatement(std::vector<AnnotatedLine
> *>::iterator &I,
> + std::vector<AnnotatedLine
> *>::iterator E,
> unsigned Limit) {
> if (Limit == 0)
> return;
> if (Style.BreakBeforeBraces == FormatStyle::BS_Allman &&
> - (I + 1)->First->is(tok::l_brace))
> + (*(I + 1))->First->is(tok::l_brace))
> return;
> - if ((I + 1)->InPPDirective != I->InPPDirective ||
> - ((I + 1)->InPPDirective && (I + 1)->First->HasUnescapedNewline))
> + if ((*(I + 1))->InPPDirective != (*I)->InPPDirective ||
> + ((*(I + 1))->InPPDirective && (*(I +
> 1))->First->HasUnescapedNewline))
> return;
> - AnnotatedLine &Line = *I;
> + AnnotatedLine &Line = **I;
> if (Line.Last->isNot(tok::r_paren))
> return;
> - if (1 + (I + 1)->Last->TotalLength > Limit)
> + if (1 + (*(I + 1))->Last->TotalLength > Limit)
> return;
> - if ((I + 1)->First->isOneOf(tok::semi, tok::kw_if, tok::kw_for,
> - tok::kw_while) ||
> - (I + 1)->First->Type == TT_LineComment)
> + if ((*(I + 1))->First->isOneOf(tok::semi, tok::kw_if, tok::kw_for,
> + tok::kw_while) ||
> + (*(I + 1))->First->Type == TT_LineComment)
> return;
> // Only inline simple if's (no nested if or else).
> if (I + 2 != E && Line.First->is(tok::kw_if) &&
> - (I + 2)->First->is(tok::kw_else))
> + (*(I + 2))->First->is(tok::kw_else))
> return;
> - join(Line, *(++I));
> + join(Line, **(++I));
> }
>
> - void tryMergeSimpleBlock(std::vector<AnnotatedLine>::iterator &I,
> - std::vector<AnnotatedLine>::iterator E,
> + void tryMergeSimpleBlock(std::vector<AnnotatedLine *>::iterator &I,
> + std::vector<AnnotatedLine *>::iterator E,
> unsigned Limit) {
> // No merging if the brace already is on the next line.
> if (Style.BreakBeforeBraces != FormatStyle::BS_Attach)
> @@ -950,7 +1035,7 @@ private:
> // First, check that the current line allows merging. This is the
> case if
> // we're not in a control flow statement and the last token is an
> opening
> // brace.
> - AnnotatedLine &Line = *I;
> + AnnotatedLine &Line = **I;
> if (Line.First->isOneOf(tok::kw_if, tok::kw_while, tok::kw_do,
> tok::r_brace,
> tok::kw_else, tok::kw_try, tok::kw_catch,
> tok::kw_for,
> @@ -958,24 +1043,24 @@ private:
> tok::at, tok::minus, tok::plus))
> return;
>
> - FormatToken *Tok = (I + 1)->First;
> + FormatToken *Tok = (*(I + 1))->First;
> if (Tok->is(tok::r_brace) && !Tok->MustBreakBefore &&
> (Tok->getNextNonComment() == NULL ||
> Tok->getNextNonComment()->is(tok::semi))) {
> // We merge empty blocks even if the line exceeds the column limit.
> Tok->SpacesRequiredBefore = 0;
> Tok->CanBreakBefore = true;
> - join(Line, *(I + 1));
> + join(Line, **(I + 1));
> I += 1;
> } else if (Limit != 0 && Line.First->isNot(tok::kw_namespace)) {
> // Check that we still have three lines and they fit into the limit.
> - if (I + 2 == E || (I + 2)->Type == LT_Invalid ||
> + if (I + 2 == E || (*(I + 2))->Type == LT_Invalid ||
> !nextTwoLinesFitInto(I, Limit))
> return;
>
> // Second, check that the next line does not contain any braces -
> if it
> // does, readability declines when putting it into a single line.
> - if ((I + 1)->Last->Type == TT_LineComment || Tok->MustBreakBefore)
> + if ((*(I + 1))->Last->Type == TT_LineComment ||
> Tok->MustBreakBefore)
> return;
> do {
> if (Tok->isOneOf(tok::l_brace, tok::r_brace))
> @@ -984,20 +1069,21 @@ private:
> } while (Tok != NULL);
>
> // Last, check that the third line contains a single closing brace.
> - Tok = (I + 2)->First;
> + Tok = (*(I + 2))->First;
> if (Tok->getNextNonComment() != NULL || Tok->isNot(tok::r_brace) ||
> Tok->MustBreakBefore)
> return;
>
> - join(Line, *(I + 1));
> - join(Line, *(I + 2));
> + join(Line, **(I + 1));
> + join(Line, **(I + 2));
> I += 2;
> }
> }
>
> - bool nextTwoLinesFitInto(std::vector<AnnotatedLine>::iterator I,
> + bool nextTwoLinesFitInto(std::vector<AnnotatedLine *>::iterator I,
> unsigned Limit) {
> - return 1 + (I + 1)->Last->TotalLength + 1 + (I +
> 2)->Last->TotalLength <=
> + return 1 + (*(I + 1))->Last->TotalLength + 1 +
> + (*(I + 2))->Last->TotalLength <=
> Limit;
> }
>
> @@ -1034,12 +1120,12 @@ private:
> return touchesRanges(LineRange);
> }
>
> - bool touchesPPDirective(std::vector<AnnotatedLine>::iterator I,
> - std::vector<AnnotatedLine>::iterator E) {
> + bool touchesPPDirective(std::vector<AnnotatedLine *>::iterator I,
> + std::vector<AnnotatedLine *>::iterator E) {
> for (; I != E; ++I) {
> - if (I->First->HasUnescapedNewline)
> + if ((*I)->First->HasUnescapedNewline)
> return false;
> - if (touchesLine(*I))
> + if (touchesLine(**I))
> return true;
> }
> return false;
> @@ -1055,7 +1141,7 @@ private:
> }
>
> virtual void consumeUnwrappedLine(const UnwrappedLine &TheLine) {
> - AnnotatedLines.push_back(AnnotatedLine(TheLine));
> + AnnotatedLines.push_back(new AnnotatedLine(TheLine));
> }
>
> /// \brief Add a new line and the required indent before the first Token
> @@ -1084,12 +1170,17 @@ private:
> InPPDirective && !RootToken.HasUnescapedNewline);
> }
>
> + unsigned getColumnLimit(bool InPPDirective) const {
> + // In preprocessor directives reserve two chars for trailing " \"
> + return Style.ColumnLimit - (InPPDirective ? 2 : 0);
> + }
> +
> FormatStyle Style;
> Lexer &Lex;
> SourceManager &SourceMgr;
> WhitespaceManager Whitespaces;
> std::vector<CharSourceRange> Ranges;
> - std::vector<AnnotatedLine> AnnotatedLines;
> + std::vector<AnnotatedLine *> AnnotatedLines;
>
> encoding::Encoding Encoding;
> bool BinPackInconclusiveFunctions;
>
> Modified: cfe/trunk/lib/Format/FormatToken.h
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/FormatToken.h?rev=190038&r1=190037&r2=190038&view=diff
>
> ==============================================================================
> --- cfe/trunk/lib/Format/FormatToken.h (original)
> +++ cfe/trunk/lib/Format/FormatToken.h Thu Sep 5 04:29:45 2013
> @@ -36,6 +36,7 @@ enum TokenType {
> TT_InlineASMColon,
> TT_InheritanceColon,
> TT_FunctionTypeLParen,
> + TT_LambdaLSquare,
> TT_LineComment,
> TT_ObjCArrayLiteral,
> TT_ObjCBlockLParen,
> @@ -75,6 +76,7 @@ enum ParameterPackingKind {
> };
>
> class TokenRole;
> +class AnnotatedLine;
>
> /// \brief A wrapper around a \c Token storing information about the
> /// whitespace characters preceeding it.
> @@ -335,6 +337,8 @@ struct FormatToken {
> FormatToken *Previous;
> FormatToken *Next;
>
> + SmallVector<AnnotatedLine *, 1> Children;
> +
> private:
> // Disallow copying.
> FormatToken(const FormatToken &) LLVM_DELETED_FUNCTION;
>
> Modified: cfe/trunk/lib/Format/TokenAnnotator.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/TokenAnnotator.cpp?rev=190038&r1=190037&r2=190038&view=diff
>
> ==============================================================================
> --- cfe/trunk/lib/Format/TokenAnnotator.cpp (original)
> +++ cfe/trunk/lib/Format/TokenAnnotator.cpp Thu Sep 5 04:29:45 2013
> @@ -182,7 +182,7 @@ private:
> FormatToken *Left = CurrentToken->Previous;
> FormatToken *Parent = Left->getPreviousNonComment();
> bool StartsObjCMethodExpr =
> - Contexts.back().CanBeExpression &&
> + Contexts.back().CanBeExpression && Left->Type != TT_LambdaLSquare
> &&
> (!Parent || Parent->isOneOf(tok::colon, tok::l_square,
> tok::l_paren,
> tok::kw_return, tok::kw_throw) ||
> Parent->isUnaryOperator() || Parent->Type == TT_ObjCForIn ||
> @@ -522,7 +522,7 @@ private:
>
> // Reset token type in case we have already looked at it and then
> recovered
> // from an error (e.g. failure to find the matching >).
> - if (CurrentToken != NULL)
> + if (CurrentToken != NULL && CurrentToken->Type != TT_LambdaLSquare)
> CurrentToken->Type = TT_Unknown;
> }
>
> @@ -974,6 +974,11 @@ private:
> } // end anonymous namespace
>
> void TokenAnnotator::annotate(AnnotatedLine &Line) {
> + for (std::vector<AnnotatedLine *>::iterator I = Line.Children.begin(),
> + E = Line.Children.end();
> + I != E; ++I) {
> + annotate(**I);
> + }
> AnnotatingParser Parser(Style, Line, Ident_in);
> Line.Type = Parser.parseLine();
> if (Line.Type == LT_Invalid)
> @@ -1026,7 +1031,7 @@ void TokenAnnotator::calculateFormatting
> }
> Current->CanBreakBefore =
> Current->MustBreakBefore || canBreakBefore(Line, *Current);
> - if (Current->MustBreakBefore ||
> + if (Current->MustBreakBefore || !Current->Children.empty() ||
> (Current->is(tok::string_literal) && Current->isMultiline()))
> Current->TotalLength = Current->Previous->TotalLength +
> Style.ColumnLimit;
> else
> @@ -1048,9 +1053,13 @@ void TokenAnnotator::calculateFormatting
> Current->Role->precomputeFormattingInfos(Current);
> }
>
> - DEBUG({
> - printDebugInfo(Line);
> - });
> + DEBUG({ printDebugInfo(Line); });
> +
> + for (std::vector<AnnotatedLine *>::iterator I = Line.Children.begin(),
> + E = Line.Children.end();
> + I != E; ++I) {
> + calculateFormattingInformation(**I);
> + }
> }
>
> void TokenAnnotator::calculateUnbreakableTailLengths(AnnotatedLine &Line)
> {
> @@ -1212,7 +1221,7 @@ bool TokenAnnotator::spaceRequiredBetwee
> if (Right.is(tok::r_square))
> return Right.Type == TT_ObjCArrayLiteral;
> if (Right.is(tok::l_square) && Right.Type != TT_ObjCMethodExpr &&
> - Left.isNot(tok::numeric_constant))
> + Right.Type != TT_LambdaLSquare && Left.isNot(tok::numeric_constant))
> return false;
> if (Left.is(tok::colon))
> return Left.Type != TT_ObjCMethodExpr;
> @@ -1233,7 +1242,7 @@ bool TokenAnnotator::spaceRequiredBetwee
> if (Left.is(tok::at) && Right.Tok.getObjCKeywordID() !=
> tok::objc_not_keyword)
> return false;
> if (Left.is(tok::l_brace) && Right.is(tok::r_brace))
> - return false; // No spaces in "{}".
> + return !Left.Children.empty(); // No spaces in "{}".
> if (Left.is(tok::l_brace) || Right.is(tok::r_brace))
> return !Style.Cpp11BracedListStyle;
> if (Right.Type == TT_UnaryOperator)
> @@ -1355,11 +1364,13 @@ bool TokenAnnotator::canBreakBefore(cons
> // change the "binding" behavior of a comment.
> return false;
>
> + if (Right.is(tok::r_paren) || Right.Type == TT_TemplateCloser)
> + return false;
> +
> // We only break before r_brace if there was a corresponding break
> before
> // the l_brace, which is tracked by BreakBeforeClosingBrace.
> - if (Right.isOneOf(tok::r_brace, tok::r_paren) ||
> - Right.Type == TT_TemplateCloser)
> - return false;
> + if (Right.is(tok::r_brace))
> + return Right.MatchingParen && Right.MatchingParen->BlockKind ==
> BK_Block;
>
> // Allow breaking after a trailing 'const', e.g. after a method
> declaration,
> // unless it is follow by ';', '{' or '='.
>
> Modified: cfe/trunk/lib/Format/TokenAnnotator.h
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/TokenAnnotator.h?rev=190038&r1=190037&r2=190038&view=diff
>
> ==============================================================================
> --- cfe/trunk/lib/Format/TokenAnnotator.h (original)
> +++ cfe/trunk/lib/Format/TokenAnnotator.h Thu Sep 5 04:29:45 2013
> @@ -38,31 +38,53 @@ enum LineType {
> class AnnotatedLine {
> public:
> AnnotatedLine(const UnwrappedLine &Line)
> - : First(Line.Tokens.front()), Level(Line.Level),
> + : First(Line.Tokens.front().Tok), Level(Line.Level),
> InPPDirective(Line.InPPDirective),
> MustBeDeclaration(Line.MustBeDeclaration),
> MightBeFunctionDecl(false),
> StartsDefinition(false) {
> assert(!Line.Tokens.empty());
> FormatToken *Current = First;
> - for (std::list<FormatToken *>::const_iterator I =
> ++Line.Tokens.begin(),
> - E = Line.Tokens.end();
> + for (std::list<UnwrappedLineNode>::const_iterator I =
> ++Line.Tokens.begin(),
> + E =
> Line.Tokens.end();
> I != E; ++I) {
> - Current->Next = *I;
> - (*I)->Previous = Current;
> + const UnwrappedLineNode &Node = *I;
> + Current->Next = I->Tok;
> + I->Tok->Previous = Current;
> Current = Current->Next;
> + for (SmallVectorImpl<UnwrappedLine>::const_iterator
> + I = Node.Children.begin(),
> + E = Node.Children.end();
> + I != E; ++I) {
> + Children.push_back(new AnnotatedLine(*I));
> + Current->Children.push_back(Children.back());
> + }
> }
> Last = Current;
> }
>
> + ~AnnotatedLine() {
> + for (unsigned i = 0, e = Children.size(); i != e; ++i) {
> + delete Children[i];
> + }
> + Children.clear();
>
Same here...
> + }
> +
> FormatToken *First;
> FormatToken *Last;
>
> + std::vector<AnnotatedLine *> Children;
> +
> LineType Type;
> unsigned Level;
> bool InPPDirective;
> bool MustBeDeclaration;
> bool MightBeFunctionDecl;
> bool StartsDefinition;
> +
> +private:
> + // Disallow copying.
> + AnnotatedLine(const AnnotatedLine &) LLVM_DELETED_FUNCTION;
> + void operator=(const AnnotatedLine &) LLVM_DELETED_FUNCTION;
> };
>
> /// \brief Determines extra information about the tokens comprising an
>
> Modified: cfe/trunk/lib/Format/UnwrappedLineParser.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/UnwrappedLineParser.cpp?rev=190038&r1=190037&r2=190038&view=diff
>
> ==============================================================================
> --- cfe/trunk/lib/Format/UnwrappedLineParser.cpp (original)
> +++ cfe/trunk/lib/Format/UnwrappedLineParser.cpp Thu Sep 5 04:29:45 2013
> @@ -122,9 +122,12 @@ class ScopedLineState {
> public:
> ScopedLineState(UnwrappedLineParser &Parser,
> bool SwitchToPreprocessorLines = false)
> - : Parser(Parser),
> SwitchToPreprocessorLines(SwitchToPreprocessorLines) {
> + : Parser(Parser) {
> + OriginalLines = Parser.CurrentLines;
> if (SwitchToPreprocessorLines)
> Parser.CurrentLines = &Parser.PreprocessorDirectives;
> + else if (!Parser.Line->Tokens.empty())
> + Parser.CurrentLines = &Parser.Line->Tokens.back().Children;
> PreBlockLine = Parser.Line.take();
> Parser.Line.reset(new UnwrappedLine());
> Parser.Line->Level = PreBlockLine->Level;
> @@ -137,16 +140,16 @@ public:
> }
> assert(Parser.Line->Tokens.empty());
> Parser.Line.reset(PreBlockLine);
> - Parser.MustBreakBeforeNextToken = true;
> - if (SwitchToPreprocessorLines)
> - Parser.CurrentLines = &Parser.Lines;
> + if (Parser.CurrentLines == &Parser.PreprocessorDirectives)
> + Parser.MustBreakBeforeNextToken = true;
> + Parser.CurrentLines = OriginalLines;
> }
>
> private:
> UnwrappedLineParser &Parser;
> - const bool SwitchToPreprocessorLines;
>
> UnwrappedLine *PreBlockLine;
> + SmallVectorImpl<UnwrappedLine> *OriginalLines;
> };
>
> namespace {
> @@ -191,7 +194,8 @@ bool UnwrappedLineParser::parse() {
> Tokens = &TokenSource;
> readToken();
> parseFile();
> - for (std::vector<UnwrappedLine>::iterator I = Lines.begin(), E =
> Lines.end();
> + for (SmallVectorImpl<UnwrappedLine>::iterator I = Lines.begin(),
> + E = Lines.end();
> I != E; ++I) {
> Callback.consumeUnwrappedLine(*I);
> }
> @@ -670,6 +674,8 @@ void UnwrappedLineParser::parseStructura
> }
>
> void UnwrappedLineParser::tryToParseLambda() {
> + assert(FormatTok->is(tok::l_square));
> + FormatToken &LSquare = *FormatTok;
> if (!tryToParseLambdaIntroducer()) {
> return;
> }
> @@ -681,7 +687,6 @@ void UnwrappedLineParser::tryToParseLamb
> switch (FormatTok->Tok.getKind()) {
> case tok::l_brace:
> break;
> - return;
> case tok::l_paren:
> parseParens();
> break;
> @@ -694,6 +699,7 @@ void UnwrappedLineParser::tryToParseLamb
> break;
> }
> }
> + LSquare.Type = TT_LambdaLSquare;
> parseChildBlock();
> }
>
> @@ -1183,23 +1189,39 @@ void UnwrappedLineParser::parseObjCProto
> parseObjCUntilAtEnd();
> }
>
> +static void printDebugInfo(const UnwrappedLine &Line, StringRef Prefix =
> "") {
> + llvm::dbgs() << Prefix << "Line(" << Line.Level << ")"
> + << (Line.InPPDirective ? " MACRO" : "") << ": ";
> + for (std::list<UnwrappedLineNode>::const_iterator I =
> Line.Tokens.begin(),
> + E = Line.Tokens.end();
> + I != E; ++I) {
> + llvm::dbgs() << I->Tok->Tok.getName() << " ";
> + }
> + for (std::list<UnwrappedLineNode>::const_iterator I =
> Line.Tokens.begin(),
> + E = Line.Tokens.end();
> + I != E; ++I) {
> + const UnwrappedLineNode &Node = *I;
> + for (SmallVectorImpl<UnwrappedLine>::const_iterator
> + I = Node.Children.begin(),
> + E = Node.Children.end();
> + I != E; ++I) {
> + printDebugInfo(*I, "\nChild: ");
> + }
> + }
> + llvm::dbgs() << "\n";
> +}
> +
> void UnwrappedLineParser::addUnwrappedLine() {
> if (Line->Tokens.empty())
> return;
> DEBUG({
> - llvm::dbgs() << "Line(" << Line->Level << ")"
> - << (Line->InPPDirective ? " MACRO" : "") << ": ";
> - for (std::list<FormatToken *>::iterator I = Line->Tokens.begin(),
> - E = Line->Tokens.end();
> - I != E; ++I) {
> - llvm::dbgs() << (*I)->Tok.getName() << " ";
> - }
> - llvm::dbgs() << "\n";
> + if (CurrentLines == &Lines)
> + printDebugInfo(*Line);
> });
> CurrentLines->push_back(*Line);
> Line->Tokens.clear();
> if (CurrentLines == &Lines && !PreprocessorDirectives.empty()) {
> - for (std::vector<UnwrappedLine>::iterator
> + for (SmallVectorImpl<UnwrappedLine>::iterator
> I = PreprocessorDirectives.begin(),
> E = PreprocessorDirectives.end();
> I != E; ++I) {
> @@ -1273,9 +1295,9 @@ void UnwrappedLineParser::readToken() {
> }
>
> void UnwrappedLineParser::pushToken(FormatToken *Tok) {
> - Line->Tokens.push_back(Tok);
> + Line->Tokens.push_back(UnwrappedLineNode(Tok));
> if (MustBreakBeforeNextToken) {
> - Line->Tokens.back()->MustBreakBefore = true;
> + Line->Tokens.back().Tok->MustBreakBefore = true;
> MustBreakBeforeNextToken = false;
> }
> }
>
> Modified: cfe/trunk/lib/Format/UnwrappedLineParser.h
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/UnwrappedLineParser.h?rev=190038&r1=190037&r2=190038&view=diff
>
> ==============================================================================
> --- cfe/trunk/lib/Format/UnwrappedLineParser.h (original)
> +++ cfe/trunk/lib/Format/UnwrappedLineParser.h Thu Sep 5 04:29:45 2013
> @@ -24,6 +24,8 @@
> namespace clang {
> namespace format {
>
> +struct UnwrappedLineNode;
> +
> /// \brief An unwrapped line is a sequence of \c Token, that we would
> like to
> /// put on a single line if there was no column limit.
> ///
> @@ -35,7 +37,7 @@ struct UnwrappedLine {
>
> // FIXME: Don't use std::list here.
> /// \brief The \c Tokens comprising this \c UnwrappedLine.
> - std::list<FormatToken *> Tokens;
> + std::list<UnwrappedLineNode> Tokens;
>
> /// \brief The indent level of the \c UnwrappedLine.
> unsigned Level;
> @@ -119,18 +121,18 @@ private:
> bool MustBreakBeforeNextToken;
>
> // The parsed lines. Only added to through \c CurrentLines.
> - std::vector<UnwrappedLine> Lines;
> + SmallVector<UnwrappedLine, 8> Lines;
>
> // Preprocessor directives are parsed out-of-order from other unwrapped
> lines.
> // Thus, we need to keep a list of preprocessor directives to be
> reported
> // after an unwarpped line that has been started was finished.
> - std::vector<UnwrappedLine> PreprocessorDirectives;
> + SmallVector<UnwrappedLine, 4> PreprocessorDirectives;
>
> // New unwrapped lines are added via CurrentLines.
> // Usually points to \c &Lines. While parsing a preprocessor directive
> when
> // there is an unfinished previous unwrapped line, will point to
> // \c &PreprocessorDirectives.
> - std::vector<UnwrappedLine> *CurrentLines;
> + SmallVectorImpl<UnwrappedLine> *CurrentLines;
>
> // We store for each line whether it must be a declaration depending on
> // whether we are in a compound statement or not.
> @@ -162,6 +164,14 @@ private:
> friend class ScopedLineState;
> };
>
> +struct UnwrappedLineNode {
> + UnwrappedLineNode() : Tok(NULL) {}
> + UnwrappedLineNode(FormatToken *Tok) : Tok(Tok) {}
> +
> + FormatToken *Tok;
> + SmallVector<UnwrappedLine, 0> Children;
> +};
> +
> } // end namespace format
> } // end namespace clang
>
>
> Modified: cfe/trunk/unittests/Format/FormatTest.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=190038&r1=190037&r2=190038&view=diff
>
> ==============================================================================
> --- cfe/trunk/unittests/Format/FormatTest.cpp (original)
> +++ cfe/trunk/unittests/Format/FormatTest.cpp Thu Sep 5 04:29:45 2013
> @@ -110,7 +110,7 @@ TEST_F(FormatTest, MessUp) {
> // Basic function tests.
>
> //===----------------------------------------------------------------------===//
>
> -TEST_F(FormatTest, DoesNotChangeCorrectlyFormatedCode) {
> +TEST_F(FormatTest, DoesNotChangeCorrectlyFormattedCode) {
> EXPECT_EQ(";", format(";"));
> }
>
> @@ -1496,7 +1496,9 @@ TEST_F(FormatTest, FormatsClasses) {
> " public G {};");
>
> verifyFormat("class\n"
> - " ReallyReallyLongClassName {\n};",
> + " ReallyReallyLongClassName {\n"
> + " int i;\n"
> + "};",
> getLLVMStyleWithColumns(32));
> }
>
> @@ -2184,23 +2186,37 @@ TEST_F(FormatTest, LayoutStatementsAroun
> }
>
> TEST_F(FormatTest, LayoutBlockInsideParens) {
> + EXPECT_EQ("functionCall({ int i; });", format(" functionCall ( {int i;}
> );"));
> EXPECT_EQ("functionCall({\n"
> " int i;\n"
> + " int j;\n"
> "});",
> - format(" functionCall ( {int i;} );"));
> -
> - // FIXME: This is bad, find a better and more generic solution.
> + format(" functionCall ( {int i;int j;} );"));
> EXPECT_EQ("functionCall({\n"
> - " int i;\n"
> - "},\n"
> + " int i;\n"
> + " int j;\n"
> + " },\n"
> " aaaa, bbbb, cccc);",
> - format(" functionCall ( {int i;}, aaaa, bbbb, cccc);"));
> + format(" functionCall ( {int i;int j;}, aaaa, bbbb,
> cccc);"));
> + EXPECT_EQ("functionCall(aaaa, bbbb, { int i; });",
> + format(" functionCall (aaaa, bbbb, {int i;});"));
> + EXPECT_EQ("functionCall(aaaa, bbbb, {\n"
> + " int i;\n"
> + " int j;\n"
> + "});",
> + format(" functionCall (aaaa, bbbb, {int i;int j;});"));
> + EXPECT_EQ("functionCall(aaaa, bbbb, { int i; });",
> + format(" functionCall (aaaa, bbbb, {int i;});"));
> verifyFormat(
> "Aaa({\n"
> - " int i;\n"
> - "},\n"
> + " int i; // break\n"
> + " },\n"
> "
> aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa(bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb,\n"
> " ccccccccccccccccc));");
> + verifyFormat("DEBUG({\n"
> + " if (a)\n"
> + " f();\n"
> + "});");
> }
>
> TEST_F(FormatTest, LayoutBlockInsideStatement) {
> @@ -3681,7 +3697,7 @@ TEST_F(FormatTest, UnderstandsUsesOfStar
> verifyGoogleFormat("return sizeof(int**);");
> verifyIndependentOfContext("Type **A = static_cast<Type **>(P);");
> verifyGoogleFormat("Type** A = static_cast<Type**>(P);");
> - verifyFormat("auto a = [](int **&, int ***) {\n};");
> + verifyFormat("auto a = [](int **&, int ***) {};");
>
> verifyIndependentOfContext("InvalidRegions[*R] = 0;");
>
> @@ -3865,7 +3881,7 @@ TEST_F(FormatTest, FormatsCasts) {
> verifyFormat("f(foo).b;");
> verifyFormat("f(foo)(b);");
> verifyFormat("f(foo)[b];");
> - verifyFormat("[](foo) {\n return 4;\n}(bar);");
> + verifyFormat("[](foo) { return 4; }(bar);");
> verifyFormat("(*funptr)(foo)[4];");
> verifyFormat("funptrs[4](foo)[4];");
> verifyFormat("void f(int *);");
> @@ -6260,68 +6276,40 @@ TEST_F(FormatTest, FormatsProtocolBuffer
> }
>
> TEST_F(FormatTest, FormatsLambdas) {
> - verifyFormat(
> - "int c = [b]() mutable {\n"
> - " return [&b] {\n"
> - " return b++;\n"
> - " }();\n"
> - "}();\n");
> - verifyFormat(
> - "int c = [&] {\n"
> - " [=] {\n"
> - " return b++;\n"
> - " }();\n"
> - "}();\n");
> - verifyFormat(
> - "int c = [&, &a, a] {\n"
> - " [=, c, &d] {\n"
> - " return b++;\n"
> - " }();\n"
> - "}();\n");
> - verifyFormat(
> - "int c = [&a, &a, a] {\n"
> - " [=, a, b, &c] {\n"
> - " return b++;\n"
> - " }();\n"
> - "}();\n");
> - verifyFormat(
> - "auto c = {[&a, &a, a] {\n"
> - " [=, a, b, &c] {\n"
> - " return b++;\n"
> - " }();\n"
> - "} }\n");
> - verifyFormat(
> - "auto c = {[&a, &a, a] {\n"
> - " [=, a, b, &c] {\n"
> - " }();\n"
> - "} }\n");
> - verifyFormat(
> - "void f() {\n"
> - " other(x.begin(), x.end(), [&](int, int) {\n"
> - " return 1;\n"
> - " });\n"
> - "}\n");
> + verifyFormat("int c = [b]() mutable {\n"
> + " return [&b] { return b++; }();\n"
> + "}();\n");
> + verifyFormat("int c = [&] {\n"
> + " [=] { return b++; }();\n"
> + "}();\n");
> + verifyFormat("int c = [&, &a, a] {\n"
> + " [=, c, &d] { return b++; }();\n"
> + "}();\n");
> + verifyFormat("int c = [&a, &a, a] {\n"
> + " [=, a, b, &c] { return b++; }();\n"
> + "}();\n");
> + verifyFormat("auto c = { [&a, &a, a] {\n"
> + " [=, a, b, &c] { return b++; }();\n"
> + "} }\n");
> + verifyFormat("auto c = { [&a, &a, a] { [=, a, b, &c] { }(); } }\n");
> + verifyFormat("void f() {\n"
> + " other(x.begin(), x.end(), [&](int, int) { return 1;
> });\n"
> + "}\n");
> // FIXME: The formatting is incorrect; this test currently checks that
> // parsing of the unwrapped lines doesn't regress.
> - verifyFormat(
> - "void f() {\n"
> - " other(x.begin(), //\n"
> - " x.end(), //\n"
> - " [&](int, int) {\n"
> - " return 1;\n"
> - " });\n"
> - "}\n");
> + verifyFormat("void f() {\n"
> + " other(x.begin(), //\n"
> + " x.end(), //\n"
> + " [&](int, int) { return 1; });\n"
> + "}\n");
> }
>
> TEST_F(FormatTest, FormatsBlocks) {
> // FIXME: Make whitespace formatting consistent. Ask a ObjC dev how
> // it would ideally look.
> - verifyFormat("[operation setCompletionBlock:^{\n"
> - " [self onOperationDone];\n"
> - "}];\n");
> - verifyFormat("int i = {[operation setCompletionBlock : ^{\n"
> - " [self onOperationDone];\n"
> - "}] };\n");
> + verifyFormat("[operation setCompletionBlock:^{ [self onOperationDone];
> }];");
> + verifyFormat("int i = {[operation setCompletionBlock : ^{ [self "
> + "onOperationDone]; }] };");
> }
>
> } // end namespace tooling
>
>
> _______________________________________________
> 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/20130905/d2aa4d43/attachment.html>
More information about the cfe-commits
mailing list