Patch for Vector Literal source ranges (Bug 15095)
John Stratton
john at multicorewareinc.com
Mon Feb 4 15:16:09 PST 2013
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 --------------
A non-text attachment was scrubbed...
Name: vector_literal_parenloc.patch
Type: text/x-patch
Size: 4454 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130204/6742ccc3/attachment.bin>
More information about the cfe-commits
mailing list