On Thu, Nov 1, 2018 at 3:36 PM Marek Polacek <pola...@redhat.com> wrote:
> On Thu, Nov 01, 2018 at 12:15:48PM -0400, Jason Merrill wrote:
> > 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.
>
> Woo!  That's nice that I can do that: it simplifies other stuff.  Removing
> this hunk triggered an assert in tsubst_copy (just one testcase), but when
> I moved it, everything seemed to work well.
>
> So this is a version which doesn't use IDENTIFIER_BINDING at all.  Thanks,
>
> Bootstrapped/regtested on x86_64-linux, ok for trunk?
>
> 2018-11-01  Marek Polacek  <pola...@redhat.com>
>
>         Implement P0846R0, ADL and function templates.
>         * decl.c (grokfndecl): Allow FUNCTION_DECL in assert.
>         * lex.c (unqualified_fn_lookup_error): Handle TEMPLATE_ID_EXPR.
>         * parser.c (cp_parser_postfix_expression): Do ADL for a template-name.
>         (cp_parser_template_id): Give errors if parsing the template argument
>         list didn't go well.  Allow FUNCTION_DECL in assert.
>         (cp_parser_template_name): Consider a name to refer to a template if
>         it is an unqualified-id followed by a <.  Don't return the identifier
>         if the decl is a function and dependent.
>         * pt.c (tsubst_copy) <case OVERLOAD>: Remove assert.
>
>         * g++.dg/addr_builtin-1.C: Adjust dg-error.
>         * g++.dg/cpp2a/fn-template1.C: New test.
>         * g++.dg/cpp2a/fn-template10.C: New test.
>         * g++.dg/cpp2a/fn-template11.C: New test.
>         * g++.dg/cpp2a/fn-template12.C: New test.
>         * g++.dg/cpp2a/fn-template13.C: New test.
>         * g++.dg/cpp2a/fn-template14.C: New test.
>         * g++.dg/cpp2a/fn-template15.C: New test.
>         * g++.dg/cpp2a/fn-template16.C: New test.
>         * g++.dg/cpp2a/fn-template2.C: New test.
>         * g++.dg/cpp2a/fn-template3.C: New test.
>         * g++.dg/cpp2a/fn-template4.C: New test.
>         * g++.dg/cpp2a/fn-template5.C: New test.
>         * g++.dg/cpp2a/fn-template6.C: New test.
>         * g++.dg/cpp2a/fn-template7.C: New test.
>         * g++.dg/cpp2a/fn-template8.C: New test.
>         * g++.dg/cpp2a/fn-template9.C: New test.
>         * g++.dg/parse/fn-template1.C: New test.
>         * g++.dg/parse/fn-template2.C: New test.
>         * g++.dg/parse/template19.C: Adjust dg-error.
>         * g++.dg/template/pr61745.C: Add target to dg-error.
>
> diff --git gcc/cp/decl.c gcc/cp/decl.c
> index 23fcf6b0471..f0033dd5458 100644
> --- gcc/cp/decl.c
> +++ gcc/cp/decl.c
> @@ -8857,7 +8857,9 @@ grokfndecl (tree ctype,
>              the information in the TEMPLATE_ID_EXPR.  */
>           SET_DECL_IMPLICIT_INSTANTIATION (decl);
>
> -         gcc_assert (identifier_p (fns) || TREE_CODE (fns) == OVERLOAD);
> +         gcc_assert (identifier_p (fns)
> +                     || TREE_CODE (fns) == OVERLOAD
> +                     || TREE_CODE (fns) == FUNCTION_DECL);
>           DECL_TEMPLATE_INFO (decl) = build_template_info (fns, args);
>
>           for (t = TYPE_ARG_TYPES (TREE_TYPE (decl)); t; t = TREE_CHAIN (t))
> diff --git gcc/cp/lex.c gcc/cp/lex.c
> index 410dfd1bc5b..26ec52f3498 100644
> --- gcc/cp/lex.c
> +++ gcc/cp/lex.c
> @@ -541,6 +541,9 @@ unqualified_fn_lookup_error (cp_expr name_expr)
>    if (loc == UNKNOWN_LOCATION)
>      loc = input_location;
>
> +  if (TREE_CODE (name) == TEMPLATE_ID_EXPR)
> +    name = TREE_OPERAND (name, 0);
> +
>    if (processing_template_decl)
>      {
>        /* In a template, it is invalid to write "f()" or "f(3)" if no
> diff --git gcc/cp/parser.c gcc/cp/parser.c
> index 206ceb048d4..263eaddd143 100644
> --- gcc/cp/parser.c
> +++ gcc/cp/parser.c
> @@ -7195,7 +7195,11 @@ 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)
> +                   /* In C++2A, we may need to perform ADL for a template
> +                      name.  */
> +                   || (TREE_CODE (postfix_expression) == TEMPLATE_ID_EXPR
> +                       && identifier_p (TREE_OPERAND (postfix_expression, 
> 0))))
>                   {
>                     if (!args->is_empty ())
>                       {
> @@ -16029,6 +16033,37 @@ cp_parser_template_id (cp_parser *parser,
>         }
>        /* Parse the arguments.  */
>        arguments = cp_parser_enclosed_template_argument_list (parser);
> +
> +      if ((cxx_dialect > cxx17)
> +         && (TREE_CODE (templ) == FUNCTION_DECL || identifier_p (templ))
> +         && !template_keyword_p
> +         && (cp_parser_error_occurred (parser)
> +             || cp_lexer_next_token_is_not (parser->lexer, CPP_OPEN_PAREN)))
> +       {
> +         /* This didn't go well.  */
> +         if (TREE_CODE (templ) == FUNCTION_DECL)
> +           {
> +             /* C++2A says that "function-name < a;" is now ill-formed.  */
> +             if (cp_parser_error_occurred (parser))
> +               {
> +                 error_at (token->location, "invalid 
> template-argument-list");
> +                 inform (token->location, "function name as the left hand "
> +                         "operand of %<<%> is ill-formed in C++2a; wrap the "
> +                         "function name in %<()%>");
> +               }
> +             else
> +               /* We expect "f<targs>" to be followed by "(args)".  */
> +               error_at (cp_lexer_peek_token (parser->lexer)->location,
> +                         "expected %<(%> after template-argument-list");
> +             if (start_of_id)
> +               /* Purge all subsequent tokens.  */
> +               cp_lexer_purge_tokens_after (parser->lexer, start_of_id);
> +           }
> +         else
> +           cp_parser_simulate_error (parser);
> +         pop_deferring_access_checks ();
> +         return error_mark_node;
> +       }
>      }
>
>    /* Set the location to be of the form:
> @@ -16085,6 +16120,7 @@ cp_parser_template_id (cp_parser *parser,
>          a function-template.  */
>        gcc_assert ((DECL_FUNCTION_TEMPLATE_P (templ)
>                    || TREE_CODE (templ) == OVERLOAD
> +                  || TREE_CODE (templ) == FUNCTION_DECL
>                    || BASELINK_P (templ)));
>
>        template_id = lookup_template_function (templ, arguments);
> @@ -16287,6 +16323,10 @@ cp_parser_template_name (cp_parser* parser,
>         }
>      }
>
> +  /* cp_parser_lookup_name clears OBJECT_TYPE.  */
> +  const bool scoped_p = ((parser->scope ? parser->scope
> +                         : parser->context->object_type) != NULL_TREE);
> +
>    /* Look up the name.  */
>    decl = cp_parser_lookup_name (parser, identifier,
>                                 tag_type,
> @@ -16319,6 +16359,25 @@ cp_parser_template_name (cp_parser* parser,
>         if (TREE_CODE (*iter) == TEMPLATE_DECL)
>           found = true;
>
> +      if (!found
> +         && (cxx_dialect > cxx17)
> +         && !scoped_p
> +         && cp_lexer_next_token_is (parser->lexer, CPP_LESS))
> +       {
> +         /* [temp.names] says "A name is also considered to refer to a 
> template
> +            if it is an unqualified-id followed by a < and name lookup finds
> +            either one or more functions or finds nothing."  */
> +
> +         /* The "more functions" case.  Just use the OVERLOAD as normally.  
> */
> +         if (TREE_CODE (decl) == OVERLOAD
> +             /* Name lookup found one function.  */
> +             || TREE_CODE (decl) == FUNCTION_DECL)
> +           found = true;

Is this specifically not using is_overloaded_fn to avoid considering
BASELINK?  That should be mentioned in a comment.

OK with that change.

Jason

Reply via email to