Skip to content

Commit 27a2da8

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 7f5c71e commit 27a2da8

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

6160+
if (Left.isOneOf(tok::r_paren, TT_TrailingAnnotation) &&
6161+
Right.is(TT_TrailingAnnotation) &&
6162+
Style.AlignAfterOpenBracket == FormatStyle::BAS_BlockIndent) {
6163+
return false;
6164+
}
6165+
61606166
// Allow breaking after a trailing annotation, e.g. after a method
61616167
// declaration.
61626168
if (Left.is(TT_TrailingAnnotation)) {

clang/unittests/Format/FormatTest.cpp

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9305,6 +9305,28 @@ TEST_F(FormatTest, AlignsAfterOpenBracket) {
93059305
" aaaaaaaaaaaaaaaa\n"
93069306
");",
93079307
Style);
9308+
verifyFormat(
9309+
"bool aaaaaaaaaaaaaaaaaaaaaaaaaaa(\n"
9310+
" const bool &aaaaaaaaa, const void *aaaaaaaaaa\n"
9311+
") const {\n"
9312+
" return true;\n"
9313+
"}",
9314+
Style);
9315+
verifyFormat(
9316+
"bool aaaaaaaaaaaaaaaaaaaaaaaa(\n"
9317+
" const bool &aaaaaaaaaa, const void *aaaaaaaaaa\n"
9318+
") const;",
9319+
Style);
9320+
verifyFormat(
9321+
"void aaaaaaaaa(\n"
9322+
" int aaaaaa, int bbbbbb, int cccccc, int dddddddddd\n"
9323+
") const noexcept -> std::vector<of_very_long_type>;",
9324+
Style);
9325+
verifyFormat(
9326+
"aaaaaaaaaaaaaaaaaaa(\n"
9327+
" \"a aaaaaaa aaaaaaaaaaaaaaaaa aaaaaaaaaaaaaaa aaaaaaaaaaaaa\"\n"
9328+
");",
9329+
Style);
93089330
}
93099331

93109332
TEST_F(FormatTest, ParenthesesAndOperandAlignment) {

0 commit comments

Comments
 (0)