From ca6623b9e3ab292558837b0543e4ab2ac147eb3b Mon Sep 17 00:00:00 2001 From: Daniel Jasper Date: Mon, 28 Jan 2013 12:45:14 +0000 Subject: [PATCH] Improve formatting of conditional expressions. Before we did not really systematically format those. Now, we format the different cases as: - 1 Line: a ? b : c; - 2 Lines: short ? loooooooooong : loooooooooong - 2 Lines: loooooooooooooooong ? short : short - 3 Lines: loooooooooooooooong ? loooooooooooooong : loooooooooooooong Not sure whether "?" and ":" should go on the new line, but it seems to be the most consistent approach. llvm-svn: 173683 --- clang/lib/Format/Format.cpp | 32 ++++++++++++++++++++--------- clang/unittests/Format/FormatTest.cpp | 38 ++++++++++++++++++++++++++++------- 2 files changed, 53 insertions(+), 17 deletions(-) diff --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp index 089035a..85f2c32 100644 --- a/clang/lib/Format/Format.cpp +++ b/clang/lib/Format/Format.cpp @@ -433,7 +433,7 @@ private: struct ParenState { ParenState(unsigned Indent, unsigned LastSpace) : Indent(Indent), LastSpace(LastSpace), AssignmentColumn(0), - FirstLessLess(0), BreakBeforeClosingBrace(false), + FirstLessLess(0), BreakBeforeClosingBrace(false), QuestionColumn(0), BreakAfterComma(false), HasMultiParameterLine(false) {} /// \brief The position to which a specific parenthesis level needs to be @@ -463,6 +463,9 @@ private: /// was a newline after the beginning left brace. bool BreakBeforeClosingBrace; + /// \brief The column of a \c ? in a conditional expression; + unsigned QuestionColumn; + bool BreakAfterComma; bool HasMultiParameterLine; @@ -477,6 +480,8 @@ private: return FirstLessLess < Other.FirstLessLess; if (BreakBeforeClosingBrace != Other.BreakBeforeClosingBrace) return BreakBeforeClosingBrace; + if (QuestionColumn != Other.QuestionColumn) + return QuestionColumn < Other.QuestionColumn; if (BreakAfterComma != Other.BreakAfterComma) return BreakAfterComma; if (HasMultiParameterLine != Other.HasMultiParameterLine) @@ -548,13 +553,15 @@ private: State.Column = State.Stack[ParenLevel].FirstLessLess; } else if (ParenLevel != 0 && (Previous.is(tok::equal) || Previous.is(tok::coloncolon) || - Previous.is(tok::question) || - Previous.Type == TT_ConditionalExpr || - Current.is(tok::period) || Current.is(tok::arrow))) { + Current.is(tok::period) || Current.is(tok::arrow) || + Current.is(tok::question))) { // Indent and extra 4 spaces after if we know the current expression is // continued. Don't do that on the top level, as we already indent 4 // there. - State.Column = State.Stack[ParenLevel].Indent + 4; + State.Column = std::max(State.Stack.back().LastSpace, + State.Stack.back().Indent) + 4; + } else if (Current.Type == TT_ConditionalExpr) { + State.Column = State.Stack.back().QuestionColumn; } else if (RootToken.is(tok::kw_for) && Previous.is(tok::comma)) { State.Column = State.ForLoopVariablePos; } else if (State.NextToken->Parent->ClosesTemplateDeclaration) { @@ -579,7 +586,7 @@ private: } State.Stack[ParenLevel].LastSpace = State.Column; - if (Current.is(tok::colon) && State.NextToken->Type != TT_ConditionalExpr) + if (Current.is(tok::colon) && Current.Type != TT_ConditionalExpr) State.Stack[ParenLevel].Indent += 2; } else { if (Current.is(tok::equal) && RootToken.is(tok::kw_for)) @@ -615,7 +622,8 @@ private: else if (Previous.is(tok::comma) && ParenLevel != 0) // Top-level spaces are exempt as that mostly leads to better results. State.Stack.back().LastSpace = State.Column; - else if (Previous.Type == TT_BinaryOperator && + else if ((Previous.Type == TT_BinaryOperator || + Previous.Type == TT_ConditionalExpr) && getPrecedence(Previous) != prec::Assignment) State.Stack.back().LastSpace = State.Column; else if (Previous.ParameterCount > 1 && @@ -656,6 +664,8 @@ private: if (Current.is(tok::lessless) && State.Stack.back().FirstLessLess == 0) State.Stack.back().FirstLessLess = State.Column; + if (Current.is(tok::question)) + State.Stack.back().QuestionColumn = State.Column; // If we encounter an opening (, [, { or <, we add a level to our stacks to // prepare for the following tokens. @@ -726,7 +736,7 @@ private: if (Left.is(tok::l_square) || Left.Type == TT_TemplateOpener) return 50; - if (Left.is(tok::question) || Left.Type == TT_ConditionalExpr) + if (Left.Type == TT_ConditionalExpr) return prec::Assignment; prec::Level Level = getPrecedence(Left); @@ -1587,8 +1597,11 @@ private: return true; if (Left.ClosesTemplateDeclaration) return true; + if (Right.Type == TT_ConditionalExpr || Right.is(tok::question)) + return true; if (Left.Type == TT_PointerOrReference || Left.Type == TT_TemplateCloser || - Left.Type == TT_UnaryOperator || Right.Type == TT_ConditionalExpr) + Left.Type == TT_UnaryOperator || Left.Type == TT_ConditionalExpr || + Left.is(tok::question)) return false; if (Left.is(tok::equal) && Line.Type == LT_VirtualFunctionDecl) return false; @@ -1617,7 +1630,6 @@ private: Right.is(tok::arrow) || Right.is(tok::period) || Right.is(tok::colon) || Left.is(tok::coloncolon) || Left.is(tok::semi) || Left.is(tok::l_brace) || - Left.is(tok::question) || Left.Type == TT_ConditionalExpr || (Left.is(tok::r_paren) && Left.Type != TT_CastRParen && Right.is(tok::identifier)) || (Left.is(tok::l_paren) && !Right.is(tok::r_paren)) || diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp index 57252f5..240ce1a 100644 --- a/clang/unittests/Format/FormatTest.cpp +++ b/clang/unittests/Format/FormatTest.cpp @@ -1111,13 +1111,31 @@ TEST_F(FormatTest, AlignsAfterReturn) { TEST_F(FormatTest, BreaksConditionalExpressions) { verifyFormat( "aaaa(aaaaaaaaaaaaaaaaaaaa,\n" - " aaaaaaaaaaaaaaaaaaaaaaaaaa ? aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa :\n" - " aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa);"); - verifyFormat("aaaa(aaaaaaaaaaaaaaaaaaaa, aaaaaaaaaaaaaaaaaaaaaaaaaa ?\n" - " aaaaaaaaaaaaaaaaaaaaaaa : aaaaaaaaaaaaaaaaaaaaa);"); - verifyFormat( - "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaa(aaaaaaaaaaaaaaaaaaaa ? aaaa(aaaaaa) :\n" - " aaaaaaaaaaaaa);"); + " aaaaaaaaaaaaaaaaaaaaaaaaaa ? aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\n" + " : aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa);"); + verifyFormat( + "aaaa(aaaaaaaaaaaaaaaaaaaa, aaaaaaa ? aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\n" + " : aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa);"); + verifyFormat( + "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaa(aaaaaaaaaaaaaaaaaaaa ? aaaa(aaaaaa)\n" + " : aaaaaaaaaaaaa);"); + verifyFormat( + "aaaaaaaaaaaaaaaaaa(aaaaaaaaaaaaaaaaaaaaaaaaa,\n" + " aaaaaaaaaaaaaaaa ? aaaaaaaaaaaaaaaaaaaaaaa\n" + " : aaaaaaaaaaaaaaaaaaaaaaaaaaaa,\n" + " aaaaaaaaaaaaa);"); + verifyFormat("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\n" + " ? aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa(\n" + " aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa)\n" + " : aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa(\n" + " aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa);"); + verifyFormat("aaaaaa(aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa,\n" + " aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\n" + " ? aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa(\n" + " aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa)\n" + " : aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa(\n" + " aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa),\n" + " aaaaaaaaaaaaaaaaaaaaaaaaaaa);"); } TEST_F(FormatTest, ConditionalExpressionsInBrackets) { @@ -1261,6 +1279,12 @@ TEST_F(FormatTest, WrapsAtNestedNameSpecifiers) { verifyFormat( "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa::aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa(\n" " aaaaaaaaaaaaaaaaaaaaaaa);"); + + verifyFormat( + "aaaaaaaaaaaaaaaaaa(aaaaaaaa, aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa::\n" + " aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa,\n" + " aaaaaaaaaaaaaaaaaaaaa);", + getLLVMStyleWithColumns(74)); } TEST_F(FormatTest, UnderstandsTemplateParameters) { -- 2.7.4