Re: [PATCH] c++: Member template function lookup failure [PR94799]

[Jason Merrill <jason@redhat.com>]

On 4/28/20 11:55 PM, Marek Polacek wrote:
> Whew, this took a while.  We fail to parse "p->template A<T>::a()"
> (where p is of type A<T> *) because since r249752 we treat the RHS of the ->
> as dependent and avoid a lookup in the enclosing context: since that rev
> cp_parser_template_name checks parser->context->object_type too, which
> here is unknown_type_node, signalling a type-dependent object:
> 
>   7756   if (dependent_p)
>   7757     /* Tell cp_parser_lookup_name that there was an object, even though it's
>   7758        type-dependent.  */
>   7759     parser->context->object_type = unknown_type_node;
> 
> with which cp_parser_template_name returns identifier 'A', cp_parser_class_name
> then creates a TEMPLATE_ID_EXPR A<T>, but then
> 
> 23735       decl = make_typename_type (scope, decl, tag_type, tf_error);
> 
> in cp_parser_class_name fails because scope is NULL.  Then we return
> error_mark_node and parse errors ensue.
> 
> I've tried various approaches, e.g. keeping TEMPLATE_ID_EXPR around
> instead of calling make_typename_type, which didn't work, whereupon I
> realized that since we don't want to perform name lookup if we've seen
> the template keyword and the scope is dependent, we can adjust
> parser->context->object_type and use the type of the object expression
> as the scope, even if it's type-dependent.  This should be in line with
> [basic.lookup.classref]p4.

> The "&& scope != unknown_type_node" line in cp_parser_class_name is there
> for diagnostic purposes only (to avoid issuing a confusing error).
> 
> Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
> (Happy to defer to GCC 11 if this doesn't seem very safe.)
> 
> 	PR c++/94799
> 	* parser.c (cp_parser_postfix_dot_deref_expression): If we have
> 	a type-dependent object of class type, stash it to
> 	parser->context->object_type.
> 	(cp_parser_class_name): Consider object scope too.  Don't call
> 	make_typename_type when the scope is unknown_type_node.
> 
> 	* g++.dg/lookup/this1.C: Adjust dg-error.
> 	* g++.dg/template/lookup12.C: New test.
> 	* g++.dg/template/lookup13.C: New test.
> 	* g++.dg/template/lookup14.C: New test.
> ---
>   gcc/cp/parser.c                          | 28 ++++++++++++++++++------
>   gcc/testsuite/g++.dg/lookup/this1.C      |  2 +-
>   gcc/testsuite/g++.dg/template/lookup12.C | 26 ++++++++++++++++++++++
>   gcc/testsuite/g++.dg/template/lookup13.C | 28 ++++++++++++++++++++++++
>   gcc/testsuite/g++.dg/template/lookup14.C | 11 ++++++++++
>   5 files changed, 87 insertions(+), 8 deletions(-)
>   create mode 100644 gcc/testsuite/g++.dg/template/lookup12.C
>   create mode 100644 gcc/testsuite/g++.dg/template/lookup13.C
>   create mode 100644 gcc/testsuite/g++.dg/template/lookup14.C
> 
> diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
> index e1f9786893a..b344721fb60 100644
> --- a/gcc/cp/parser.c
> +++ b/gcc/cp/parser.c
> @@ -7694,11 +7694,16 @@ cp_parser_postfix_dot_deref_expression (cp_parser *parser,
>     bool pseudo_destructor_p;
>     tree scope = NULL_TREE;
>     location_t start_loc = postfix_expression.get_start ();
> +  tree type = TREE_TYPE (postfix_expression);
>   
>     /* If this is a `->' operator, dereference the pointer.  */
>     if (token_type == CPP_DEREF)
> -    postfix_expression = build_x_arrow (location, postfix_expression,
> -					tf_warning_or_error);
> +    {
> +      if (type && POINTER_TYPE_P (type))
> +	type = TREE_TYPE (type);
> +      postfix_expression = build_x_arrow (location, postfix_expression,
> +					  tf_warning_or_error);
> +    }
>     /* Check to see whether or not the expression is type-dependent and
>        not the current instantiation.  */
>     dependent_p = type_dependent_object_expression_p (postfix_expression);
> @@ -7754,9 +7759,10 @@ cp_parser_postfix_dot_deref_expression (cp_parser *parser,
>       }
>   
>     if (dependent_p)
> -    /* Tell cp_parser_lookup_name that there was an object, even though it's
> -       type-dependent.  */
> -    parser->context->object_type = unknown_type_node;
> +    /* If we don't have a (type-dependent) object of class type, use
> +       unknown_type_node to signal that there was an object.  */
> +    parser->context->object_type = (type && CLASS_TYPE_P (type)
> +				    ? type : unknown_type_node);

Anything that depends on CLASS_TYPE_P won't work if 'p' isn't clearly a 
class, i.e. if it has type T*, T, or NULL_TREE.  Why not use any 
non-null type here?

For null type, I wonder if using decltype would make sense.

>     /* Assume this expression is not a pseudo-destructor access.  */
>     pseudo_destructor_p = false;
> @@ -23625,8 +23631,15 @@ cp_parser_class_name (cp_parser *parser,
>       }
>   
>     /* PARSER->SCOPE can be cleared when parsing the template-arguments
> -     to a template-id, so we save it here.  */
> -  scope = parser->scope;
> +     to a template-id, so we save it here.  Consider object scope too,
> +     so that make_typename_type below can use it (cp_parser_template_name
> +     considers object scope also).  This may happen with code like
> +
> +       p->template A<T>::a()
> +
> +      where we first want to look up A<T>::a in the class of the object
> +      expression, as per [basic.lookup.classref].  */
> +  scope = parser->scope ? parser->scope : parser->context->object_type;
>     if (scope == error_mark_node)
>       return error_mark_node;
>   
> @@ -23720,6 +23733,7 @@ cp_parser_class_name (cp_parser *parser,
>     /* Check to see that it is really the name of a class.  */
>     if (TREE_CODE (decl) == TEMPLATE_ID_EXPR
>         && identifier_p (TREE_OPERAND (decl, 0))
> +      && scope != unknown_type_node

This will make the more general dependent case give an error.

>         && cp_lexer_next_token_is (parser->lexer, CPP_SCOPE))
>       /* Situations like this:
>   
> diff --git a/gcc/testsuite/g++.dg/lookup/this1.C b/gcc/testsuite/g++.dg/lookup/this1.C
> index 20051bf7515..6b85cefcd37 100644
> --- a/gcc/testsuite/g++.dg/lookup/this1.C
> +++ b/gcc/testsuite/g++.dg/lookup/this1.C
> @@ -4,5 +4,5 @@
>   struct A
>   {
>       template<int> static void foo();
> -    static void bar() { this->A::foo<0>(); } // { dg-error "unavailable" }
> +    static void bar() { this->A::foo<0>(); } // { dg-error "unavailable|not a class|expected" }
>   };
> diff --git a/gcc/testsuite/g++.dg/template/lookup12.C b/gcc/testsuite/g++.dg/template/lookup12.C
> new file mode 100644
> index 00000000000..fc5939ab0f6
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/template/lookup12.C
> @@ -0,0 +1,26 @@
> +// PR c++/94799 - member template function lookup fails.
> +
> +template<typename T> struct B {
> +  void foo ();
> +  int i;
> +};
> +
> +template<typename T>
> +struct D : public B<T> { };
> +
> +template<typename T>
> +void fn (D<T> d)
> +{
> +  d.template B<T>::foo ();
> +  d.template B<T>::i = 42;
> +  D<T>().template B<T>::foo ();
> +  d.template D<T>::template B<T>::foo ();
> +  d.template D<T>::template B<T>::i = 10;
> +}
> +
> +int
> +main ()
> +{
> +  D<int> d;
> +  fn(d);
> +}
> diff --git a/gcc/testsuite/g++.dg/template/lookup13.C b/gcc/testsuite/g++.dg/template/lookup13.C
> new file mode 100644
> index 00000000000..a8c7e18a707
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/template/lookup13.C
> @@ -0,0 +1,28 @@
> +// PR c++/94799 - member template function lookup fails.
> +
> +template <typename T>
> +struct A {
> +    int a() {
> +        return 42;
> +    }
> +
> +    template<typename> struct X { typedef int type; };
> +};
> +
> +template <typename T>
> +struct B {
> +    int b(A<T> *p) {
> +	int i = 0;
> +        i += p->a();
> +        i += p->template A<T>::a();
> +        i += p->template A<T>::template A<T>::a();
> +	i += A<T>().template A<T>::a();
> +	return i;
> +    }
> +};
> +
> +int main() {
> +    A<int> a;
> +    B<int> b;
> +    return b.b(&a);
> +}
> diff --git a/gcc/testsuite/g++.dg/template/lookup14.C b/gcc/testsuite/g++.dg/template/lookup14.C
> new file mode 100644
> index 00000000000..e1c945a6dca
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/template/lookup14.C
> @@ -0,0 +1,11 @@
> +// PR c++/94799 - member template function lookup fails.
> +
> +template<typename T>
> +struct A { };
> +
> +template<typename T>
> +void fn (A<T> a)
> +{
> +  // Don't perform name lookup of foo when parsing this template.
> +  a.template A<T>::foo ();
> +}
> 
> base-commit: 19667c82e479dc2bf8351588ed57aff90220b748
>