[cfe-commits] r171390 - in /cfe/trunk: lib/Format/Format.cpp unittests/Format/FormatTest.cpp

[Daniel Jasper]

Well, there are two additional spaces.

One is before the "*". This is not present in google-style and there is a
FIXME in the test to look into that for LLVM style (and I referenced it on
the bug report).

Thus, I thought you were referring to the second, the space after ")".

Both will be addressed.


On Wed, Jan 2, 2013 at 5:13 PM, Nico Weber <thakis at chromium.org> wrote:

> On Wed, Jan 2, 2013 at 8:03 AM, Daniel Jasper <djasper at google.com> wrote:
> >
> >
> >
> > On Wed, Jan 2, 2013 at 4:52 PM, Nico Weber <thakis at chromium.org> wrote:
> >>
> >> On Wed, Jan 2, 2013 at 7:47 AM, Daniel Jasper <djasper at google.com>
> wrote:
> >> > Author: djasper
> >> > Date: Wed Jan  2 09:46:59 2013
> >> > New Revision: 171390
> >> >
> >> > URL: http://llvm.org/viewvc/llvm-project?rev=171390&view=rev
> >> > Log:
> >> > Correctly format pointers and references in casts.
> >>
> >> Cool!
> >>
> >> >
> >> > This fixes llvm.org/PR14747.
> >> >
> >> > Before: Type *A = (Type * ) P;
> >> > After:  Type *A = (Type *) P;
> >>
> >> Shouldn't this be "(Type*)P" though? At least for google style? (It
> >> feels like that's more common in clang too, but I haven't checked if
> >> that's true.)
> >
> >
> > I think you are right. It has been quite a while since I have used this
> type
> > of cast.. ;-)
>
> I meant static_cast too. Since that's easy to grep for, this time with
> data:
>
> Nicos-MacBook-Pro:clang thakis$ ack 'static_cast[^>]*[ ]\*>' lib | wc -l
>       89
> Nicos-MacBook-Pro:clang thakis$ ack 'static_cast[^>]*[^ ]\*>' lib | wc -l
>      162
>
> > It is a separate issue, though. I will address it soon.
>
> Thanks!
>
> >
> >> >
> >> > 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=171390&r1=171389&r2=171390&view=diff
> >> >
> >> >
> ==============================================================================
> >> > --- cfe/trunk/lib/Format/Format.cpp (original)
> >> > +++ cfe/trunk/lib/Format/Format.cpp Wed Jan  2 09:46:59 2013
> >> > @@ -822,19 +822,23 @@
> >> >    TokenAnnotation::TokenType determineStarAmpUsage(unsigned Index,
> bool
> >> > IsRHS) {
> >> >      if (Index == Annotations.size())
> >> >        return TokenAnnotation::TT_Unknown;
> >> > +    const FormatToken &PrevToken = Line.Tokens[Index - 1];
> >> > +    const FormatToken &NextToken = Line.Tokens[Index + 1];
> >> >
> >> > -    if (Index == 0 || Line.Tokens[Index - 1].Tok.is(tok::l_paren) ||
> >> > -        Line.Tokens[Index - 1].Tok.is(tok::comma) ||
> >> > -        Line.Tokens[Index - 1].Tok.is(tok::kw_return) ||
> >> > -        Line.Tokens[Index - 1].Tok.is(tok::colon) ||
> >> > +    if (Index == 0 || PrevToken.Tok.is(tok::l_paren) ||
> >> > +        PrevToken.Tok.is(tok::comma) ||
> >> > PrevToken.Tok.is(tok::kw_return) ||
> >> > +        PrevToken.Tok.is(tok::colon) ||
> >> >          Annotations[Index - 1].Type ==
> >> > TokenAnnotation::TT_BinaryOperator)
> >> >        return TokenAnnotation::TT_UnaryOperator;
> >> >
> >> > -    if (Line.Tokens[Index - 1].Tok.isLiteral() ||
> >> > -        Line.Tokens[Index + 1].Tok.isLiteral() ||
> >> > -        Line.Tokens[Index + 1].Tok.is(tok::kw_sizeof))
> >> > +    if (PrevToken.Tok.isLiteral() || NextToken.Tok.isLiteral() ||
> >> > +        NextToken.Tok.is(tok::kw_sizeof))
> >> >        return TokenAnnotation::TT_BinaryOperator;
> >> >
> >> > +    if (NextToken.Tok.is(tok::comma) || NextToken.Tok.is
> (tok::r_paren)
> >> > ||
> >> > +        NextToken.Tok.is(tok::greater))
> >> > +      return TokenAnnotation::TT_PointerOrReference;
> >> > +
> >> >      // It is very unlikely that we are going to find a pointer or
> >> > reference type
> >> >      // definition on the RHS of an assignment.
> >> >      if (IsRHS)
> >> >
> >> > Modified: cfe/trunk/unittests/Format/FormatTest.cpp
> >> > URL:
> >> >
> http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=171390&r1=171389&r2=171390&view=diff
> >> >
> >> >
> ==============================================================================
> >> > --- cfe/trunk/unittests/Format/FormatTest.cpp (original)
> >> > +++ cfe/trunk/unittests/Format/FormatTest.cpp Wed Jan  2 09:46:59 2013
> >> > @@ -699,6 +699,9 @@
> >> >    verifyFormat("A<int **> a;");
> >> >    verifyFormat("A<int *, int *> a;");
> >> >    verifyFormat("A<int **, int **> a;");
> >> > +  verifyFormat("Type *A = static_cast<Type *>(P);");
> >> > +  verifyFormat("Type *A = (Type *) P;");
> >> > +  verifyFormat("Type *A = (vector<Type *, int *>) P;");
> >> >
> >> >    verifyGoogleFormat("int main(int argc, char** argv) {\n}");
> >> >    verifyGoogleFormat("A<int*> a;");
> >> >
> >> >
> >> > _______________________________________________
> >> > 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/20130102/512078b3/attachment.html>