Re: C++ PATCH to Implement P0846R0, ADL and function templates [v2]

[Jason Merrill <jason at redhat dot com>]

On 10/31/18 6:45 PM, Marek Polacek wrote:
> On Mon, Oct 29, 2018 at 05:59:13PM -0400, Jason Merrill wrote:
> > On 10/28/18 3:56 PM, Marek Polacek wrote:
> > > This patch implements P0846R0: ADL and Function Templates that are not Visible
> > > <http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2017/p0846r0.html>
> > > whereby a name for which a normal lookup produces either no result or finds one
> > > or more functions and that is followed by a "<" would be treated as if a function
> > > template name had been found and would cause ADL to be performed.  Thus e.g.
> > >
> > > namespace N {
> > >     struct S { };
> > >     template<int X> void f(S);
> > > }
> > >
> > > void
> > > bar (N::S s)
> > > {
> > >     f<3>(s);
> > > }
> > >
> > > will now compile; ADL will find N::f.  The gist of the approach I took is in
> > > cp_parser_template_name and setting TEMPLATE_ID_TRY_ADL_P.
> >
> > Why do you need that flag?  Can't you tell from the first operand of the
> > TEMPLATE_ID_EXPR whether it's suitable?
>
> I needed to distinguish between the two kinds of identifiers
> cp_parser_template_name can return.  But I found out I can (ab)use
> IDENTIFIER_BINDING for that.  If it's empty then name lookup didn't find
> anything (push_binding wasn't called for such an id), so we should try ADL.
> Otherwise it's a dependent name and we need to wait for instantiation.
>
> > > There's something I'm not clear on; the proposal talks about an
> > > unqualified-id followed by a <, which is also the case for
> > >
> > >     a.foo < 1;
> > >
> > > which is "postfix-expression . unqualified-id <", but treating "foo" as a
> > > template name would break valid programs.  I don't think this proposal should
> > > be in effect for class member accesses, so I've disabled it by using scoped_p
> > > below.  See fn-template8.C for a complete testcase for this scenario.
> >
> > Agreed; ADL doesn't apply to class member access, so it shouldn't apply
> > here.  Sent mail to the core reflector.
>
> Great, thanks for that.
>
> > > @@ -7165,7 +7165,9 @@ cp_parser_postfix_expression (cp_parser *parser, bool address_p, bool cast_p,
> > >    	    if (idk == CP_ID_KIND_UNQUALIFIED
> > >    		|| idk == CP_ID_KIND_TEMPLATE_ID)
> > >    	      {
> > > -		if (identifier_p (postfix_expression))
> > > +		if (identifier_p (postfix_expression)
> > > +		    || (TREE_CODE (postfix_expression) == TEMPLATE_ID_EXPR
> > > +			&& TEMPLATE_ID_TRY_ADL_P (postfix_expression)))
> >
> > Here I think you can check whether the first operand is an identifier.
>
> Changed, along with checking for IDENTIFIER_BINDING.  E.g. this is a test that
> would fail if we tried ADL for a template-id with any identifier:
>
>    template <int> struct X {
>      X() { fn<>(0); }
>      template <int> void fn();
>    };
>
> here we need to perform two-stage lookup.

I don't see why; when we look up fn we find the member template, and we 
should remember that.  I think this code:

>   /* If DECL is dependent, and refers to a function, then just return                                          
>      its name; we will look it up again during template instantiation.  */
>   if (DECL_FUNCTION_TEMPLATE_P (decl) || !DECL_P (decl))
>     {
>       tree scope = ovl_scope (decl);
>       if (TYPE_P (scope) && dependent_type_p (scope))
>         return identifier;
>     }

should go so that returning an identifier can consistently mean nothing 
found.

Jason