[clang] patch for 18645

[Richard Smith]

On Fri, Jan 9, 2015 at 11:35 AM, Nathan Sidwell <nathan at acm.org> wrote:

> the attached patch fixes bu 18645:
> http://llvm.org/bugs/show_bug.cgi?id=18645
>
> In c++11, call deduction of a template parameter of type 'T &&' will
> deduce T as lvalue reference, if the corresponding argument is an lvalue.
> The bug occurs when the argument is (for instance) a template-id-expr for a
> template function.  At the point of checking this behaviour, the id-expr
> hasn't been resolved and the modification to an lvalue ref doesn't occur.
> We end up deducing T as a function type, and further never decay  it to
> pointer to function.
>
> It is this bit of code that's trying that test:
> -    if (isa<RValueReferenceType>(ParamType)) {
> -      if (!PointeeType.getQualifiers() &&
> -          isa<TemplateTypeParmType>(PointeeType) &&
> -          Arg->Classify(S.Context).isLValue() &&
> -          Arg->getType() != S.Context.OverloadTy &&
> -          Arg->getType() != S.Context.BoundMemberTy)
>
> and for an arg of template-id-expr, Arg->getType () is an OverloadTy, so
> the test fails.
>
> Just after this test is the following deduction check:
>   if (ArgType == S.Context.OverloadTy) {
>     ArgType = ResolveOverloadForDeduction(S, TemplateParams,
>                                           Arg, ParamType,
>                                           ParamRefType != nullptr);
> which is doing the template-id resolution needed.  Now, just after that is
> the following:
>
>   if (ParamRefType) {
>     // C++0x [temp.deduct.call]p3:
>     //   [...] If P is of the form T&&, where T is a template parameter,
> and
>     //   the argument is an lvalue, the type A& is used in place of A for
>     //   type deduction.
>     if (ParamRefType->isRValueReferenceType() &&
>         ParamRefType->getAs<TemplateTypeParmType>() &&  <-- this line
>         Arg->isLValue())
>       ArgType = S.Context.getLValueReferenceType(ArgType);
>
> which is also attempting to do the same check, but it's flubbed the
> wording on the indicated line by checking ParamRefType not
> ParamRefType->getPointeeType () (aka ParamType at this point).
>
> Just fixing that line would work, but it is unnecessary and wrong to have
> the check remain in two places.  Hence I removed the checking from before
> the call to ResolveOverloadForDeduction and moved it all into the lower if
> conditional. For good measure I moved the earlier check about incomplete
> array types there too, to keep the reference parameter handling all in one
> place.  The only piece of this check that happens before the ROFD call is
> to remove the referenceness from ParamType.
>
> built and tested on x86_64-linux.  ok?


You have a tab character on the

+ isa<TemplateTypeParmType>(ParamType) &&

line; please convert to spaces.

You can make the test a bit simpler:

  template<typename F> F Quux(F &&f);
  float (&Baz)(float&&) = Quux(Quux<float>);

Other than that, LGTM.

(The BoundMemberTy check is missing compared to the original, but it looks
like that never actually did anything here, because we'd fall through into
the second copy of the 'create an lvalue reference' code regardless.)

Thank you!


>
> nathan
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150109/a7353ba7/attachment.html>