From 9c837d05c9e6d6971900d36b1e462ed3666b7487 Mon Sep 17 00:00:00 2001 From: Daniel Jasper Date: Wed, 9 Jan 2013 07:06:56 +0000 Subject: [PATCH] Improve formatting of conditional operators. This addresses llvm.org/PR14864. We used to completely mess this up and now format as: Diag(NewFD->getLocation(), getLangOpts().MicrosoftExt ? diag::ext_function_specialization_in_class : diag::err_function_specialization_in_class) << NewFD->getDeclName(); git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@171957 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/Format/Format.cpp | 64 ++++++++++++++++++--------------- unittests/Format/FormatTest.cpp | 9 +++++ 2 files changed, 45 insertions(+), 28 deletions(-) diff --git a/lib/Format/Format.cpp b/lib/Format/Format.cpp index a371aa3c4a..e8c6210fae 100644 --- a/lib/Format/Format.cpp +++ b/lib/Format/Format.cpp @@ -269,30 +269,32 @@ private: /// If \p DryRun is \c false, also creates and stores the required /// \c Replacement. void addTokenToState(bool Newline, bool DryRun, IndentState &State) { - const FormatToken &Current = State.NextToken->FormatTok; - const FormatToken &Previous = State.NextToken->Parent->FormatTok; + const AnnotatedToken &Current = *State.NextToken; + const AnnotatedToken &Previous = *State.NextToken->Parent; unsigned ParenLevel = State.Indent.size() - 1; if (Newline) { unsigned WhitespaceStartColumn = State.Column; - if (Previous.Tok.is(tok::l_brace)) { + if (Previous.is(tok::l_brace)) { // FIXME: This does not work with nested static initializers. // Implement a better handling for static initializers and similar // constructs. State.Column = Line.Level * 2 + 2; - } else if (Current.Tok.is(tok::string_literal) && - Previous.Tok.is(tok::string_literal)) { - State.Column = State.Column - Previous.TokenLength; - } else if (Current.Tok.is(tok::lessless) && + } else if (Current.is(tok::string_literal) && + Previous.is(tok::string_literal)) { + State.Column = State.Column - Previous.FormatTok.TokenLength; + } else if (Current.is(tok::lessless) && State.FirstLessLess[ParenLevel] != 0) { State.Column = State.FirstLessLess[ParenLevel]; } else if (ParenLevel != 0 && - (Previous.Tok.is(tok::equal) || Current.Tok.is(tok::arrow) || - Current.Tok.is(tok::period))) { - // Indent and extra 4 spaces after '=' as it continues an expression. - // Don't do that on the top level, as we already indent 4 there. + (Previous.is(tok::equal) || Current.is(tok::arrow) || + Current.is(tok::period) || Previous.is(tok::question) || + Previous.Type == TT_ConditionalExpr)) { + // 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.Indent[ParenLevel] + 4; - } else if (RootToken.is(tok::kw_for) && Previous.Tok.is(tok::comma)) { + } else if (RootToken.is(tok::kw_for) && Previous.is(tok::comma)) { State.Column = State.ForLoopVariablePos; } else if (State.NextToken->Parent->ClosesTemplateDeclaration) { State.Column = State.Indent[ParenLevel] - 4; @@ -303,23 +305,24 @@ private: State.StartOfLineLevel = ParenLevel + 1; if (RootToken.is(tok::kw_for)) - State.LineContainsContinuedForLoopSection = - Previous.Tok.isNot(tok::semi); + State.LineContainsContinuedForLoopSection = Previous.isNot(tok::semi); if (!DryRun) { if (!Line.InPPDirective) - replaceWhitespace(Current, 1, State.Column); + replaceWhitespace(Current.FormatTok, 1, State.Column); else - replacePPWhitespace(Current, 1, State.Column, WhitespaceStartColumn); + replacePPWhitespace(Current.FormatTok, 1, State.Column, + WhitespaceStartColumn); } State.LastSpace[ParenLevel] = State.Indent[ParenLevel]; - if (Current.Tok.is(tok::colon) && CurrentLineType != LT_ObjCMethodDecl && + if (Current.is(tok::colon) && CurrentLineType != LT_ObjCMethodDecl && State.NextToken->Type != TT_ConditionalExpr) State.Indent[ParenLevel] += 2; } else { - if (Current.Tok.is(tok::equal) && RootToken.is(tok::kw_for)) - State.ForLoopVariablePos = State.Column - Previous.TokenLength; + if (Current.is(tok::equal) && RootToken.is(tok::kw_for)) + State.ForLoopVariablePos = State.Column - + Previous.FormatTok.TokenLength; unsigned Spaces = State.NextToken->SpaceRequiredBefore ? 1 : 0; if (State.NextToken->Type == TT_LineComment) @@ -330,9 +333,9 @@ private: if (RootToken.isNot(tok::kw_for) && (getPrecedence(Previous) == prec::Assignment || - Previous.Tok.is(tok::kw_return))) + Previous.is(tok::kw_return))) State.Indent[ParenLevel] = State.Column + Spaces; - if (Previous.Tok.is(tok::l_paren) || + if (Previous.is(tok::l_paren) || State.NextToken->Parent->Type == TT_TemplateOpener) State.Indent[ParenLevel] = State.Column; @@ -398,6 +401,8 @@ private: if (Left.is(tok::l_paren)) return 20; + if (Left.is(tok::question) || Left.Type == TT_ConditionalExpr) + return prec::Assignment; prec::Level Level = getPrecedence(Left); // Breaking after an assignment leads to a bad result as the two sides of @@ -484,10 +489,11 @@ private: /// \brief Replaces the whitespace in front of \p Tok. Only call once for /// each \c FormatToken. - void replaceWhitespace(const FormatToken &Tok, unsigned NewLines, + void replaceWhitespace(const AnnotatedToken &Tok, unsigned NewLines, unsigned Spaces) { Replaces.insert(tooling::Replacement( - SourceMgr, Tok.WhiteSpaceStart, Tok.WhiteSpaceLength, + SourceMgr, Tok.FormatTok.WhiteSpaceStart, + Tok.FormatTok.WhiteSpaceLength, std::string(NewLines, '\n') + std::string(Spaces, ' '))); } @@ -496,7 +502,7 @@ private: /// /// This function and \c replaceWhitespace have the same behavior if /// \c Newlines == 0. - void replacePPWhitespace(const FormatToken &Tok, unsigned NewLines, + void replacePPWhitespace(const AnnotatedToken &Tok, unsigned NewLines, unsigned Spaces, unsigned WhitespaceStartColumn) { std::string NewLineText; if (NewLines > 0) { @@ -508,9 +514,10 @@ private: Offset = 0; } } - Replaces.insert(tooling::Replacement(SourceMgr, Tok.WhiteSpaceStart, - Tok.WhiteSpaceLength, NewLineText + - std::string(Spaces, ' '))); + Replaces.insert( + tooling::Replacement(SourceMgr, Tok.FormatTok.WhiteSpaceStart, + Tok.FormatTok.WhiteSpaceLength, + NewLineText + std::string(Spaces, ' '))); } /// \brief Add a new line and the required indent before the first Token @@ -1061,7 +1068,8 @@ private: Left.is(tok::comma) || Right.is(tok::lessless) || Right.is(tok::arrow) || Right.is(tok::period) || Right.is(tok::colon) || Left.is(tok::semi) || - Left.is(tok::l_brace) || + Left.is(tok::l_brace) || Left.is(tok::question) || + Left.Type == TT_ConditionalExpr || (Left.is(tok::l_paren) && !Right.is(tok::r_paren)); } diff --git a/unittests/Format/FormatTest.cpp b/unittests/Format/FormatTest.cpp index d280f1dc6c..9bf3097a5c 100644 --- a/unittests/Format/FormatTest.cpp +++ b/unittests/Format/FormatTest.cpp @@ -759,6 +759,15 @@ TEST_F(FormatTest, AlignsAfterReturn) { " aaaaaaaaaaaaaaaaaaaaaaaaa);"); } +TEST_F(FormatTest, BreaksConditionalExpressions) { + verifyFormat( + "aaaa(aaaaaaaaaaaaaaaaaaaa,\n" + " aaaaaaaaaaaaaaaaaaaaaaaaaa ? aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa :\n" + " aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa);"); + verifyFormat("aaaa(aaaaaaaaaaaaaaaaaaaa, aaaaaaaaaaaaaaaaaaaaaaaaaa ?\n" + " aaaaaaaaaaaaaaaaaaaaaaa : aaaaaaaaaaaaaaaaaaaaa);"); +} + TEST_F(FormatTest, AlignsStringLiterals) { verifyFormat("loooooooooooooooooooooooooongFunction(\"short literal \"\n" " \"short literal\");"); -- 2.40.0