Patch for Vector Literal source ranges (Bug 15095)

Richard Smith richard at metafoo.co.uk
Mon Feb 4 15:31:23 PST 2013


Thanks, I'll commit this once SVN comes back up.

On Mon, Feb 4, 2013 at 3:16 PM, John Stratton <john at multicorewareinc.com>wrote:

> Looks like init list and compound literal didn't have location test
> cases before now, so I threw in four test cases based on the vector
> literals.  I'm assuming more test coverage than requested isn't a bad
> thing?
>
> Note: also had to edit unittest/AST/MatchVerifier to add OpenCL as a
> selectable language, and include/clang/ASTMatcher/ASTMatchers.h to add
> compound literals as matchable AST nodes.
>
> --John
>
> On 2/1/13, Richard Smith <richard at metafoo.co.uk> wrote:
> > 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
> > <john at multicorewareinc.com>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 <richard at metafoo.co.uk> wrote:
> >> > On Wed, Jan 30, 2013 at 10:57 AM, John Stratton
> >> > <john at multicorewareinc.com> 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).
> >> >
> >>
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130204/a0dceb44/attachment.html>


More information about the cfe-commits mailing list