[cfe-commits] r171957 - in /cfe/trunk: lib/Format/Format.cpp unittests/Format/FormatTest.cpp
Chandler Carruth
chandlerc at google.com
Wed Jan 9 02:07:13 PST 2013
On Tue, Jan 8, 2013 at 11:06 PM, Daniel Jasper <djasper at google.com> 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
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130109/83ebe2e5/attachment.html>
More information about the cfe-commits
mailing list