This looks fine. Please also provide a new testcase for
unittests/AST/SourceLocationTest.cpp.

On Fri, Feb 1, 2013 at 5:29 PM, John Stratton <[email protected]>wrote:

> Thanks for the feedback.  Here's an updated patch.  To some extent,
> yes, this situation is weird, but I think the only valid input
> exercising this code path should are cases where it would have been
> semantically equivalent to braces anyway.
>
> --John
>
> On 1/30/13, Richard Smith <[email protected]> wrote:
> > On Wed, Jan 30, 2013 at 10:57 AM, John Stratton
> > <[email protected]> wrote:
> >> Hello.  I've attached a small patch that fixes the 15095 bug I filed a
> >> couple of days ago, and makes sure that the CompoundLiteral and child
> >> AST nodes created from a vector literal have correct source-location
> >> information.  Does this make sense to the veteran Clang developers?
> >
> > It's a bit weird that we're building a CompoundLiteralExpr here for a
> > construct which used parentheses not braces, but given that's our
> > representation, it seems appropriate to pretend the braces are where
> > the parens of the initializer were.
> >
> > +  if (isVectorLiteral) {
> > +    SourceLocation litLParenLoc = (PE ? PE->getLParen() :
> > PLE->getLParenLoc());
> > +    SourceLocation litRParenLoc = (PE ? PE->getRParen() :
> > PLE->getRParenLoc());
> >
> > Please compute these locations in BuildVectorLiteral instead (within
> > its 'if (ParenListExpr *PE = dyn_cast<ParenListExpr>(E))' code).
> >
> > Also, variable names should start with an uppercase letter (this code
> > breaks the convention in a few places, but new code should conform).
> >
>
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to