[PATCH] Fix return type deduction for member templates (revised)

Richard Smith richard at metafoo.co.uk
Fri Jun 14 14:09:57 PDT 2013


On Fri, Jun 14, 2013 at 5:40 AM, Faisal Vali <faisalv at gmail.com> wrote:
> Hi Richard,
>    please let me know if you're ok with me committing the attached patch with
> the following log:
> A quick fix to allow return type deduction on member templates
> by ensuring DiagnoseUseOfDecl is called both on the found decl and the
> decl being used
> (i.e the specialization in the case of member templates) whenever they
> are different.
> Per the exchange captured in
> http://lists.cs.uiuc.edu/pipermail/cfe-commits/Week-of-Mon-20130610/081636.html
> a more comprehensive fix that allows both decls to be passed
> into DiagnoseUseOfDecl is (or should be) forthcoming relatively soon.

Yes, this is fine.

> Faisal Vali
>
>
>
> On Thu, Jun 13, 2013 at 7:38 PM, Richard Smith <richard at metafoo.co.uk> wrote:
>> On Thu, Jun 13, 2013 at 5:11 PM, Faisalv <faisalv at gmail.com> wrote:
>>> Also can i just check if the founddecl and the decl being used are different - and if so, call diagnoseusedecl on both - or does one have to be a specialization of the other? (do u have an example in mind where it is not a specilaization)
>>
>> It should be fine to call DiagnoseUseOfDecl on both if they're
>> different; they're both "used" in some sense.
>>
>>> On Jun 13, 2013, at 8:03 PM, Faisalv <faisalv at gmail.com> wrote:
>>>
>>>>
>>>>
>>>> Sent from my iPhone
>>>>
>>>> On Jun 13, 2013, at 7:36 PM, Richard Smith <richard at metafoo.co.uk> wrote:
>>>>
>>>>> On Thu, Jun 13, 2013 at 3:40 PM, Faisalv <faisalv at gmail.com> wrote:
>>>>>>
>>>>>>
>>>>>> On Jun 13, 2013, at 4:30 PM, Richard Smith <richard at metafoo.co.uk> wrote:
>>>>>>
>>>>>>> Hi Faisal,
>>>>>>>
>>>>>>> On Thu, Jun 13, 2013 at 11:30 AM, Faisal Vali <faisalv at gmail.com> wrote:
>>>>>>>> While implementing return type deduction for generic lambdas, I stumbled upon
>>>>>>>> a bug which led to clang mishandling the following code:
>>>>>>>>
>>>>>>>> struct Lambda {
>>>>>>>> template<class T> auto operator()(T t) {
>>>>>>>>  return 5;
>>>>>>>> }
>>>>>>>> template<class T> auto foo(T t) { return t; }
>>>>>>>> };
>>>>>>>> void test() {
>>>>>>>>  Lambda L;
>>>>>>>>  int i = L(3);
>>>>>>>> }
>>>>>>>>
>>>>>>>>
>>>>>>>> The issue (as i understand it) was that in some contexts only the
>>>>>>>> function template (and not the specialization) would get passed to
>>>>>>>> DiagnoseUseOfDecl (which deduces the return type of the function).  So
>>>>>>>> the specialization would flow through to Codegen with auto as its
>>>>>>>> return type.
>>>>>>>>
>>>>>>>> So, with that in mind, this patch implements a trivial fix.
>>>>>>>>
>>>>>>>> What do you think Richard? Is there a better way to address this bug?
>>>>>>>
>>>>>>> I was aware of this issue, but got snowed under with other things.
>>>>>>> This bug is not limited to 'auto' type deduction; the other code in
>>>>>>> DiagnoseUseOfDecl suffers the same way. For instance:
>>>>>>>
>>>>>>> struct Lambda {
>>>>>>> template<class T> static __attribute__((unused)) int foo(T) {
>>>>>>>  return 5;
>>>>>>> }
>>>>>>> };
>>>>>>> int bar() {
>>>>>>>  Lambda L;
>>>>>>>  return L.foo(3);
>>>>>>> }
>>>>>>>
>>>>>>> ... does not warn that the function is marked 'unused' but is used,
>>>>>>> but if you call it as Lambda::foo(3), it does warn.
>>>>>>>
>>>>>>> I think the right fix here is to pass both the declaration found by
>>>>>>> name lookup and the declaration that is actually used into
>>>>>>> DiagnoseUseOfDecl.
>>>>>>>
>>>>>>
>>>>>> What if i just call diagnoseuseofdecl again on the used specialization instead of adding a parameter to it?
>>>>>
>>>>> If this is the only call which has the problem, then that would be OK.
>>>>> I suspect there are other cases, though, and there are probably cases
>>>>> which have the opposite bug (passing only the used decl and not the
>>>>> found decl). As a quick fix, making two calls would work, but it's
>>>>> probably not what we want in the longer term.
>>>>
>>>>
>>>> If its ok w u, for now id rather do the quick fix, place the appropriate fixmes - and then try and return to this once generic lambdas are starting to be reasonably usable (plus i still have to figure out that pesky non-odr use, rvalue emission issue - which also i'm going to deprioritize a lil in favor of usable generic lambdas for now). Thoughts?




More information about the cfe-commits mailing list