On Mon, Apr 14, 2014 at 1:09 AM, suyog sarda <sardas...@gmail.com> wrote:
> Hi Richard,
>
> Thanks for the review. I Agree to your explaination. Following are the
> points what i understood :
>
> 1. setInstantiatedFromMemberTemplate call should not be skipped since it
> causes mix up of template parameter list numbering and wrong
> substitutions,
> which is clearly visible in the test case as well.
>
More fundamentally, it shouldn't be skipped because this function is
instantiated from a "member template" (it's not actually a member, but this
really means class-scope template, not member template).
> 2. When C is instantiated twice, it should throw re-definition error of
> function f, which is not done (with or without my patch).
>
That should be handled by the 'if (isFriend) {' loop starting at around
line 1424.
> 3. In the same function call
> 'TemplateDeclInstantiator::VisitFunctionDecl', there is a check 'if
> (isFriend)'.
> There is a standard described '[temp.friend] p4 (DR329)' inside that
> check, which checks if the *FunctionDecl D *is Declared as well as
> Defined (isThisDeclarationADefintion).
> In my opinion, at this point the function defintion is not being set
> for friend function defined inside the class template.
>
There's also something strange going on in VisitFunctionTemplateDecl --
that code is also calling setInstantiatedFromMemberTemplate for a friend
template definition. So at least there appears to be some redundancy here.
4. While debugging, it is seen that, the *FunctionDecl D *is *defined *holds
> true but '*Function->isDefined(Definition)*' evaluates to false (I did
> not get why it evaluates to false), because of which
> re-defintion error is not emitted.
>
The function isn't defined because we've not instantiated its definition
yet; that's probably to be expected.
> There is a similar bug for operator overloading - Bug 14785. There also
> same case is happening - not able to identify if friend function (in 14785
> its operator overloading function) is defined inside class template.
>
> Can you please help me out if my above analysis is correct as well as
> indicate where/how to set the definition of the declared function? Your
> help is highly appreciated.
>
I don't get see what's going wrong, but I suspect something in
FunctionDecl::getTemplateInstantiationPattern() is broken.
> On Sat, Apr 12, 2014 at 1:57 AM, Richard Smith <rich...@metafoo.co.uk>wrote:
>
>> Hi!
>>
>> I'm afraid this patch is incorrect; it even does the wrong thing in your
>> test case. Skipping the setInstantiatedFromMemberTemplate call causes us to
>> mix up the template parameter list numbering -- we substitute the argument
>> given for T as the value U, and we don't substitute anything for T. For
>> instance, in the definition of f instantiated for the f(3.0) call, we
>> should have T=double, U=int, but we actually have T=<no value>, U=double,
>> as can be seen in the AST dump.
>>
>> This also does the wrong thing if C is instantiated twice -- such a case
>> should be an error, because f would have multiple definitions.
>>
>>
>> On Mon, Mar 24, 2014 at 5:29 AM, suyog sarda <sardas...@gmail.com>wrote:
>>
>>> Gentle Ping!!
>>>
>>>
>>> On Thu, Mar 20, 2014 at 9:46 AM, suyog sarda <sardas...@gmail.com>wrote:
>>>
>>>> Gentle Ping!!
>>>>
>>>>
>>>> On Fri, Mar 14, 2014 at 10:29 PM, suyog sarda <sardas...@gmail.com>wrote:
>>>>
>>>>> Hi,
>>>>>
>>>>> Attaching patch for bug 19095. Please help in reviewing the same.
>>>>>
>>>>> Also, I haven't attached a test case yet in the patch as i am not sure
>>>>> how it should be and in which file it should be.
>>>>>
>>>>> In my opinion, the test case would go into
>>>>> *tools/clang/test/SemaCXX/friend.cpp
>>>>> *would be something like below (similar to that mentioned in the bug)
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>> * template <class T>void f(T);template <class U>class C{ template
>>>>> <class T> friend void f(T) { C<U> c; c.i = 3; } public :
>>>>> void g() { f(3.0); // OK } int i;};void h (){ f(7); //
>>>>> OK C<double> c; c.g();}*
>>>>>
>>>>>
>>>>> Please help in reviewing the patch as well as the test case.
>>>>> --
>>>>> With regards,
>>>>> Suyog Sarda
>>>>>
>>>>
>>>>
>>>>
>>>> --
>>>> With regards,
>>>> Suyog Sarda
>>>>
>>>
>>>
>>>
>>> --
>>> With regards,
>>> Suyog Sarda
>>>
>>> _______________________________________________
>>> cfe-commits mailing list
>>> cfe-commits@cs.uiuc.edu
>>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>>>
>>>
>>
>
>
> --
> With regards,
> Suyog Sarda
>
_______________________________________________
cfe-commits mailing list
cfe-commits@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
