Author: Gedare Bloom Date: 2024-01-22T19:01:16-08:00 New Revision: 4ef646eab3023b52ce8ee529d16605c92caba8ba
URL: https://github.com/llvm/llvm-project/commit/4ef646eab3023b52ce8ee529d16605c92caba8ba DIFF: https://github.com/llvm/llvm-project/commit/4ef646eab3023b52ce8ee529d16605c92caba8ba.diff LOG: [clang-format]: Fix formatting of if statements with BlockIndent (#77699) A bug with BlockIndent prevents line breaks within if (and else if) clauses. While fixing this bug, it appears that AlignAfterOpenBracket is not designed to work with loop and if statements, but AlwaysBreak works on if clauses. The documentation and tests are not clear on whether or not this behavior is intended. This PR preserves the `AlwaysBreak` behavior on `if` clauses without supporting `BlockIndent` on `if` clauses to avoid regressions while fixing the bug. It may be reasonable to create an explicit option for alignment of if (and loop) clauses intentionally for both `AlwaysBreak` and `BlockIndent` Fixes #54663. Migrated from Differential Revision: https://reviews.llvm.org/D154755 See more discussion there. Addressed last open comment from the rev about refactoring the complex conditional logic involved with the `AlignAfterOpenBracket` line break behavior. Added: Modified: clang/lib/Format/ContinuationIndenter.cpp clang/unittests/Format/FormatTest.cpp Removed: ################################################################################ diff --git a/clang/lib/Format/ContinuationIndenter.cpp b/clang/lib/Format/ContinuationIndenter.cpp index a099813c9100b4..a3eb9138b21833 100644 --- a/clang/lib/Format/ContinuationIndenter.cpp +++ b/clang/lib/Format/ContinuationIndenter.cpp @@ -771,15 +771,25 @@ void ContinuationIndenter::addTokenOnCurrentLine(LineState &State, bool DryRun, // parenthesis by disallowing any further line breaks if there is no line // break after the opening parenthesis. Don't break if it doesn't conserve // columns. + auto IsOpeningBracket = [&](const FormatToken &Tok) { + auto IsStartOfBracedList = [&]() { + return Tok.is(tok::l_brace) && Tok.isNot(BK_Block) && + Style.Cpp11BracedListStyle; + }; + if (!Tok.isOneOf(tok::l_paren, TT_TemplateOpener, tok::l_square) && + !IsStartOfBracedList()) { + return false; + } + if (!Tok.Previous) + return true; + if (Tok.Previous->isIf()) + return Style.AlignAfterOpenBracket == FormatStyle::BAS_AlwaysBreak; + return !Tok.Previous->isOneOf(TT_CastRParen, tok::kw_for, tok::kw_while, + tok::kw_switch); + }; if ((Style.AlignAfterOpenBracket == FormatStyle::BAS_AlwaysBreak || Style.AlignAfterOpenBracket == FormatStyle::BAS_BlockIndent) && - (Previous.isOneOf(tok::l_paren, TT_TemplateOpener, tok::l_square) || - (Previous.is(tok::l_brace) && Previous.isNot(BK_Block) && - Style.Cpp11BracedListStyle)) && - State.Column > getNewLineColumn(State) && - (!Previous.Previous || - !Previous.Previous->isOneOf(TT_CastRParen, tok::kw_for, tok::kw_while, - tok::kw_switch)) && + IsOpeningBracket(Previous) && State.Column > getNewLineColumn(State) && // Don't do this for simple (no expressions) one-argument function calls // as that feels like needlessly wasting whitespace, e.g.: // diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp index fa3bea5ae5d755..e5e763edf5b5bf 100644 --- a/clang/unittests/Format/FormatTest.cpp +++ b/clang/unittests/Format/FormatTest.cpp @@ -26172,8 +26172,8 @@ TEST_F(FormatTest, AlignAfterOpenBracketBlockIndentIfStatement) { "}", Style); - verifyFormat("if (quitelongarg !=\n" - " (alsolongarg - 1)) { // ABC is a very longgggggggggggg " + verifyFormat("if (quiteLongArg !=\n" + " (alsoLongArg - 1)) { // ABC is a very longgggggggggggg " "comment\n" " return;\n" "}", @@ -26186,12 +26186,44 @@ TEST_F(FormatTest, AlignAfterOpenBracketBlockIndentIfStatement) { "}", Style); - verifyFormat("if (quitelongarg !=\n" - " (alsolongarg - 1)) { // ABC is a very longgggggggggggg " + verifyFormat("if (quiteLongArg !=\n" + " (alsoLongArg - 1)) { // ABC is a very longgggggggggggg " "comment\n" " return;\n" "}", Style); + + verifyFormat("void foo() {\n" + " if (camelCaseName < alsoLongName ||\n" + " anotherEvenLongerName <=\n" + " thisReallyReallyReallyReallyReallyReallyLongerName ||" + "\n" + " otherName < thisLastName) {\n" + " return;\n" + " } else if (quiteLongName < alsoLongName ||\n" + " anotherEvenLongerName <=\n" + " thisReallyReallyReallyReallyReallyReallyLonger" + "Name ||\n" + " otherName < thisLastName) {\n" + " return;\n" + " }\n" + "}", + Style); + + Style.ContinuationIndentWidth = 2; + verifyFormat("void foo() {\n" + " if (ThisIsRatherALongIfClause && thatIExpectToBeBroken ||\n" + " ontoMultipleLines && whenFormattedCorrectly) {\n" + " if (false) {\n" + " return;\n" + " } else if (thisIsRatherALongIfClause && " + "thatIExpectToBeBroken ||\n" + " ontoMultipleLines && whenFormattedCorrectly) {\n" + " return;\n" + " }\n" + " }\n" + "}", + Style); } TEST_F(FormatTest, AlignAfterOpenBracketBlockIndentForStatement) { _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits