Patch for Vector Literal source ranges (Bug 15095)

Richard Smith richard at metafoo.co.uk
Mon Feb 4 21:56:31 PST 2013


Commited in r174339. Thank you! Please go ahead and close PR15095.

On Mon, Feb 4, 2013 at 3:31 PM, Richard Smith <richard at metafoo.co.uk> wrote:

> 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/250173f8/attachment.html>


More information about the cfe-commits mailing list