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

Jason Merrill jason@redhat.com
Tue May 5 14:16:05 GMT 2020


On 5/5/20 9:36 AM, Marek Polacek wrote:
> On Tue, May 05, 2020 at 01:01:00AM -0400, Jason Merrill wrote:
>> On 5/4/20 9:51 PM, Marek Polacek wrote:
>>> On Mon, May 04, 2020 at 05:41:37PM -0400, Jason Merrill via Gcc-patches wrote:
>>>> On 5/4/20 4:37 PM, Marek Polacek wrote:
>>>>> On Fri, May 01, 2020 at 04:12:35PM -0400, Jason Merrill wrote:
>>>>>>> @@ -7754,9 +7755,22 @@ 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
>>>>>>> +	 decltype to signal that there was an object.  */
>>>>>>> +      if (type == NULL_TREE)
>>>>>>> +	{
>>>>>>> +	  type = finish_decltype_type (postfix_expression,
>>>>>>> +				       /*member_access_p=*/true,
>>>>>>
>>>>>> This should be false, we don't want the special decltype semantics for a
>>>>>> member-access expression.
>>>>>
>>>>> Fixed.
>>>>>
>>>>>>> +				       tf_warning_or_error);
>>>>>>> +	  /* For -> this decltype will produce T*, but we want T.  */
>>>>>>> +	  if (token_type == CPP_DEREF)
>>>>>>> +	    type = build_min_nt_loc (start_loc, INDIRECT_REF, type);
>>>>>>
>>>>>> Don't we want the INDIRECT_REF inside the decltype?  How does it work like
>>>>>> this?
>>>>>
>>>>> I'm now not quite sure what I perpetrated there; I must've messed it up when I was
>>>>> looking at what we do with it in the debugger.  I assume it worked by accident.
>>>>>
>>>>> I've moved the INDIRECT_REF inside the decltype, but I had to use the original
>>>>> expression
>>>>
>>>> Hmm, I would expect the type of the ARROW_EXPR to be what we want without
>>>> messing with INDIRECT_REF separately.
>>>
>>> Weirdly decltype/typeof of the ARROW_EXPR for e.g. bar((T)1)->template M<T>::fn ()
>>> from the lookup15.C test still gives me M<T> *.
>>
>> In general, if something seems weird, please investigate more before working
>> around it.
>>
>> In this case I can't reproduce the weirdness; your new tests all still pass
>> for me with the patch below:
> 
> And I've verified in gdb that in tsubst I'm seeing what I would expect
> there.  Argh.  It beats me why I kept seeing the error.  Sorry :(
> 
> Anyway, bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk and 10.2?

OK, thanks.

> -- >8 --
> 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.  If the postfix expression doesn't have a type,
> use typeof to carry its type.  This typeof will be processed in
> tsubst/TYPENAME_TYPE.
> 
> 	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.  If the postfix expression doesn't have
> 	a type, use typeof.
> 	(cp_parser_class_name): Consider object scope too.
> 	(cp_parser_lookup_name): Remove code dealing with the case when
> 	object_type 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.
> 	* g++.dg/template/lookup15.C: New test.
> ---
>   gcc/cp/parser.c                          | 26 ++++++++++++++--------
>   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 ++++++++++
>   gcc/testsuite/g++.dg/template/lookup15.C | 24 ++++++++++++++++++++
>   6 files changed, 107 insertions(+), 10 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
>   create mode 100644 gcc/testsuite/g++.dg/template/lookup15.C
> 
> diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
> index 337f22d2784..5832025443d 100644
> --- a/gcc/cp/parser.c
> +++ b/gcc/cp/parser.c
> @@ -7754,9 +7754,14 @@ 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;
> +    {
> +      tree type = TREE_TYPE (postfix_expression);
> +      /* If we don't have a (type-dependent) object of class type, use
> +	 typeof to figure out the type of the object.  */
> +      if (type == NULL_TREE)
> +	type = finish_typeof (postfix_expression);
> +      parser->context->object_type = type;
> +    }
>   
>     /* Assume this expression is not a pseudo-destructor access.  */
>     pseudo_destructor_p = false;
> @@ -23625,8 +23630,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;
>   
> @@ -28340,10 +28352,6 @@ cp_parser_lookup_name (cp_parser *parser, tree name,
>   	decl = lookup_name_real (name, prefer_type_arg (tag_type, is_template),
>   				 /*nonclass=*/0,
>   				 /*block_p=*/true, is_namespace, 0);
> -      if (object_type == unknown_type_node)
> -	/* The object is type-dependent, so we can't look anything up; we used
> -	   this to get the DR 141 behavior.  */
> -	object_type = NULL_TREE;
>         parser->object_scope = object_type;
>         parser->qualifying_scope = NULL_TREE;
>       }
> 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 ();
> +}
> diff --git a/gcc/testsuite/g++.dg/template/lookup15.C b/gcc/testsuite/g++.dg/template/lookup15.C
> new file mode 100644
> index 00000000000..c7f3ba01576
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/template/lookup15.C
> @@ -0,0 +1,24 @@
> +// PR c++/94799 - member template function lookup fails.
> +
> +template<typename>
> +struct M { void fn() { } };
> +
> +M<int>* bar (int);
> +M<int> bar2 (int);
> +
> +template<typename T>
> +struct X : M<T> {
> +  void xfn ()
> +  {
> +    this->template M<T>::fn ();
> +    bar((T)1)->template M<T>::fn ();
> +    bar2((T)1).template M<T>::fn ();
> +  }
> +};
> +
> +int
> +main ()
> +{
> +  X<int> x;
> +  x.xfn();
> +}
> 
> base-commit: ba84e01d81b135594e63a2a830194862b6e358bc
> 



More information about the Gcc-patches mailing list