PR19352 getLocation() points to the wrong position for FriendDecls

[Nikola Smiljanic]

r209511


On Fri, May 23, 2014 at 11:06 AM, Nikola Smiljanic <popizdeh at gmail.com>wrote:

> That's what I asked in my first email :D I don't see any existing tests
> for something like this...
>
>
> On Fri, May 23, 2014 at 11:05 AM, Richard Smith <richard at metafoo.co.uk>wrote:
>
>> LGTM
>>
>> Is there any way you can test this?
>>
>>
>> On Mon, May 19, 2014 at 3:07 PM, Nikola Smiljanic <popizdeh at gmail.com>wrote:
>>
>>> Ping.
>>>
>>>
>>> On Fri, May 16, 2014 at 9:44 AM, Nikola Smiljanic <popizdeh at gmail.com>wrote:
>>>
>>>> Waiting for an OK from you Richard :)
>>>>
>>>>
>>>> On Sat, May 10, 2014 at 7:38 PM, Nikola Smiljanic <popizdeh at gmail.com>wrote:
>>>>
>>>>> I figured out what's going on, I was initially trying to use the
>>>>> location returned by getTypeSpecTypeNameLoc but decided against it once I
>>>>> saw the assert in this function. Using the location from TypeSourceInfo
>>>>> does seem to give more consistent result. Location always points to
>>>>> whatever comes after 'friend' keyword. The change itself is a oneliner. Are
>>>>> you guys happy with this version?
>>>>>
>>>>>
>>>>> On Fri, May 9, 2014 at 11:55 AM, Nikola Smiljanic <popizdeh at gmail.com>wrote:
>>>>>
>>>>>> You're absolutely right. But I'm seeing something strange now,
>>>>>> location points to 'class A' and not 'A'. Same with fully qualified names.
>>>>>> Has there been a recent change that did this? And more importantly are you
>>>>>> happy with this as FriendDecl's location?
>>>>>>
>>>>>>
>>>>>> On Fri, May 9, 2014 at 9:43 AM, Richard Smith <richard at metafoo.co.uk>wrote:
>>>>>>
>>>>>>> Can you use TSI->getTypeLoc().getLocStart() instead of wiring
>>>>>>> through a new SourceLocation?
>>>>>>>
>>>>>>>
>>>>>>> On Thu, May 8, 2014 at 4:37 PM, Nikola Smiljanic <popizdeh at gmail.com
>>>>>>> > wrote:
>>>>>>>
>>>>>>>> Nudge nudge :)
>>>>>>>>
>>>>>>>>
>>>>>>>> On Tue, Apr 29, 2014 at 5:35 PM, Manuel Klimek <klimek at google.com>wrote:
>>>>>>>>
>>>>>>>>> +richard
>>>>>>>>>
>>>>>>>>> This makes sense to me, but I don't understand the code well
>>>>>>>>> enough to approve... Looping in Richard for an expert opinion ;)
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> On Tue, Apr 29, 2014 at 5:13 AM, Nikola Smiljanic <
>>>>>>>>> popizdeh at gmail.com> wrote:
>>>>>>>>>
>>>>>>>>>> Ping.
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> On Wed, Apr 16, 2014 at 8:38 PM, Nikola Smiljanic <
>>>>>>>>>> popizdeh at gmail.com> wrote:
>>>>>>>>>>
>>>>>>>>>>> LocStart in CheckFriendTypeDecl is the start of DeclSpec range.
>>>>>>>>>>> This is OK for checking that declaration starts with 'friend' keyword but
>>>>>>>>>>> is redundant when it comes to FriendDecl creation. Location of the keyword
>>>>>>>>>>> is already passed as FriendLoc.
>>>>>>>>>>>
>>>>>>>>>>> I've added another parameter to this function that points to
>>>>>>>>>>> location of the type from the declaration. The call to CheckFriendTypeDecl
>>>>>>>>>>> from SemaTemplateInstantiateDecl now isn't technically correct because it's
>>>>>>>>>>> passing the the type location as LocStart. But the error that's checked for
>>>>>>>>>>> using this parameter can only happen inside class declaration, not
>>>>>>>>>>> instantiated template, I think :)
>>>>>>>>>>>
>>>>>>>>>>> Is there a way to test this? I'm not seeing any regressions.
>>>>>>>>>>>
>>>>>>>>>>> struct A{};
>>>>>>>>>>> typedef A Atypedef;
>>>>>>>>>>>
>>>>>>>>>>> namespace ns { struct B{}; }
>>>>>>>>>>>
>>>>>>>>>>>  template <typename T>
>>>>>>>>>>> class C
>>>>>>>>>>> {
>>>>>>>>>>> friend class A; // points to A
>>>>>>>>>>> friend Atypedef; // points to Atypedef
>>>>>>>>>>> friend T; //  points to T
>>>>>>>>>>> friend decltype(whatever); // points to decltype
>>>>>>>>>>> friend ns::B; // points to B
>>>>>>>>>>> friend typename T::something; // points to something
>>>>>>>>>>> }
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140523/bd3784ef/attachment.html>