Skip to content

Commit 09bd1f0

Browse files
committed
[clang-format] Improve BlockIndent at ColumnLimit
Fixes llvm#55731 Fixes llvm#73584 The reported formatting problems were related to ignoring deep nesting of "simple" functions (causing llvm#54808) and to allowing the trailing annotation to become separated from the closing parens, which allowed a break to occur between the closing parens and the trailing annotation. The fix for the nesting of "simple" functions is to detect them more carefully. "Simple" was defined in a comment as being a single non-expression argument. I tried to stay as close to the original intent of the implementation while fixing the various bad formatting reports. In the process of fixing these bugs, some latent bugs were discovered related to how JavaScript Template Strings are handled. Those are also fixed here.
1 parent 9628777 commit 09bd1f0

File tree

3 files changed

+72
-3
lines changed

3 files changed

+72
-3
lines changed

clang/lib/Format/ContinuationIndenter.cpp

Lines changed: 44 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -803,6 +803,46 @@ void ContinuationIndenter::addTokenOnCurrentLine(LineState &State, bool DryRun,
803803
return !Tok.Previous->isOneOf(TT_CastRParen, tok::kw_for, tok::kw_while,
804804
tok::kw_switch);
805805
};
806+
// Detecting functions is brittle. It would be better if we could annotate
807+
// the LParen type of functions/calls.
808+
const auto IsFunctionDeclParen = [&](const FormatToken &Tok) {
809+
return Tok.is(tok::l_paren) && Tok.Previous &&
810+
(Tok.Previous->is(TT_FunctionDeclarationName) ||
811+
(Tok.Previous->Previous &&
812+
Tok.Previous->Previous->is(tok::coloncolon) &&
813+
Tok.Previous->Previous->Previous &&
814+
Tok.Previous->Previous->Previous->is(TT_FunctionDeclarationName)));
815+
};
816+
const auto IsFunctionCallParen = [&](const FormatToken &Tok) {
817+
return Tok.is(tok::l_paren) && Tok.ParameterCount > 0 && Tok.Previous &&
818+
Tok.Previous->is(tok::identifier);
819+
};
820+
const auto IsInTemplateString = [&](const FormatToken &Tok) {
821+
if (!Style.isJavaScript())
822+
return false;
823+
for (const FormatToken *Prev = &Tok; Prev; Prev = Prev->Previous) {
824+
if (Prev->is(TT_TemplateString) && Prev->opensScope())
825+
return true;
826+
if (Prev->is(TT_TemplateString) && Prev->closesScope())
827+
break;
828+
}
829+
return false;
830+
};
831+
const auto IsNotSimpleFunction = [&](const FormatToken &Tok) {
832+
const auto *Previous = Tok.Previous;
833+
const auto *Next = Tok.Next;
834+
if (Tok.FakeLParens.size() > 0 && Tok.FakeLParens.back() > prec::Unknown)
835+
return true;
836+
if (Previous &&
837+
(IsFunctionDeclParen(*Previous) || IsFunctionCallParen(*Previous))) {
838+
if (!IsOpeningBracket(Tok) && Next && !Next->isMemberAccess() &&
839+
!IsInTemplateString(Tok) && !IsFunctionDeclParen(*Next) &&
840+
!IsFunctionCallParen(*Next)) {
841+
return true;
842+
}
843+
}
844+
return false;
845+
};
806846
if ((Style.AlignAfterOpenBracket == FormatStyle::BAS_AlwaysBreak ||
807847
Style.AlignAfterOpenBracket == FormatStyle::BAS_BlockIndent) &&
808848
IsOpeningBracket(Previous) && State.Column > getNewLineColumn(State) &&
@@ -813,10 +853,10 @@ void ContinuationIndenter::addTokenOnCurrentLine(LineState &State, bool DryRun,
813853
// caaaaaaaaaaaall(
814854
// caaaaaaaaaaaall(
815855
// caaaaaaaaaaaaaaaaaaaaaaall(aaaaaaaaaaaaaa, aaaaaaaaa))));
816-
Current.FakeLParens.size() > 0 &&
817-
Current.FakeLParens.back() > prec::Unknown) {
856+
IsNotSimpleFunction(Current)) {
818857
CurrentState.NoLineBreak = true;
819858
}
859+
820860
if (Previous.is(TT_TemplateString) && Previous.opensScope())
821861
CurrentState.NoLineBreak = true;
822862

@@ -831,7 +871,8 @@ void ContinuationIndenter::addTokenOnCurrentLine(LineState &State, bool DryRun,
831871
Previous.isNot(TT_TableGenDAGArgOpenerToBreak) &&
832872
!(Current.MacroParent && Previous.MacroParent) &&
833873
(Current.isNot(TT_LineComment) ||
834-
Previous.isOneOf(BK_BracedInit, TT_VerilogMultiLineListLParen))) {
874+
Previous.isOneOf(BK_BracedInit, TT_VerilogMultiLineListLParen)) &&
875+
!IsInTemplateString(Current)) {
835876
CurrentState.Indent = State.Column + Spaces;
836877
CurrentState.IsAligned = true;
837878
}

clang/lib/Format/TokenAnnotator.cpp

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6205,6 +6205,12 @@ bool TokenAnnotator::canBreakBefore(const AnnotatedLine &Line,
62056205
return !(Previous && (Previous->is(tok::kw_for) || Previous->isIf()));
62066206
}
62076207

6208+
if (Left.isOneOf(tok::r_paren, TT_TrailingAnnotation) &&
6209+
Right.is(TT_TrailingAnnotation) &&
6210+
Style.AlignAfterOpenBracket == FormatStyle::BAS_BlockIndent) {
6211+
return false;
6212+
}
6213+
62086214
// Allow breaking after a trailing annotation, e.g. after a method
62096215
// declaration.
62106216
if (Left.is(TT_TrailingAnnotation)) {

clang/unittests/Format/FormatTest.cpp

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9338,6 +9338,28 @@ TEST_F(FormatTest, AlignsAfterOpenBracket) {
93389338
" aaaaaaaaaaaaaaaa\n"
93399339
");",
93409340
Style);
9341+
verifyFormat(
9342+
"bool aaaaaaaaaaaaaaaaaaaaaaaaaaa(\n"
9343+
" const bool &aaaaaaaaa, const void *aaaaaaaaaa\n"
9344+
") const {\n"
9345+
" return true;\n"
9346+
"}",
9347+
Style);
9348+
verifyFormat(
9349+
"bool aaaaaaaaaaaaaaaaaaaaaaaa(\n"
9350+
" const bool &aaaaaaaaaa, const void *aaaaaaaaaa\n"
9351+
") const;",
9352+
Style);
9353+
verifyFormat(
9354+
"void aaaaaaaaa(\n"
9355+
" int aaaaaa, int bbbbbb, int cccccc, int dddddddddd\n"
9356+
") const noexcept -> std::vector<of_very_long_type>;",
9357+
Style);
9358+
verifyFormat(
9359+
"aaaaaaaaaaaaaaaaaaa(\n"
9360+
" \"a aaaaaaa aaaaaaaaaaaaaaaaa aaaaaaaaaaaaaaa aaaaaaaaaaaaa\"\n"
9361+
");",
9362+
Style);
93419363
}
93429364

93439365
TEST_F(FormatTest, ParenthesesAndOperandAlignment) {

0 commit comments

Comments
 (0)