Index: lib/Format/ContinuationIndenter.h =================================================================== --- lib/Format/ContinuationIndenter.h +++ lib/Format/ContinuationIndenter.h @@ -56,6 +56,9 @@ /// \brief Returns \c true, if a line break after \p State is mandatory. bool mustBreak(const LineState &State); + /// \brief Returns \c true, if the first token after \p State can be split. + bool canSplit(LineState &State); + /// \brief Appends the next token to \p State and updates information /// necessary for indentation. /// @@ -64,7 +67,7 @@ /// /// If \p DryRun is \c false, also creates and stores the required /// \c Replacement. - unsigned addTokenToState(LineState &State, bool Newline, bool DryRun, + unsigned addTokenToState(LineState &State, bool Newline, bool BreakToken, bool DryRun, unsigned ExtraSpaces = 0); /// \brief Get the column limit for this line. This is the style's column @@ -74,7 +77,8 @@ private: /// \brief Mark the next token as consumed in \p State and modify its stacks /// accordingly. - unsigned moveStateToNextToken(LineState &State, bool DryRun, bool Newline); + unsigned moveStateToNextToken(LineState &State, bool DryRun, bool Newline, + bool BreakToken); /// \brief Update 'State' according to the next token's fake left parentheses. void moveStatePastFakeLParens(LineState &State, bool Newline); @@ -97,7 +101,7 @@ /// column limit violation in all lines except for the last one. The penalty /// for the column limit violation in the last line (and in single line /// tokens) is handled in \c addNextStateToQueue. - unsigned breakProtrudingToken(const FormatToken &Current, LineState &State, + unsigned breakProtrudingToken(const FormatToken &Current, LineState &State, bool BreakToken, bool DryRun); /// \brief Appends the next token to \p State and updates information Index: lib/Format/ContinuationIndenter.cpp =================================================================== --- lib/Format/ContinuationIndenter.cpp +++ lib/Format/ContinuationIndenter.cpp @@ -92,7 +92,7 @@ State.IgnoreStackForComparison = false; // The first token has already been indented and thus consumed. - moveStateToNextToken(State, DryRun, /*Newline=*/false); + moveStateToNextToken(State, DryRun, /*Newline=*/false, /*BreakToken*/true); return State; } @@ -291,8 +291,14 @@ return false; } +bool ContinuationIndenter::canSplit(LineState & State) +{ + return !State.Stack.back().NoLineBreak && + !State.Stack.back().NoLineBreakInOperand; +} + unsigned ContinuationIndenter::addTokenToState(LineState &State, bool Newline, - bool DryRun, + bool BreakToken, bool DryRun, unsigned ExtraSpaces) { const FormatToken &Current = *State.NextToken; @@ -313,7 +319,7 @@ assert(EndColumn >= StartColumn); State.Column += EndColumn - StartColumn; } - moveStateToNextToken(State, DryRun, /*Newline=*/false); + moveStateToNextToken(State, DryRun, /*Newline=*/false, /*BreakToken=*/true); return 0; } @@ -323,7 +329,7 @@ else addTokenOnCurrentLine(State, DryRun, ExtraSpaces); - return moveStateToNextToken(State, DryRun, Newline) + Penalty; + return moveStateToNextToken(State, DryRun, Newline, BreakToken) + Penalty; } void ContinuationIndenter::addTokenOnCurrentLine(LineState &State, bool DryRun, @@ -761,7 +767,8 @@ } unsigned ContinuationIndenter::moveStateToNextToken(LineState &State, - bool DryRun, bool Newline) { + bool DryRun, bool Newline, + bool BreakToken) { assert(State.Stack.size()); const FormatToken &Current = *State.NextToken; @@ -876,7 +883,7 @@ State.NextToken = State.NextToken->Next; unsigned Penalty = 0; if (CanBreakProtrudingToken) - Penalty = breakProtrudingToken(Current, State, DryRun); + Penalty = breakProtrudingToken(Current, State, BreakToken, DryRun); if (State.Column > getColumnLimit(State)) { unsigned ExcessCharacters = State.Column - getColumnLimit(State); Penalty += Style.PenaltyExcessCharacter * ExcessCharacters; @@ -890,7 +897,7 @@ // after the "{" is already done and both options are tried and evaluated. // FIXME: This is ugly, find a better way. if (Previous && Previous->Role) - Penalty += Previous->Role->formatAfterToken(State, this, DryRun); + Penalty += Previous->Role->formatAfterToken(State, this, BreakToken, DryRun); return Penalty; } @@ -1134,7 +1141,7 @@ unsigned ContinuationIndenter::breakProtrudingToken(const FormatToken &Current, LineState &State, - bool DryRun) { + bool BreakToken, bool DryRun) { // Don't break multi-line tokens other than block comments. Instead, just // update the state. if (Current.isNot(TT_BlockComment) && Current.IsMultiline) @@ -1267,6 +1274,12 @@ break; } + if (!BreakToken) { + Penalty += Style.PenaltyExcessCharacter * + (RemainingTokenColumns - RemainingSpace); + break; + } + unsigned NewRemainingTokenColumns = Token->getLineLengthAfterSplit( LineIndex, TailOffset + Split.first + Split.second, StringRef::npos); Index: lib/Format/FormatToken.h =================================================================== --- lib/Format/FormatToken.h +++ lib/Format/FormatToken.h @@ -534,7 +534,7 @@ /// already been set thereby deciding on the first line break. virtual unsigned formatAfterToken(LineState &State, ContinuationIndenter *Indenter, - bool DryRun) { + bool BreakToken, bool DryRun) { return 0; } @@ -553,7 +553,7 @@ void precomputeFormattingInfos(const FormatToken *Token) override; unsigned formatAfterToken(LineState &State, ContinuationIndenter *Indenter, - bool DryRun) override; + bool BreakToken, bool DryRun) override; unsigned formatFromToken(LineState &State, ContinuationIndenter *Indenter, bool DryRun) override; Index: lib/Format/FormatToken.cpp =================================================================== --- lib/Format/FormatToken.cpp +++ lib/Format/FormatToken.cpp @@ -73,7 +73,7 @@ unsigned CommaSeparatedList::formatAfterToken(LineState &State, ContinuationIndenter *Indenter, - bool DryRun) { + bool BreakToken, bool DryRun) { if (State.NextToken == nullptr || !State.NextToken->Previous) return 0; @@ -125,7 +125,8 @@ } // Place token using the continuation indenter and store the penalty. - Penalty += Indenter->addTokenToState(State, NewLine, DryRun, ExtraSpaces); + Penalty += Indenter->addTokenToState(State, NewLine, BreakToken, DryRun, + ExtraSpaces); } return Penalty; } Index: lib/Format/UnwrappedLineFormatter.cpp =================================================================== --- lib/Format/UnwrappedLineFormatter.cpp +++ lib/Format/UnwrappedLineFormatter.cpp @@ -590,7 +590,8 @@ (Indenter->canBreak(State) && State.NextToken->NewlinesBefore > 0); unsigned Penalty = 0; formatChildren(State, Newline, /*DryRun=*/false, Penalty); - Indenter->addTokenToState(State, Newline, /*DryRun=*/false); + Indenter->addTokenToState(State, Newline, /*BreakToken=*/true, + /*DryRun=*/false); } return 0; } @@ -611,7 +612,8 @@ LineState State = Indenter->getInitialState(FirstIndent, &Line, DryRun); while (State.NextToken) { formatChildren(State, /*Newline=*/false, DryRun, Penalty); - Indenter->addTokenToState(State, /*Newline=*/false, DryRun); + Indenter->addTokenToState(State, /*Newline=*/false, /*BreakToken=*/true, + DryRun); } return Penalty; } @@ -658,10 +660,11 @@ /// \brief An edge in the solution space from \c Previous->State to \c State, /// inserting a newline dependent on the \c NewLine. struct StateNode { - StateNode(const LineState &State, bool NewLine, StateNode *Previous) - : State(State), NewLine(NewLine), Previous(Previous) {} + StateNode(const LineState &State, bool NewLine, bool BreakToken, StateNode *Previous) + : State(State), NewLine(NewLine), BreakToken(BreakToken), Previous(Previous) {} LineState State; bool NewLine; + bool BreakToken; StateNode *Previous; }; @@ -691,7 +694,7 @@ // Insert start element into queue. StateNode *Node = - new (Allocator.Allocate()) StateNode(InitialState, false, nullptr); + new (Allocator.Allocate()) StateNode(InitialState, false, true, nullptr); Queue.push(QueueItem(OrderedPenalty(0, Count), Node)); ++Count; @@ -718,9 +721,17 @@ FormatDecision LastFormat = Node->State.NextToken->Decision; if (LastFormat == FD_Unformatted || LastFormat == FD_Continue) - addNextStateToQueue(Penalty, Node, /*NewLine=*/false, &Count, &Queue); + addNextStateToQueue(Penalty, Node, /*NewLine=*/false, + /*BreakToken=*/true, &Count, &Queue); if (LastFormat == FD_Unformatted || LastFormat == FD_Break) - addNextStateToQueue(Penalty, Node, /*NewLine=*/true, &Count, &Queue); + addNextStateToQueue(Penalty, Node, /*NewLine=*/true, + /*BreakToken=*/true, &Count, &Queue); + if (LastFormat == FD_Unformatted || LastFormat == FD_Continue) + addNextStateToQueue(Penalty, Node, /*NewLine=*/false, + /*BreakToken=*/false, &Count, &Queue); + if (LastFormat == FD_Unformatted || LastFormat == FD_Break) + addNextStateToQueue(Penalty, Node, /*NewLine=*/true, + /*BreakToken=*/false, &Count, &Queue); } if (Queue.empty()) { @@ -745,18 +756,20 @@ /// Assume the current state is \p PreviousNode and has been reached with a /// penalty of \p Penalty. Insert a line break if \p NewLine is \c true. void addNextStateToQueue(unsigned Penalty, StateNode *PreviousNode, - bool NewLine, unsigned *Count, QueueType *Queue) { + bool NewLine, bool BreakToken, unsigned *Count, QueueType *Queue) { if (NewLine && !Indenter->canBreak(PreviousNode->State)) return; if (!NewLine && Indenter->mustBreak(PreviousNode->State)) return; + if (!BreakToken && !Indenter->canSplit(PreviousNode->State)) + return; StateNode *Node = new (Allocator.Allocate()) - StateNode(PreviousNode->State, NewLine, PreviousNode); + StateNode(PreviousNode->State, NewLine, BreakToken, PreviousNode); if (!formatChildren(Node->State, NewLine, /*DryRun=*/true, Penalty)) return; - Penalty += Indenter->addTokenToState(Node->State, NewLine, true); + Penalty += Indenter->addTokenToState(Node->State, NewLine, BreakToken, true); Queue->push(QueueItem(OrderedPenalty(Penalty, *Count), Node)); ++(*Count); @@ -775,7 +788,7 @@ I != E; ++I) { unsigned Penalty = 0; formatChildren(State, (*I)->NewLine, /*DryRun=*/false, Penalty); - Penalty += Indenter->addTokenToState(State, (*I)->NewLine, false); + Penalty += Indenter->addTokenToState(State, (*I)->NewLine, (*I)->BreakToken, false); DEBUG({ printLineState((*I)->Previous->State); Index: unittests/Format/FormatTest.cpp =================================================================== --- unittests/Format/FormatTest.cpp +++ unittests/Format/FormatTest.cpp @@ -8542,6 +8542,35 @@ EXPECT_EQ("#pragma option -C -A", format("#pragma option -C -A")); } +TEST_F(FormatTest, OptimizeBreakPenaltyVsExcess) { + FormatStyle Style = getLLVMStyle(); + Style.ColumnLimit = 20; + + verifyFormat("int a; // the\n" + " // comment", Style); + verifyFormat("int a; /* first line\n" + " * second\n" + " * line third\n" + " * line\n" + " */", Style); + + Style.PenaltyExcessCharacter = 30; + verifyFormat("int a; // the comment", Style); + EXPECT_EQ("int a; // the\n" + " // comment aa", + format("int a; // the comment aa", Style)); + verifyFormat("int a; /* first line\n" + " * second line\n" + " * third line\n" + " */", Style); + EXPECT_EQ("int a; /* first line\n" + " * second\n" + " * line third\n" + " * line\n" + " */", + format("int a; /* first line second line third line */", Style)); +} + #define EXPECT_ALL_STYLES_EQUAL(Styles) \ for (size_t i = 1; i < Styles.size(); ++i) \ EXPECT_EQ(Styles[0], Styles[i]) << "Style #" << i << " of " << Styles.size() \