public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Marek Polacek <polacek@redhat.com>
To: Jason Merrill <jason@redhat.com>, GCC Patches <gcc-patches@gcc.gnu.org>
Subject: [PATCH] c++: Member template function lookup failure [PR94799]
Date: Tue, 28 Apr 2020 23:55:49 -0400	[thread overview]
Message-ID: <20200429035549.1981957-1-polacek@redhat.com> (raw)

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


             reply	other threads:[~2020-04-29  3:55 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-04-29  3:55 Marek Polacek [this message]
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
2020-10-20  0:52 [PATCH] " Marek Polacek
2020-10-28 19:19 ` Jason Merrill

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200429035549.1981957-1-polacek@redhat.com \
    --to=polacek@redhat.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=jason@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).