PR19352 getLocation() points to the wrong position for FriendDecls

[Manuel Klimek]

On Fri, May 23, 2014 at 3:04 PM, Nikola Smiljanic <popizdeh at gmail.com>wrote:

> Where were you three minutes ago :P I'll have a look and come up with
> something.
>

Yea, sorry I completely missed Richard's question. Oh well, adding
regression tests after-the-fact is not the end of the world as long as we
make sure they actually fail when we remove the fix ;)


>
>
> On Fri, May 23, 2014 at 11:02 PM, Manuel Klimek <klimek at google.com> wrote:
>
>> On Fri, May 23, 2014 at 3: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...
>>>
>>
>> This is where we can add regression tests for stuff like this:
>> unittests/AST/SourceLocationTest.cpp
>>
>> Cheers,
>> /Manuel
>>
>>
>>>
>>>
>>> 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/8a6d028b/attachment.html>