https://github.com/gedare updated https://github.com/llvm/llvm-project/pull/93140
>From 27a2da876926d2157ea9f18c5fd71a2e81e097fc Mon Sep 17 00:00:00 2001 From: Gedare Bloom <ged...@rtems.org> Date: Tue, 21 May 2024 22:21:59 -0600 Subject: [PATCH 1/4] [clang-format] Improve BlockIndent at ColumnLimit Fixes #55731 Fixes #73584 The reported formatting problems were related to ignoring deep nesting of "simple" functions (causing #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. --- clang/lib/Format/ContinuationIndenter.cpp | 47 +++++++++++++++++++++-- clang/lib/Format/TokenAnnotator.cpp | 6 +++ clang/unittests/Format/FormatTest.cpp | 22 +++++++++++ 3 files changed, 72 insertions(+), 3 deletions(-) diff --git a/clang/lib/Format/ContinuationIndenter.cpp b/clang/lib/Format/ContinuationIndenter.cpp index 6b9fbfe0ebf53..b0a232e97b314 100644 --- a/clang/lib/Format/ContinuationIndenter.cpp +++ b/clang/lib/Format/ContinuationIndenter.cpp @@ -803,6 +803,46 @@ void ContinuationIndenter::addTokenOnCurrentLine(LineState &State, bool DryRun, return !Tok.Previous->isOneOf(TT_CastRParen, tok::kw_for, tok::kw_while, tok::kw_switch); }; + // Detecting functions is brittle. It would be better if we could annotate + // the LParen type of functions/calls. + const auto IsFunctionDeclParen = [&](const FormatToken &Tok) { + return Tok.is(tok::l_paren) && Tok.Previous && + (Tok.Previous->is(TT_FunctionDeclarationName) || + (Tok.Previous->Previous && + Tok.Previous->Previous->is(tok::coloncolon) && + Tok.Previous->Previous->Previous && + Tok.Previous->Previous->Previous->is(TT_FunctionDeclarationName))); + }; + const auto IsFunctionCallParen = [&](const FormatToken &Tok) { + return Tok.is(tok::l_paren) && Tok.ParameterCount > 0 && Tok.Previous && + Tok.Previous->is(tok::identifier); + }; + const auto IsInTemplateString = [&](const FormatToken &Tok) { + if (!Style.isJavaScript()) + return false; + for (const FormatToken *Prev = &Tok; Prev; Prev = Prev->Previous) { + if (Prev->is(TT_TemplateString) && Prev->opensScope()) + return true; + if (Prev->is(TT_TemplateString) && Prev->closesScope()) + break; + } + return false; + }; + const auto IsNotSimpleFunction = [&](const FormatToken &Tok) { + const auto *Previous = Tok.Previous; + const auto *Next = Tok.Next; + if (Tok.FakeLParens.size() > 0 && Tok.FakeLParens.back() > prec::Unknown) + return true; + if (Previous && + (IsFunctionDeclParen(*Previous) || IsFunctionCallParen(*Previous))) { + if (!IsOpeningBracket(Tok) && Next && !Next->isMemberAccess() && + !IsInTemplateString(Tok) && !IsFunctionDeclParen(*Next) && + !IsFunctionCallParen(*Next)) { + return true; + } + } + return false; + }; if ((Style.AlignAfterOpenBracket == FormatStyle::BAS_AlwaysBreak || Style.AlignAfterOpenBracket == FormatStyle::BAS_BlockIndent) && IsOpeningBracket(Previous) && State.Column > getNewLineColumn(State) && @@ -813,10 +853,10 @@ void ContinuationIndenter::addTokenOnCurrentLine(LineState &State, bool DryRun, // caaaaaaaaaaaall( // caaaaaaaaaaaall( // caaaaaaaaaaaaaaaaaaaaaaall(aaaaaaaaaaaaaa, aaaaaaaaa)))); - Current.FakeLParens.size() > 0 && - Current.FakeLParens.back() > prec::Unknown) { + IsNotSimpleFunction(Current)) { CurrentState.NoLineBreak = true; } + if (Previous.is(TT_TemplateString) && Previous.opensScope()) CurrentState.NoLineBreak = true; @@ -831,7 +871,8 @@ void ContinuationIndenter::addTokenOnCurrentLine(LineState &State, bool DryRun, Previous.isNot(TT_TableGenDAGArgOpenerToBreak) && !(Current.MacroParent && Previous.MacroParent) && (Current.isNot(TT_LineComment) || - Previous.isOneOf(BK_BracedInit, TT_VerilogMultiLineListLParen))) { + Previous.isOneOf(BK_BracedInit, TT_VerilogMultiLineListLParen)) && + !IsInTemplateString(Current)) { CurrentState.Indent = State.Column + Spaces; CurrentState.IsAligned = true; } diff --git a/clang/lib/Format/TokenAnnotator.cpp b/clang/lib/Format/TokenAnnotator.cpp index 7c4c76a91f2c5..094e87db2426a 100644 --- a/clang/lib/Format/TokenAnnotator.cpp +++ b/clang/lib/Format/TokenAnnotator.cpp @@ -6157,6 +6157,12 @@ bool TokenAnnotator::canBreakBefore(const AnnotatedLine &Line, return !(Previous && (Previous->is(tok::kw_for) || Previous->isIf())); } + if (Left.isOneOf(tok::r_paren, TT_TrailingAnnotation) && + Right.is(TT_TrailingAnnotation) && + Style.AlignAfterOpenBracket == FormatStyle::BAS_BlockIndent) { + return false; + } + // Allow breaking after a trailing annotation, e.g. after a method // declaration. if (Left.is(TT_TrailingAnnotation)) { diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp index a9df994189f00..0413f1f08fd9c 100644 --- a/clang/unittests/Format/FormatTest.cpp +++ b/clang/unittests/Format/FormatTest.cpp @@ -9305,6 +9305,28 @@ TEST_F(FormatTest, AlignsAfterOpenBracket) { " aaaaaaaaaaaaaaaa\n" ");", Style); + verifyFormat( + "bool aaaaaaaaaaaaaaaaaaaaaaaaaaa(\n" + " const bool &aaaaaaaaa, const void *aaaaaaaaaa\n" + ") const {\n" + " return true;\n" + "}", + Style); + verifyFormat( + "bool aaaaaaaaaaaaaaaaaaaaaaaa(\n" + " const bool &aaaaaaaaaa, const void *aaaaaaaaaa\n" + ") const;", + Style); + verifyFormat( + "void aaaaaaaaa(\n" + " int aaaaaa, int bbbbbb, int cccccc, int dddddddddd\n" + ") const noexcept -> std::vector<of_very_long_type>;", + Style); + verifyFormat( + "aaaaaaaaaaaaaaaaaaa(\n" + " \"a aaaaaaa aaaaaaaaaaaaaaaaa aaaaaaaaaaaaaaa aaaaaaaaaaaaa\"\n" + ");", + Style); } TEST_F(FormatTest, ParenthesesAndOperandAlignment) { >From 497d1f8b351b45e0e24ffcd9a39fac8ca2463967 Mon Sep 17 00:00:00 2001 From: Gedare Bloom <ged...@rtems.org> Date: Thu, 23 May 2024 00:05:33 -0600 Subject: [PATCH 2/4] FormatTest.cpp: fix format --- clang/unittests/Format/FormatTest.cpp | 31 ++++++++++++--------------- 1 file changed, 14 insertions(+), 17 deletions(-) diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp index 0413f1f08fd9c..0a272d7482021 100644 --- a/clang/unittests/Format/FormatTest.cpp +++ b/clang/unittests/Format/FormatTest.cpp @@ -9305,23 +9305,20 @@ TEST_F(FormatTest, AlignsAfterOpenBracket) { " aaaaaaaaaaaaaaaa\n" ");", Style); - verifyFormat( - "bool aaaaaaaaaaaaaaaaaaaaaaaaaaa(\n" - " const bool &aaaaaaaaa, const void *aaaaaaaaaa\n" - ") const {\n" - " return true;\n" - "}", - Style); - verifyFormat( - "bool aaaaaaaaaaaaaaaaaaaaaaaa(\n" - " const bool &aaaaaaaaaa, const void *aaaaaaaaaa\n" - ") const;", - Style); - verifyFormat( - "void aaaaaaaaa(\n" - " int aaaaaa, int bbbbbb, int cccccc, int dddddddddd\n" - ") const noexcept -> std::vector<of_very_long_type>;", - Style); + verifyFormat("bool aaaaaaaaaaaaaaaaaaaaaaaaaaa(\n" + " const bool &aaaaaaaaa, const void *aaaaaaaaaa\n" + ") const {\n" + " return true;\n" + "}", + Style); + verifyFormat("bool aaaaaaaaaaaaaaaaaaaaaaaa(\n" + " const bool &aaaaaaaaaa, const void *aaaaaaaaaa\n" + ") const;", + Style); + verifyFormat("void aaaaaaaaa(\n" + " int aaaaaa, int bbbbbb, int cccccc, int dddddddddd\n" + ") const noexcept -> std::vector<of_very_long_type>;", + Style); verifyFormat( "aaaaaaaaaaaaaaaaaaa(\n" " \"a aaaaaaa aaaaaaaaaaaaaaaaa aaaaaaaaaaaaaaa aaaaaaaaaaaaa\"\n" >From 1bd837c72e64998eb3f23edbc6953b1a30b53584 Mon Sep 17 00:00:00 2001 From: Gedare Bloom <ged...@rtems.org> Date: Fri, 31 May 2024 15:03:04 -0600 Subject: [PATCH 3/4] handle lambdas --- clang/lib/Format/ContinuationIndenter.cpp | 26 +++++++++++++++++------ clang/unittests/Format/FormatTest.cpp | 7 ++++++ 2 files changed, 27 insertions(+), 6 deletions(-) diff --git a/clang/lib/Format/ContinuationIndenter.cpp b/clang/lib/Format/ContinuationIndenter.cpp index b0a232e97b314..d448b79a2e439 100644 --- a/clang/lib/Format/ContinuationIndenter.cpp +++ b/clang/lib/Format/ContinuationIndenter.cpp @@ -813,6 +813,20 @@ void ContinuationIndenter::addTokenOnCurrentLine(LineState &State, bool DryRun, Tok.Previous->Previous->Previous && Tok.Previous->Previous->Previous->is(TT_FunctionDeclarationName))); }; + const auto IsLambdaParameterList = [](const FormatToken *Left) { + // adapted from TokenAnnotator.cpp:isLambdaParameterList() + // Skip <...> if present. + if (Left->Previous && Left->Previous->is(tok::greater) && + Left->Previous->MatchingParen && + Left->Previous->MatchingParen->is(TT_TemplateOpener)) { + Left = Left->Previous->MatchingParen; + } + + // Check for `[...]`. + return Left->Previous && Left->Previous->is(tok::r_square) && + Left->Previous->MatchingParen && + Left->Previous->MatchingParen->is(TT_LambdaLSquare); + }; const auto IsFunctionCallParen = [&](const FormatToken &Tok) { return Tok.is(tok::l_paren) && Tok.ParameterCount > 0 && Tok.Previous && Tok.Previous->is(tok::identifier); @@ -828,18 +842,18 @@ void ContinuationIndenter::addTokenOnCurrentLine(LineState &State, bool DryRun, } return false; }; + // Identifies simple (no expression) one-argument function calls. const auto IsNotSimpleFunction = [&](const FormatToken &Tok) { const auto *Previous = Tok.Previous; const auto *Next = Tok.Next; if (Tok.FakeLParens.size() > 0 && Tok.FakeLParens.back() > prec::Unknown) return true; if (Previous && - (IsFunctionDeclParen(*Previous) || IsFunctionCallParen(*Previous))) { - if (!IsOpeningBracket(Tok) && Next && !Next->isMemberAccess() && - !IsInTemplateString(Tok) && !IsFunctionDeclParen(*Next) && - !IsFunctionCallParen(*Next)) { - return true; - } + (IsFunctionDeclParen(*Previous) || IsFunctionCallParen(*Previous) || + IsLambdaParameterList(Previous))) { + return !IsOpeningBracket(Tok) && Next && !Next->isMemberAccess() && + !IsInTemplateString(Tok) && !IsFunctionDeclParen(*Next) && + !IsFunctionCallParen(*Next); } return false; }; diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp index 0a272d7482021..deb89a3adc9aa 100644 --- a/clang/unittests/Format/FormatTest.cpp +++ b/clang/unittests/Format/FormatTest.cpp @@ -9324,6 +9324,13 @@ TEST_F(FormatTest, AlignsAfterOpenBracket) { " \"a aaaaaaa aaaaaaaaaaaaaaaaa aaaaaaaaaaaaaaa aaaaaaaaaaaaa\"\n" ");", Style); + verifyFormat( + "auto lambda =\n" + " [&b](\n" + " auto " + "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\n" + " ) {};", + Style); } TEST_F(FormatTest, ParenthesesAndOperandAlignment) { >From d04eaf4ffa97dec664a29c5aa2efe0c6736f48df Mon Sep 17 00:00:00 2001 From: Gedare Bloom <ged...@rtems.org> Date: Thu, 20 Jun 2024 12:49:11 -0600 Subject: [PATCH 4/4] Address review comments --- clang/lib/Format/ContinuationIndenter.cpp | 25 ++++++++++------------- 1 file changed, 11 insertions(+), 14 deletions(-) diff --git a/clang/lib/Format/ContinuationIndenter.cpp b/clang/lib/Format/ContinuationIndenter.cpp index d448b79a2e439..46c41ee76a76b 100644 --- a/clang/lib/Format/ContinuationIndenter.cpp +++ b/clang/lib/Format/ContinuationIndenter.cpp @@ -805,29 +805,26 @@ void ContinuationIndenter::addTokenOnCurrentLine(LineState &State, bool DryRun, }; // Detecting functions is brittle. It would be better if we could annotate // the LParen type of functions/calls. - const auto IsFunctionDeclParen = [&](const FormatToken &Tok) { + auto IsFunctionDeclParen = [&](const FormatToken &Tok) { return Tok.is(tok::l_paren) && Tok.Previous && (Tok.Previous->is(TT_FunctionDeclarationName) || - (Tok.Previous->Previous && - Tok.Previous->Previous->is(tok::coloncolon) && - Tok.Previous->Previous->Previous && - Tok.Previous->Previous->Previous->is(TT_FunctionDeclarationName))); + Tok.endsSequence(TT_FunctionDeclarationName, tok::coloncolon)); }; - const auto IsLambdaParameterList = [](const FormatToken *Left) { + auto IsLambdaParameterList = [](const FormatToken *Tok) { // adapted from TokenAnnotator.cpp:isLambdaParameterList() // Skip <...> if present. - if (Left->Previous && Left->Previous->is(tok::greater) && - Left->Previous->MatchingParen && - Left->Previous->MatchingParen->is(TT_TemplateOpener)) { - Left = Left->Previous->MatchingParen; + if (Tok->Previous && Tok->Previous->is(tok::greater) && + Tok->Previous->MatchingParen && + Tok->Previous->MatchingParen->is(TT_TemplateOpener)) { + Tok = Tok->Previous->MatchingParen; } // Check for `[...]`. - return Left->Previous && Left->Previous->is(tok::r_square) && - Left->Previous->MatchingParen && - Left->Previous->MatchingParen->is(TT_LambdaLSquare); + return Tok->Previous && Tok->Previous->is(tok::r_square) && + Tok->Previous->MatchingParen && + Tok->Previous->MatchingParen->is(TT_LambdaLSquare); }; - const auto IsFunctionCallParen = [&](const FormatToken &Tok) { + auto IsFunctionCallParen = [&](const FormatToken &Tok) { return Tok.is(tok::l_paren) && Tok.ParameterCount > 0 && Tok.Previous && Tok.Previous->is(tok::identifier); }; _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits