* [PATCH] c++: Member template function lookup failure [PR94799]
@ 2020-04-29 3:55 Marek Polacek
2020-04-29 21:10 ` Jason Merrill
0 siblings, 1 reply; 10+ messages in thread
From: Marek Polacek @ 2020-04-29 3:55 UTC (permalink / raw)
To: Jason Merrill, GCC Patches
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);
/* 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
&& 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
--
Marek Polacek • Red Hat, Inc. • 300 A St, Boston, MA
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] c++: Member template function lookup failure [PR94799]
2020-04-29 3:55 [PATCH] c++: Member template function lookup failure [PR94799] Marek Polacek
@ 2020-04-29 21:10 ` Jason Merrill
2020-04-30 23:48 ` [PATCH v2] " Marek Polacek
0 siblings, 1 reply; 10+ messages in thread
From: Jason Merrill @ 2020-04-29 21:10 UTC (permalink / raw)
To: Marek Polacek, GCC Patches
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
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] c++: Member template function lookup failure [PR94799]
2020-04-29 21:10 ` Jason Merrill
@ 2020-04-30 23:48 ` Marek Polacek
2020-05-01 20:12 ` Jason Merrill
0 siblings, 1 reply; 10+ messages in thread
From: Marek Polacek @ 2020-04-30 23:48 UTC (permalink / raw)
To: Jason Merrill; +Cc: GCC Patches
On Wed, Apr 29, 2020 at 05:10:44PM -0400, Jason Merrill via Gcc-patches wrote:
> 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.
I've reworked this part to use decltype; curiously it seems to work just fine.
If we use decltype, tsubst/TYPENAME_TYPE will take care of it. I had to play
some games with dereferencing the result in the -> case.
lookup15.C tests this magic decltype. Thanks for the tip!
Now that the unknown_type_node thing is gone, I've dropped reseting object_type
in cp_parser_lookup_name. I considered changing the if to dependent_scope
(object_type) but removing it didn't seem to break anything, and we should
probably handle type-dependent scopes now.
> > /* 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.
This is now gone.
Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
-- >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 decltype (possibly dereferenced) to carry its type. This decltype
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 decltype.
(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 | 35 ++++++++++++++++++------
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, 116 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 e1f9786893a..79a90713221 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -7694,6 +7694,7 @@ 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)
@@ -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,
+ 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);
+ }
+ else if (token_type == CPP_DEREF && POINTER_TYPE_P (type))
+ type = TREE_TYPE (type);
+ parser->context->object_type = type;
+ }
/* Assume this expression is not a pseudo-destructor access. */
pseudo_destructor_p = false;
@@ -23625,8 +23639,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 +28361,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: 66ec22b0d3feb96049283abe5c6c9a05ecef8b86
--
Marek Polacek • Red Hat, Inc. • 300 A St, Boston, MA
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] c++: Member template function lookup failure [PR94799]
2020-04-30 23:48 ` [PATCH v2] " Marek Polacek
@ 2020-05-01 20:12 ` Jason Merrill
2020-05-04 20:37 ` [PATCH v3] " Marek Polacek
0 siblings, 1 reply; 10+ messages in thread
From: Jason Merrill @ 2020-05-01 20:12 UTC (permalink / raw)
To: Marek Polacek; +Cc: GCC Patches
On 4/30/20 7:48 PM, Marek Polacek wrote:
> On Wed, Apr 29, 2020 at 05:10:44PM -0400, Jason Merrill via Gcc-patches wrote:
>> 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.
>
> I've reworked this part to use decltype; curiously it seems to work just fine.
> If we use decltype, tsubst/TYPENAME_TYPE will take care of it. I had to play
> some games with dereferencing the result in the -> case.
>
> lookup15.C tests this magic decltype. Thanks for the tip!
>
> Now that the unknown_type_node thing is gone, I've dropped reseting object_type
> in cp_parser_lookup_name. I considered changing the if to dependent_scope
> (object_type) but removing it didn't seem to break anything, and we should
> probably handle type-dependent scopes now.
>
>>> /* 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.
>
> This is now gone.
>
> Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
>
> -- >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 decltype (possibly dereferenced) to carry its type. This decltype
> 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 decltype.
> (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 | 35 ++++++++++++++++++------
> 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, 116 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 e1f9786893a..79a90713221 100644
> --- a/gcc/cp/parser.c
> +++ b/gcc/cp/parser.c
> @@ -7694,6 +7694,7 @@ 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)
> @@ -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.
> + 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?
> + }
> + else if (token_type == CPP_DEREF && POINTER_TYPE_P (type))
> + type = TREE_TYPE (type);
> + parser->context->object_type = type;
> + }
>
> /* Assume this expression is not a pseudo-destructor access. */
> pseudo_destructor_p = false;
> @@ -23625,8 +23639,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 +28361,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: 66ec22b0d3feb96049283abe5c6c9a05ecef8b86
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3] c++: Member template function lookup failure [PR94799]
2020-05-01 20:12 ` Jason Merrill
@ 2020-05-04 20:37 ` Marek Polacek
2020-05-04 21:41 ` Jason Merrill
0 siblings, 1 reply; 10+ messages in thread
From: Marek Polacek @ 2020-05-04 20:37 UTC (permalink / raw)
To: Jason Merrill; +Cc: GCC Patches
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, and also set DECLTYPE_FOR_LAMBDA_CAPTURE so that the result of the
decltype is e.g. M<T>, not M<T> &. Not sure how hacky this is, but I've
verified that tsubst/TYPENAME_TYPE now gets what I would expect it to.
Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
-- >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 decltype (possibly dereferenced) to carry its type. This decltype
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 decltype.
(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 | 39 ++++++++++++++++++------
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, 120 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..673f3622125 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -7694,6 +7694,7 @@ 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 orig_expr = postfix_expression;
/* If this is a `->' operator, dereference the pointer. */
if (token_type == CPP_DEREF)
@@ -7754,9 +7755,26 @@ 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 (orig_expr);
+ /* If we don't have a (type-dependent) object of class type, use
+ decltype to figure out the type of the object. Use the original
+ expression -- decltype on an ARROW_EXPR may not give the desirable
+ result. */
+ if (type == NULL_TREE)
+ {
+ /* For -> this decltype will produce T*, but we want T. */
+ if (token_type == CPP_DEREF)
+ orig_expr = build_min_nt_loc (start_loc, INDIRECT_REF, orig_expr);
+ type = finish_decltype_type (orig_expr, /*member_access_p=*/false,
+ tf_warning_or_error);
+ /* We don't want the result to be of reference type. */
+ DECLTYPE_FOR_LAMBDA_CAPTURE (type) = true;
+ }
+ else if (token_type == CPP_DEREF && POINTER_TYPE_P (type))
+ type = TREE_TYPE (type);
+ parser->context->object_type = type;
+ }
/* Assume this expression is not a pseudo-destructor access. */
pseudo_destructor_p = false;
@@ -23625,8 +23643,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 +28365,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: e6b31fc717207565f144aaefa608344789221f07
--
Marek Polacek • Red Hat, Inc. • 300 A St, Boston, MA
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3] c++: Member template function lookup failure [PR94799]
2020-05-04 20:37 ` [PATCH v3] " Marek Polacek
@ 2020-05-04 21:41 ` Jason Merrill
2020-05-05 1:51 ` [PATCH v4] " Marek Polacek
0 siblings, 1 reply; 10+ messages in thread
From: Jason Merrill @ 2020-05-04 21:41 UTC (permalink / raw)
To: Marek Polacek; +Cc: GCC Patches
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.
> and also set DECLTYPE_FOR_LAMBDA_CAPTURE so that the result of the
> decltype is e.g. M<T>, not M<T> &. Not sure how hacky this is, but I've
> verified that tsubst/TYPENAME_TYPE now gets what I would expect it to.
Hmm, does it work to use finish_typeof instead? If not, using that flag
is fine, but we should rename it. :)
> Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
>
> -- >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 decltype (possibly dereferenced) to carry its type. This decltype
> 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 decltype.
> (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 | 39 ++++++++++++++++++------
> 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, 120 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..673f3622125 100644
> --- a/gcc/cp/parser.c
> +++ b/gcc/cp/parser.c
> @@ -7694,6 +7694,7 @@ 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 orig_expr = postfix_expression;
>
> /* If this is a `->' operator, dereference the pointer. */
> if (token_type == CPP_DEREF)
> @@ -7754,9 +7755,26 @@ 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 (orig_expr);
> + /* If we don't have a (type-dependent) object of class type, use
> + decltype to figure out the type of the object. Use the original
> + expression -- decltype on an ARROW_EXPR may not give the desirable
> + result. */
> + if (type == NULL_TREE)
> + {
> + /* For -> this decltype will produce T*, but we want T. */
> + if (token_type == CPP_DEREF)
> + orig_expr = build_min_nt_loc (start_loc, INDIRECT_REF, orig_expr);
> + type = finish_decltype_type (orig_expr, /*member_access_p=*/false,
> + tf_warning_or_error);
> + /* We don't want the result to be of reference type. */
> + DECLTYPE_FOR_LAMBDA_CAPTURE (type) = true;
> + }
> + else if (token_type == CPP_DEREF && POINTER_TYPE_P (type))
> + type = TREE_TYPE (type);
> + parser->context->object_type = type;
> + }
>
> /* Assume this expression is not a pseudo-destructor access. */
> pseudo_destructor_p = false;
> @@ -23625,8 +23643,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 +28365,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: e6b31fc717207565f144aaefa608344789221f07
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v4] c++: Member template function lookup failure [PR94799]
2020-05-04 21:41 ` Jason Merrill
@ 2020-05-05 1:51 ` Marek Polacek
2020-05-05 5:01 ` Jason Merrill
0 siblings, 1 reply; 10+ messages in thread
From: Marek Polacek @ 2020-05-05 1:51 UTC (permalink / raw)
To: Jason Merrill; +Cc: GCC Patches
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> *.
> > and also set DECLTYPE_FOR_LAMBDA_CAPTURE so that the result of the
> > decltype is e.g. M<T>, not M<T> &. Not sure how hacky this is, but I've
> > verified that tsubst/TYPENAME_TYPE now gets what I would expect it to.
>
> Hmm, does it work to use finish_typeof instead? If not, using that flag is
> fine, but we should rename it. :)
Ah, very cool, typeof works too, and doesn't need the reference stripping.
Thanks,
Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk/10?
-- >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 (possibly dereferenced) 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 | 36 ++++++++++++++++++------
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, 117 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..90fc9304ef2 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -7694,6 +7694,7 @@ 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 orig_expr = postfix_expression;
/* If this is a `->' operator, dereference the pointer. */
if (token_type == CPP_DEREF)
@@ -7754,9 +7755,23 @@ 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 (orig_expr);
+ /* If we don't have a (type-dependent) object of class type, use
+ typeof to figure out the type of the object. Use the original
+ expression -- typeof on an ARROW_EXPR may not give the desirable
+ result. */
+ if (type == NULL_TREE)
+ {
+ /* For -> this typeof will produce T*, but we want T. */
+ if (token_type == CPP_DEREF)
+ orig_expr = build_min_nt_loc (start_loc, INDIRECT_REF, orig_expr);
+ type = finish_typeof (orig_expr);
+ }
+ else if (token_type == CPP_DEREF && POINTER_TYPE_P (type))
+ type = TREE_TYPE (type);
+ parser->context->object_type = type;
+ }
/* Assume this expression is not a pseudo-destructor access. */
pseudo_destructor_p = false;
@@ -23625,8 +23640,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 +28362,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
--
Marek Polacek • Red Hat, Inc. • 300 A St, Boston, MA
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v4] c++: Member template function lookup failure [PR94799]
2020-05-05 1:51 ` [PATCH v4] " Marek Polacek
@ 2020-05-05 5:01 ` Jason Merrill
2020-05-05 13:36 ` [PATCH v5] " Marek Polacek
0 siblings, 1 reply; 10+ messages in thread
From: Jason Merrill @ 2020-05-05 5:01 UTC (permalink / raw)
To: Marek Polacek; +Cc: GCC Patches
[-- Attachment #1: Type: text/plain, Size: 2028 bytes --]
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:
Jason
[-- Attachment #2: arrow.diff --]
[-- Type: text/x-patch, Size: 1516 bytes --]
commit 042929a2220ff6e2f54c23cd27f360fc285363e3
Author: Jason Merrill <jason@redhat.com>
Date: Mon May 4 22:57:55 2020 -0400
simpler
diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index 90fc9304ef2..5832025443d 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -7694,7 +7694,6 @@ 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 orig_expr = postfix_expression;
/* If this is a `->' operator, dereference the pointer. */
if (token_type == CPP_DEREF)
@@ -7756,20 +7755,11 @@ cp_parser_postfix_dot_deref_expression (cp_parser *parser,
if (dependent_p)
{
- tree type = TREE_TYPE (orig_expr);
+ 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. Use the original
- expression -- typeof on an ARROW_EXPR may not give the desirable
- result. */
+ typeof to figure out the type of the object. */
if (type == NULL_TREE)
- {
- /* For -> this typeof will produce T*, but we want T. */
- if (token_type == CPP_DEREF)
- orig_expr = build_min_nt_loc (start_loc, INDIRECT_REF, orig_expr);
- type = finish_typeof (orig_expr);
- }
- else if (token_type == CPP_DEREF && POINTER_TYPE_P (type))
- type = TREE_TYPE (type);
+ type = finish_typeof (postfix_expression);
parser->context->object_type = type;
}
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v5] c++: Member template function lookup failure [PR94799]
2020-05-05 5:01 ` Jason Merrill
@ 2020-05-05 13:36 ` Marek Polacek
2020-05-05 14:16 ` Jason Merrill
0 siblings, 1 reply; 10+ messages in thread
From: Marek Polacek @ 2020-05-05 13:36 UTC (permalink / raw)
To: Jason Merrill; +Cc: GCC Patches
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?
-- >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
--
Marek Polacek • Red Hat, Inc. • 300 A St, Boston, MA
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v5] c++: Member template function lookup failure [PR94799]
2020-05-05 13:36 ` [PATCH v5] " Marek Polacek
@ 2020-05-05 14:16 ` Jason Merrill
0 siblings, 0 replies; 10+ messages in thread
From: Jason Merrill @ 2020-05-05 14:16 UTC (permalink / raw)
To: Marek Polacek; +Cc: GCC Patches
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
>
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2020-05-05 14:16 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-29 3:55 [PATCH] c++: Member template function lookup failure [PR94799] Marek Polacek
2020-04-29 21:10 ` Jason Merrill
2020-04-30 23:48 ` [PATCH v2] " Marek Polacek
2020-05-01 20:12 ` Jason Merrill
2020-05-04 20:37 ` [PATCH v3] " Marek Polacek
2020-05-04 21:41 ` Jason Merrill
2020-05-05 1:51 ` [PATCH v4] " Marek Polacek
2020-05-05 5:01 ` Jason Merrill
2020-05-05 13:36 ` [PATCH v5] " Marek Polacek
2020-05-05 14:16 ` Jason Merrill
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).