Skip to content

Commit 75c85ba

Browse files
authored
[clang-format] Continue aligned lines without parentheses (#167979)
before, with the options `AlignConsecutiveDeclarations` and `AlignConsecutiveAssignments` enabled ```C++ veryverylongvariablename = somethingelse; shortervariablename = anotherverylonglonglongvariablename + // somevariablethatwastoolongtofitonthesamerow; double i234 = 0; auto v = false ? type{} : type{ 1, }; ``` after ```C++ veryverylongvariablename = somethingelse; shortervariablename = anotherverylonglonglongvariablename + // somevariablethatwastoolongtofitonthesamerow; double i234 = 0; auto v = false ? type{} : type{ 1, }; ``` Fixes #126873. Fixes #57612. Previously, the part for determining whether aligning a line should move the next line relied on having a pair of tokens such as parentheses surrounding both lines. There are often no such tokens. For example in the first block above. This patch removes the requirement for those tokens. Now the program keeps track of how the position is calculated. The alignment step moves the next line if its position is based on a column to the right of the token that gets aligned. The column that the position of the line is based on is more detailed than the `IsAligned` property that the program used before this patch. It enables the program to handle cases where parts that should not usually move with the previous line and parts that should are nested like in the second block above. That is why the patch uses it instead of fake parentheses.
1 parent f83f6f5 commit 75c85ba

File tree

7 files changed

+259
-72
lines changed

7 files changed

+259
-72
lines changed

clang/lib/Format/ContinuationIndenter.cpp

Lines changed: 85 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -240,6 +240,45 @@ RawStringFormatStyleManager::getEnclosingFunctionStyle(
240240
return It->second;
241241
}
242242

243+
IndentationAndAlignment
244+
IndentationAndAlignment::addPadding(unsigned Spaces) const {
245+
return IndentationAndAlignment(Total + Spaces, IndentedFrom);
246+
}
247+
248+
IndentationAndAlignment
249+
IndentationAndAlignment::operator+(unsigned Spaces) const {
250+
return IndentationAndAlignment(Total + Spaces, Total);
251+
}
252+
253+
IndentationAndAlignment
254+
IndentationAndAlignment::operator-(unsigned Spaces) const {
255+
return IndentationAndAlignment(Total - Spaces, Total);
256+
}
257+
258+
IndentationAndAlignment &IndentationAndAlignment::operator+=(unsigned Spaces) {
259+
*this = *this + Spaces;
260+
return *this;
261+
}
262+
263+
IndentationAndAlignment::IndentationAndAlignment(unsigned Total,
264+
unsigned IndentedFrom)
265+
: Total(Total), IndentedFrom(IndentedFrom) {}
266+
267+
IndentationAndAlignment::IndentationAndAlignment(unsigned Spaces)
268+
: Total(Spaces), IndentedFrom(Spaces) {}
269+
270+
bool IndentationAndAlignment::operator<(
271+
const IndentationAndAlignment &Other) const {
272+
if (Total != Other.Total)
273+
return Total < Other.Total;
274+
// The sign to use here was decided arbitrarily. This operator is mostly used
275+
// when a line's indentation should be the max of 2 things. Using this sign
276+
// here makes the program prefer alignment over continuation indentation. That
277+
// is, it makes the alignment step that follows prefer to move the line when
278+
// aligning the previous line.
279+
return IndentedFrom > Other.IndentedFrom;
280+
}
281+
243282
ContinuationIndenter::ContinuationIndenter(const FormatStyle &Style,
244283
const AdditionalKeywords &Keywords,
245284
const SourceManager &SourceMgr,
@@ -491,7 +530,7 @@ bool ContinuationIndenter::mustBreak(const LineState &State) {
491530
return true;
492531
}
493532

494-
unsigned NewLineColumn = getNewLineColumn(State);
533+
unsigned NewLineColumn = getNewLineColumn(State).Total;
495534
if (Current.isMemberAccess() && Style.ColumnLimit != 0 &&
496535
State.Column + getLengthToNextOperator(Current) > Style.ColumnLimit &&
497536
(State.Column > NewLineColumn ||
@@ -819,8 +858,9 @@ void ContinuationIndenter::addTokenOnCurrentLine(LineState &State, bool DryRun,
819858
}
820859

821860
if (Current.is(TT_SelectorName) && !CurrentState.ObjCSelectorNameFound) {
822-
unsigned MinIndent = std::max(
823-
State.FirstIndent + Style.ContinuationIndentWidth, CurrentState.Indent);
861+
unsigned MinIndent =
862+
std::max(State.FirstIndent + Style.ContinuationIndentWidth,
863+
CurrentState.Indent.Total);
824864
unsigned FirstColonPos = State.Column + Spaces + Current.ColumnWidth;
825865
if (Current.LongestObjCSelectorName == 0)
826866
CurrentState.AlignColons = false;
@@ -910,7 +950,8 @@ void ContinuationIndenter::addTokenOnCurrentLine(LineState &State, bool DryRun,
910950
return !Next || Next->isMemberAccess() ||
911951
Next->is(TT_FunctionDeclarationLParen) || IsFunctionCallParen(*Next);
912952
};
913-
if (IsOpeningBracket(Previous) && State.Column > getNewLineColumn(State) &&
953+
if (IsOpeningBracket(Previous) &&
954+
State.Column > getNewLineColumn(State).Total &&
914955
// Don't do this for simple (no expressions) one-argument function calls
915956
// as that feels like needlessly wasting whitespace, e.g.:
916957
//
@@ -955,7 +996,7 @@ void ContinuationIndenter::addTokenOnCurrentLine(LineState &State, bool DryRun,
955996
CurrentState.NoLineBreak = true;
956997

957998
if (startsSegmentOfBuilderTypeCall(Current) &&
958-
State.Column > getNewLineColumn(State)) {
999+
State.Column > getNewLineColumn(State).Total) {
9591000
CurrentState.ContainsUnwrappedBuilder = true;
9601001
}
9611002

@@ -1086,7 +1127,8 @@ unsigned ContinuationIndenter::addTokenOnNewLine(LineState &State,
10861127
Penalty += Style.PenaltyBreakFirstLessLess;
10871128
}
10881129

1089-
State.Column = getNewLineColumn(State);
1130+
const auto [TotalColumn, IndentedFromColumn] = getNewLineColumn(State);
1131+
State.Column = TotalColumn;
10901132

10911133
// Add Penalty proportional to amount of whitespace away from FirstColumn
10921134
// This tends to penalize several lines that are far-right indented,
@@ -1132,9 +1174,9 @@ unsigned ContinuationIndenter::addTokenOnNewLine(LineState &State,
11321174
} else {
11331175
CurrentState.ColonPos =
11341176
(shouldIndentWrappedSelectorName(Style, State.Line->Type)
1135-
? std::max(CurrentState.Indent,
1177+
? std::max(CurrentState.Indent.Total,
11361178
State.FirstIndent + Style.ContinuationIndentWidth)
1137-
: CurrentState.Indent) +
1179+
: CurrentState.Indent.Total) +
11381180
std::max(NextNonComment->LongestObjCSelectorName,
11391181
NextNonComment->ColumnWidth);
11401182
}
@@ -1155,7 +1197,7 @@ unsigned ContinuationIndenter::addTokenOnNewLine(LineState &State,
11551197
// when we consume all of the "}"'s FakeRParens at the "{".
11561198
if (State.Stack.size() > 1) {
11571199
State.Stack[State.Stack.size() - 2].LastSpace =
1158-
std::max(CurrentState.LastSpace, CurrentState.Indent) +
1200+
std::max(CurrentState.LastSpace, CurrentState.Indent.Total) +
11591201
Style.ContinuationIndentWidth;
11601202
}
11611203
}
@@ -1196,7 +1238,8 @@ unsigned ContinuationIndenter::addTokenOnNewLine(LineState &State,
11961238
State.Line->Type != LT_ImportStatement &&
11971239
Current.isNot(TT_LineComment);
11981240
Whitespaces.replaceWhitespace(Current, Newlines, State.Column, State.Column,
1199-
CurrentState.IsAligned, ContinuePPDirective);
1241+
CurrentState.IsAligned, ContinuePPDirective,
1242+
IndentedFromColumn);
12001243
}
12011244

12021245
if (!Current.isTrailingComment())
@@ -1340,7 +1383,8 @@ unsigned ContinuationIndenter::addTokenOnNewLine(LineState &State,
13401383
return Penalty;
13411384
}
13421385

1343-
unsigned ContinuationIndenter::getNewLineColumn(const LineState &State) {
1386+
IndentationAndAlignment
1387+
ContinuationIndenter::getNewLineColumn(const LineState &State) {
13441388
if (!State.NextToken || !State.NextToken->Previous)
13451389
return 0;
13461390

@@ -1354,8 +1398,9 @@ unsigned ContinuationIndenter::getNewLineColumn(const LineState &State) {
13541398

13551399
const FormatToken &Previous = *Current.Previous;
13561400
// If we are continuing an expression, we want to use the continuation indent.
1357-
unsigned ContinuationIndent =
1358-
std::max(CurrentState.LastSpace, CurrentState.Indent) +
1401+
const auto ContinuationIndent =
1402+
std::max(IndentationAndAlignment(CurrentState.LastSpace),
1403+
CurrentState.Indent) +
13591404
Style.ContinuationIndentWidth;
13601405
const FormatToken *PreviousNonComment = Current.getPreviousNonComment();
13611406
const FormatToken *NextNonComment = Previous.getNextNonComment();
@@ -1365,7 +1410,7 @@ unsigned ContinuationIndenter::getNewLineColumn(const LineState &State) {
13651410
// Java specific bits.
13661411
if (Style.isJava() &&
13671412
Current.isOneOf(Keywords.kw_implements, Keywords.kw_extends)) {
1368-
return std::max(CurrentState.LastSpace,
1413+
return std::max(IndentationAndAlignment(CurrentState.LastSpace),
13691414
CurrentState.Indent + Style.ContinuationIndentWidth);
13701415
}
13711416

@@ -1378,7 +1423,8 @@ unsigned ContinuationIndenter::getNewLineColumn(const LineState &State) {
13781423

13791424
if (Style.BreakBeforeBraces == FormatStyle::BS_Whitesmiths &&
13801425
State.Line->First->is(tok::kw_enum)) {
1381-
return (Style.IndentWidth * State.Line->First->IndentLevel) +
1426+
return IndentationAndAlignment(Style.IndentWidth *
1427+
State.Line->First->IndentLevel) +
13821428
Style.IndentWidth;
13831429
}
13841430

@@ -1497,7 +1543,7 @@ unsigned ContinuationIndenter::getNewLineColumn(const LineState &State) {
14971543
// * not remove the 'lead' ContinuationIndentWidth
14981544
// * always un-indent by the operator when
14991545
// BreakBeforeTernaryOperators=true
1500-
unsigned Indent = CurrentState.Indent;
1546+
unsigned Indent = CurrentState.Indent.Total;
15011547
if (Style.AlignOperands != FormatStyle::OAS_DontAlign)
15021548
Indent -= Style.ContinuationIndentWidth;
15031549
if (Style.BreakBeforeTernaryOperators && CurrentState.UnindentOperator)
@@ -1537,14 +1583,16 @@ unsigned ContinuationIndenter::getNewLineColumn(const LineState &State) {
15371583
TT_LeadingJavaAnnotation))) ||
15381584
(!Style.IndentWrappedFunctionNames &&
15391585
NextNonComment->isOneOf(tok::kw_operator, TT_FunctionDeclarationName))) {
1540-
return std::max(CurrentState.LastSpace, CurrentState.Indent);
1586+
return std::max(IndentationAndAlignment(CurrentState.LastSpace),
1587+
CurrentState.Indent);
15411588
}
15421589
if (NextNonComment->is(TT_SelectorName)) {
15431590
if (!CurrentState.ObjCSelectorNameFound) {
1544-
unsigned MinIndent = CurrentState.Indent;
1591+
auto MinIndent = CurrentState.Indent;
15451592
if (shouldIndentWrappedSelectorName(Style, State.Line->Type)) {
1546-
MinIndent = std::max(MinIndent,
1547-
State.FirstIndent + Style.ContinuationIndentWidth);
1593+
MinIndent =
1594+
std::max(MinIndent, IndentationAndAlignment(State.FirstIndent) +
1595+
Style.ContinuationIndentWidth);
15481596
}
15491597
// If LongestObjCSelectorName is 0, we are indenting the first
15501598
// part of an ObjC selector (or a selector component which is
@@ -1555,10 +1603,10 @@ unsigned ContinuationIndenter::getNewLineColumn(const LineState &State) {
15551603
// component of the ObjC selector.
15561604
//
15571605
// In either case, we want to respect Style.IndentWrappedFunctionNames.
1558-
return MinIndent +
1559-
std::max(NextNonComment->LongestObjCSelectorName,
1560-
NextNonComment->ColumnWidth) -
1561-
NextNonComment->ColumnWidth;
1606+
return MinIndent.addPadding(
1607+
std::max(NextNonComment->LongestObjCSelectorName,
1608+
NextNonComment->ColumnWidth) -
1609+
NextNonComment->ColumnWidth);
15621610
}
15631611
if (!CurrentState.AlignColons)
15641612
return CurrentState.Indent;
@@ -1628,7 +1676,7 @@ unsigned ContinuationIndenter::getNewLineColumn(const LineState &State) {
16281676
return CurrentState.Indent - NextNonComment->Tok.getLength() -
16291677
NextNonComment->SpacesRequiredBefore;
16301678
}
1631-
if (CurrentState.Indent == State.FirstIndent && PreviousNonComment &&
1679+
if (CurrentState.Indent.Total == State.FirstIndent && PreviousNonComment &&
16321680
PreviousNonComment->isNoneOf(tok::r_brace, TT_CtorInitializerComma)) {
16331681
// Ensure that we fall back to the continuation indent width instead of
16341682
// just flushing continuations left.
@@ -1718,7 +1766,7 @@ unsigned ContinuationIndenter::moveStateToNextToken(LineState &State,
17181766
FormatStyle::BCIS_BeforeComma
17191767
? 0
17201768
: 2);
1721-
CurrentState.NestedBlockIndent = CurrentState.Indent;
1769+
CurrentState.NestedBlockIndent = CurrentState.Indent.Total;
17221770
if (Style.PackConstructorInitializers > FormatStyle::PCIS_BinPack) {
17231771
CurrentState.AvoidBinPacking = true;
17241772
CurrentState.BreakBeforeParameter =
@@ -1733,7 +1781,7 @@ unsigned ContinuationIndenter::moveStateToNextToken(LineState &State,
17331781
Style.BreakConstructorInitializers == FormatStyle::BCIS_AfterColon) {
17341782
CurrentState.Indent =
17351783
State.FirstIndent + Style.ConstructorInitializerIndentWidth;
1736-
CurrentState.NestedBlockIndent = CurrentState.Indent;
1784+
CurrentState.NestedBlockIndent = CurrentState.Indent.Total;
17371785
if (Style.PackConstructorInitializers > FormatStyle::PCIS_BinPack)
17381786
CurrentState.AvoidBinPacking = true;
17391787
else
@@ -1877,8 +1925,9 @@ void ContinuationIndenter::moveStatePastFakeLParens(LineState &State,
18771925
(!Style.isTableGen() ||
18781926
(Previous && Previous->isOneOf(TT_TableGenDAGArgListComma,
18791927
TT_TableGenDAGArgListCommaToBreak)))) {
1880-
NewParenState.Indent = std::max(
1881-
std::max(State.Column, NewParenState.Indent), CurrentState.LastSpace);
1928+
NewParenState.Indent =
1929+
std::max({IndentationAndAlignment(State.Column), NewParenState.Indent,
1930+
IndentationAndAlignment(CurrentState.LastSpace)});
18821931
}
18831932

18841933
// Special case for generic selection expressions, its comma-separated
@@ -1986,7 +2035,7 @@ void ContinuationIndenter::moveStatePastScopeOpener(LineState &State,
19862035
return Prev->is(tok::comma);
19872036
}(Current.MatchingParen);
19882037

1989-
unsigned NewIndent;
2038+
IndentationAndAlignment NewIndent = 0;
19902039
unsigned LastSpace = CurrentState.LastSpace;
19912040
bool AvoidBinPacking;
19922041
bool BreakBeforeParameter = false;
@@ -1999,7 +2048,7 @@ void ContinuationIndenter::moveStatePastScopeOpener(LineState &State,
19992048
std::min(State.Column, CurrentState.NestedBlockIndent);
20002049
} else if (Current.is(tok::l_brace)) {
20012050
const auto Width = Style.BracedInitializerIndentWidth;
2002-
NewIndent = CurrentState.LastSpace +
2051+
NewIndent = IndentationAndAlignment(CurrentState.LastSpace) +
20032052
(Width < 0 ? Style.ContinuationIndentWidth : Width);
20042053
} else {
20052054
NewIndent = CurrentState.LastSpace + Style.ContinuationIndentWidth;
@@ -2014,9 +2063,9 @@ void ContinuationIndenter::moveStatePastScopeOpener(LineState &State,
20142063
if (Current.ParameterCount > 1)
20152064
NestedBlockIndent = std::max(NestedBlockIndent, State.Column + 1);
20162065
} else {
2017-
NewIndent =
2018-
Style.ContinuationIndentWidth +
2019-
std::max(CurrentState.LastSpace, CurrentState.StartOfFunctionCall);
2066+
NewIndent = IndentationAndAlignment(std::max(
2067+
CurrentState.LastSpace, CurrentState.StartOfFunctionCall)) +
2068+
Style.ContinuationIndentWidth;
20202069

20212070
if (Style.isTableGen() && Current.is(TT_TableGenDAGArgOpenerToBreak) &&
20222071
Style.TableGenBreakInsideDAGArg == FormatStyle::DAS_BreakElements) {
@@ -2035,7 +2084,7 @@ void ContinuationIndenter::moveStatePastScopeOpener(LineState &State,
20352084
// FIXME: We likely want to do this for more combinations of brackets.
20362085
if (Current.is(tok::less) && Current.ParentBracket == tok::l_paren) {
20372086
NewIndent = std::max(NewIndent, CurrentState.Indent);
2038-
LastSpace = std::max(LastSpace, CurrentState.Indent);
2087+
LastSpace = std::max(LastSpace, CurrentState.Indent.Total);
20392088
}
20402089

20412090
// If ObjCBinPackProtocolList is unspecified, fall back to BinPackParameters
@@ -2281,7 +2330,7 @@ unsigned ContinuationIndenter::reformatRawStringLiteral(
22812330
unsigned CurrentIndent =
22822331
(!Newline && Current.Next && Current.Next->is(tok::r_paren))
22832332
? State.Stack.back().NestedBlockIndent
2284-
: State.Stack.back().Indent;
2333+
: State.Stack.back().Indent.Total;
22852334
unsigned NextStartColumn = ContentStartsOnNewline
22862335
? CurrentIndent + Style.IndentWidth
22872336
: FirstStartColumn;

clang/lib/Format/ContinuationIndenter.h

Lines changed: 43 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,41 @@ struct RawStringFormatStyleManager {
4343
getEnclosingFunctionStyle(StringRef EnclosingFunction) const;
4444
};
4545

46+
/// Represents the spaces at the start of a line, keeping track of what the
47+
/// spaces are for.
48+
struct IndentationAndAlignment {
49+
unsigned Total;
50+
51+
/// The column that the position of the start of the line is calculated
52+
/// from. It can be more than Total.
53+
unsigned IndentedFrom;
54+
55+
/// Add spaces for right-justifying the token. The IndentedFrom field does not
56+
/// change.
57+
///
58+
/// This example in Objective-C shows why the field should not change. The
59+
/// token `xx` is right-justified with this method to align the `:`
60+
/// symbols. The `:` symbols should remain aligned through the step that
61+
/// aligns assignments. That step uses the IndentedFrom field to tell what
62+
/// lines to move. Not changing the field in this method ensures that the 2
63+
/// lines move together.
64+
///
65+
/// [x //
66+
/// xxxx:0
67+
/// xx:0];
68+
IndentationAndAlignment addPadding(unsigned Spaces) const;
69+
/// Adding indentation is more common than padding. So the operator does that.
70+
IndentationAndAlignment operator+(unsigned Spaces) const;
71+
IndentationAndAlignment operator-(unsigned Spaces) const;
72+
IndentationAndAlignment &operator+=(unsigned Spaces);
73+
74+
IndentationAndAlignment(unsigned Total, unsigned IndentedFrom);
75+
76+
IndentationAndAlignment(unsigned Spaces);
77+
78+
bool operator<(const IndentationAndAlignment &Other) const;
79+
};
80+
4681
class ContinuationIndenter {
4782
public:
4883
/// Constructs a \c ContinuationIndenter to format \p Line starting in
@@ -168,7 +203,7 @@ class ContinuationIndenter {
168203
unsigned addTokenOnNewLine(LineState &State, bool DryRun);
169204

170205
/// Calculate the new column for a line wrap before the next token.
171-
unsigned getNewLineColumn(const LineState &State);
206+
IndentationAndAlignment getNewLineColumn(const LineState &State);
172207

173208
/// Adds a multiline token to the \p State.
174209
///
@@ -195,10 +230,10 @@ class ContinuationIndenter {
195230
};
196231

197232
struct ParenState {
198-
ParenState(const FormatToken *Tok, unsigned Indent, unsigned LastSpace,
199-
bool AvoidBinPacking, bool NoLineBreak)
233+
ParenState(const FormatToken *Tok, IndentationAndAlignment Indent,
234+
unsigned LastSpace, bool AvoidBinPacking, bool NoLineBreak)
200235
: Tok(Tok), Indent(Indent), LastSpace(LastSpace),
201-
NestedBlockIndent(Indent), IsAligned(false),
236+
NestedBlockIndent(Indent.Total), IsAligned(false),
202237
BreakBeforeClosingBrace(false), BreakBeforeClosingParen(false),
203238
BreakBeforeClosingAngle(false), AvoidBinPacking(AvoidBinPacking),
204239
BreakBeforeParameter(false), NoLineBreak(NoLineBreak),
@@ -219,7 +254,7 @@ struct ParenState {
219254

220255
/// The position to which a specific parenthesis level needs to be
221256
/// indented.
222-
unsigned Indent;
257+
IndentationAndAlignment Indent;
223258

224259
/// The position of the last space on each level.
225260
///
@@ -356,8 +391,8 @@ struct ParenState {
356391
bool UnindentOperator : 1;
357392

358393
bool operator<(const ParenState &Other) const {
359-
if (Indent != Other.Indent)
360-
return Indent < Other.Indent;
394+
if (Indent.Total != Other.Indent.Total)
395+
return Indent.Total < Other.Indent.Total;
361396
if (LastSpace != Other.LastSpace)
362397
return LastSpace < Other.LastSpace;
363398
if (NestedBlockIndent != Other.NestedBlockIndent)
@@ -406,7 +441,7 @@ struct ParenState {
406441
return IsWrappedConditional;
407442
if (UnindentOperator != Other.UnindentOperator)
408443
return UnindentOperator;
409-
return false;
444+
return Indent < Other.Indent;
410445
}
411446
};
412447

0 commit comments

Comments
 (0)