Typz updated this revision to Diff 100698.
Typz marked an inline comment as done.
Typz added a comment.
fix code & tests
https://reviews.llvm.org/D33589
Files:
lib/Format/ContinuationIndenter.cpp
lib/Format/ContinuationIndenter.h
lib/Format/FormatToken.cpp
lib/Format/FormatToken.h
lib/Format/UnwrappedLineFormatter.cpp
unittests/Format/FormatTest.cpp
Index: unittests/Format/FormatTest.cpp
===================================================================
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -8542,6 +8542,45 @@
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);
+ EXPECT_EQ("int a; /* first line\n"
+ " * second\n"
+ " * line third\n"
+ " * line\n"
+ " */",
+ format("int a; /* first line\n"
+ " * second\n"
+ " * line third\n"
+ " * line\n"
+ " */", Style));
+
+ Style.PenaltyExcessCharacter = 90;
+ verifyFormat("int a; // the comment", Style);
+ EXPECT_EQ("int a; // the\n"
+ " // comment aa",
+ format("int a; // the comment aa", Style));
+ EXPECT_EQ("int a; /* first line\n"
+ " * second line\n"
+ " * third line\n"
+ " */",
+ format("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"
+ "\n*/", 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() \
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: 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/ContinuationIndenter.h
===================================================================
--- lib/Format/ContinuationIndenter.h
+++ lib/Format/ContinuationIndenter.h
@@ -56,15 +56,18 @@
/// \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.
///
/// Puts the token on the current line if \p Newline is \c false and adds a
/// line break and necessary indentation otherwise.
///
/// 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)
@@ -1242,6 +1249,13 @@
RemainingSpace, SplitBefore, Whitespaces);
RemainingTokenColumns = Token->getLineLengthAfterSplitBefore(
LineIndex, TailOffset, RemainingTokenColumns, ColumnLimit, SplitBefore);
+ if (!BreakToken) {
+ // The last line's penalty is handled in addNextStateToQueue().
+ if (RemainingTokenColumns > RemainingSpace && LineIndex < EndIndex - 1)
+ Penalty += Style.PenaltyExcessCharacter *
+ (RemainingTokenColumns - RemainingSpace);
+ continue;
+ }
while (RemainingTokenColumns > RemainingSpace) {
BreakableToken::Split Split = Token->getSplit(
LineIndex, TailOffset, ColumnLimit, CommentPragmasRegex);
@@ -1267,6 +1281,7 @@
break;
}
+
unsigned NewRemainingTokenColumns = Token->getLineLengthAfterSplit(
LineIndex, TailOffset + Split.first + Split.second, StringRef::npos);
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
