On Tue, Jan 8, 2013 at 11:06 PM, Daniel Jasper <[email protected]> wrote:
> Author: djasper > Date: Wed Jan 9 01:06:56 2013 > New Revision: 171957 > > URL: http://llvm.org/viewvc/llvm-project?rev=171957&view=rev > Log: > 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(); > <breaks out the bikeshed paint> There is a reasonably common pattern in Clang-land at least that I find significantly more readable: Diag(NewFD->getLocation(), getLangOpts().MicrosoftExt ? diag::ext_function_specialization_in_class : diag::err_function_specialization_in_class) << NewFD->getDeclName(); Thoughts? > Modified: > cfe/trunk/lib/Format/Format.cpp > cfe/trunk/unittests/Format/FormatTest.cpp > > Modified: cfe/trunk/lib/Format/Format.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/Format.cpp?rev=171957&r1=171956&r2=171957&view=diff > > ============================================================================== > --- cfe/trunk/lib/Format/Format.cpp (original) > +++ cfe/trunk/lib/Format/Format.cpp Wed Jan 9 01:06:56 2013 > @@ -269,30 +269,32 @@ > /// 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 @@ > 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 @@ > > 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 @@ > 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 @@ > > /// \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 @@ > /// > /// 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 @@ > 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 @@ > 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)); > } > > > Modified: cfe/trunk/unittests/Format/FormatTest.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=171957&r1=171956&r2=171957&view=diff > > ============================================================================== > --- cfe/trunk/unittests/Format/FormatTest.cpp (original) > +++ cfe/trunk/unittests/Format/FormatTest.cpp Wed Jan 9 01:06:56 2013 > @@ -759,6 +759,15 @@ > " 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\");"); > > > _______________________________________________ > cfe-commits mailing list > [email protected] > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
