Better trade-off for excess characters vs. staying within the column limits.
When we break a long line like:
Column limit: 21
|
// foo foo foo foo foo foo foo foo foo foo foo foo
The local decision when to allow protruding vs. breaking can lead to this
outcome (2 excess characters, 2 breaks):
// foo foo foo foo foo
// foo foo foo foo foo
// foo foo
While strictly staying within the column limit leads to this strictly better
outcome (fully below the column limit, 2 breaks):
// foo foo foo foo
// foo foo foo foo
// foo foo foo foo
To get an optimal solution, we would need to consider all combinations of excess
characters vs. breaking for all lines, but that would lead to a significant
increase in the search space of the algorithm for little gain.
Instead, we blindly try both approches and·select the one that leads to the
overall lower penalty.
Differential Revision: https://reviews.llvm.org/D40605
llvm-svn: 319541
diff --git a/clang/lib/Format/ContinuationIndenter.cpp b/clang/lib/Format/ContinuationIndenter.cpp
index c0d2bf6..e022a3a 100644
--- a/clang/lib/Format/ContinuationIndenter.cpp
+++ b/clang/lib/Format/ContinuationIndenter.cpp
@@ -1390,7 +1390,36 @@
Penalty = addMultilineToken(Current, State);
} else if (State.Line->Type != LT_ImportStatement) {
// We generally don't break import statements.
- Penalty = breakProtrudingToken(Current, State, AllowBreak, DryRun);
+ LineState OriginalState = State;
+
+ // Whether we force the reflowing algorithm to stay strictly within the
+ // column limit.
+ bool Strict = false;
+ // Whether the first non-strict attempt at reflowing did intentionally
+ // exceed the column limit.
+ bool Exceeded = false;
+ std::tie(Penalty, Exceeded) = breakProtrudingToken(
+ Current, State, AllowBreak, /*DryRun=*/true, Strict);
+ if (Exceeded) {
+ // If non-strict reflowing exceeds the column limit, try whether strict
+ // reflowing leads to an overall lower penalty.
+ LineState StrictState = OriginalState;
+ unsigned StrictPenalty =
+ breakProtrudingToken(Current, StrictState, AllowBreak,
+ /*DryRun=*/true, /*Strict=*/true)
+ .first;
+ Strict = StrictPenalty <= Penalty;
+ if (Strict) {
+ Penalty = StrictPenalty;
+ State = StrictState;
+ }
+ }
+ if (!DryRun) {
+ // If we're not in dry-run mode, apply the changes with the decision on
+ // strictness made above.
+ breakProtrudingToken(Current, OriginalState, AllowBreak, /*DryRun=*/false,
+ Strict);
+ }
}
if (State.Column > getColumnLimit(State)) {
unsigned ExcessCharacters = State.Column - getColumnLimit(State);
@@ -1480,14 +1509,14 @@
return nullptr;
}
-unsigned ContinuationIndenter::breakProtrudingToken(const FormatToken &Current,
- LineState &State,
- bool AllowBreak,
- bool DryRun) {
+std::pair<unsigned, bool>
+ContinuationIndenter::breakProtrudingToken(const FormatToken &Current,
+ LineState &State, bool AllowBreak,
+ bool DryRun, bool Strict) {
std::unique_ptr<const BreakableToken> Token =
createBreakableToken(Current, State, AllowBreak);
if (!Token)
- return 0;
+ return {0, false};
assert(Token->getLineCount() > 0);
unsigned ColumnLimit = getColumnLimit(State);
if (Current.is(TT_LineComment)) {
@@ -1495,13 +1524,16 @@
ColumnLimit = Style.ColumnLimit;
}
if (Current.UnbreakableTailLength >= ColumnLimit)
- return 0;
+ return {0, false};
// ColumnWidth was already accounted into State.Column before calling
// breakProtrudingToken.
unsigned StartColumn = State.Column - Current.ColumnWidth;
unsigned NewBreakPenalty = Current.isStringLiteral()
? Style.PenaltyBreakString
: Style.PenaltyBreakComment;
+ // Stores whether we intentionally decide to let a line exceed the column
+ // limit.
+ bool Exceeded = false;
// Stores whether we introduce a break anywhere in the token.
bool BreakInserted = Token->introducesBreakBeforeToken();
// Store whether we inserted a new line break at the end of the previous
@@ -1612,7 +1644,7 @@
bool ContinueOnLine =
ContentStartColumn + ToNextSplitColumns <= ColumnLimit;
unsigned ExcessCharactersPenalty = 0;
- if (!ContinueOnLine) {
+ if (!ContinueOnLine && !Strict) {
// Similarly, if the excess characters' penalty is lower than the
// penalty of introducing a new break, continue on the current line.
ExcessCharactersPenalty =
@@ -1621,8 +1653,10 @@
DEBUG(llvm::dbgs()
<< " Penalty excess: " << ExcessCharactersPenalty
<< "\n break : " << NewBreakPenalty << "\n");
- if (ExcessCharactersPenalty < NewBreakPenalty)
+ if (ExcessCharactersPenalty < NewBreakPenalty) {
+ Exceeded = true;
ContinueOnLine = true;
+ }
}
if (ContinueOnLine) {
DEBUG(llvm::dbgs() << " Continuing on line...\n");
@@ -1817,7 +1851,7 @@
Token->updateNextToken(State);
- return Penalty;
+ return {Penalty, Exceeded};
}
unsigned ContinuationIndenter::getColumnLimit(const LineState &State) const {