Skip to content

Commit 6f35c68

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 9f5756a commit 6f35c68

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
@@ -6195,6 +6195,12 @@ bool TokenAnnotator::canBreakBefore(const AnnotatedLine &Line,
61956195
return !(Previous && (Previous->is(tok::kw_for) || Previous->isIf()));
61966196
}
61976197

6198+
if (Left.isOneOf(tok::r_paren, TT_TrailingAnnotation) &&
6199+
Right.is(TT_TrailingAnnotation) &&
6200+
Style.AlignAfterOpenBracket == FormatStyle::BAS_BlockIndent) {
6201+
return false;
6202+
}
6203+
61986204
// Allow breaking after a trailing annotation, e.g. after a method
61996205
// declaration.
62006206
if (Left.is(TT_TrailingAnnotation)) {

clang/unittests/Format/FormatTest.cpp

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

93429364
TEST_F(FormatTest, ParenthesesAndOperandAlignment) {

0 commit comments

Comments
 (0)