Re: [PATCH] c++: TYPENAME_TYPE lookup ignoring non-types [PR107773]

2022-11-28 Thread Patrick Palka via Gcc-patches
On Mon, 28 Nov 2022, Patrick Palka wrote:

> [temp.res.general]/3 says, in a note, "the usual qualified name lookup
> ([basic.lookup.qual]) applies even in the presence of typename".  Thus
> when resolving a TYPENAME_TYPE, it seems we shouldn't be looking past
> non-type members.
> 
> This patch fixes this by passing want_type=false instead of =true during
> the member lookup from make_typename_type.  An old nearby comment
> mentions that we want to continue to set want_type=true when resolving a
> nested typename type, but it appears that the nested case is handled by
> resolve_typename_type instead (which passes want_type=true appropriately).

Whoops, it seems this isn't true -- not all nested TYPENAME_TYPEs are
handled by resolve_typename_type, e.g. for T::b in

  struct a {
struct b { typedef void get; };
int b;
  };

  template
  void f() {
typedef typename T::b::get type;
  }

  template void f();

Passing want_type=false in make_typename_type causes us to incorrectly
reject the TYPENAME_TYPE for T::b here because qualified lookup now
finds the data member a::b instead of the nested class of the same name.
So it looks like we need a flag to control whether we're dealing with a
nested TYPENAME_TYPE or not and to pass want_type=true/false appropriately,
I'll poke more tomorrow.

> 
> In passing, use lookup_member instead of lookup_field so that we give a
> better diagnostic when a member function is found, and generalize the T
> format specifier to D in the diagnostic.
> 
> Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for
> trunk?
> 
>   PR c++/107773
> 
> gcc/cp/ChangeLog:
> 
>   * decl.cc (make_typename_type): Use lookup_member instead of
>   lookup_field.  Pass want_type=false instead of =true.  Use D
>   instead of T format specifier.
>   * search.cc (lookup_member): Document default argument.
> 
> gcc/testsuite/ChangeLog:
> 
>   * g++.dg/template/typename24.C: New test.
>   * g++.dg/template/typename25.C: New test.
> ---
>  gcc/cp/decl.cc |  7 +++
>  gcc/cp/search.cc   |  2 +-
>  gcc/testsuite/g++.dg/template/typename24.C | 16 
>  gcc/testsuite/g++.dg/template/typename25.C | 20 
>  4 files changed, 40 insertions(+), 5 deletions(-)
>  create mode 100644 gcc/testsuite/g++.dg/template/typename24.C
>  create mode 100644 gcc/testsuite/g++.dg/template/typename25.C
> 
> diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc
> index 238e72f90da..673e10801a6 100644
> --- a/gcc/cp/decl.cc
> +++ b/gcc/cp/decl.cc
> @@ -4303,9 +4303,8 @@ make_typename_type (tree context, tree name, enum 
> tag_types tag_type,
>   member of the current instantiation or a non-dependent base;
>   lookup will stop when we hit a dependent base.  */
>if (!dependent_scope_p (context))
> -/* We should only set WANT_TYPE when we're a nested typename type.
> -   Then we can give better diagnostics if we find a non-type.  */
> -t = lookup_field (context, name, 2, /*want_type=*/true);
> +t = lookup_member (context, name, /*protect=*/2, /*want_type=*/false,
> +complain);
>else
>  t = NULL_TREE;
>  
> @@ -4357,7 +4356,7 @@ make_typename_type (tree context, tree name, enum 
> tag_types tag_type,
>else
>   {
> if (complain & tf_error)
> - error ("% names %q#T, which is not a type",
> + error ("% names %q#D, which is not a type",
>  context, name, t);
> return error_mark_node;
>   }
> diff --git a/gcc/cp/search.cc b/gcc/cp/search.cc
> index 0dbb3be1ee7..e5848ebc620 100644
> --- a/gcc/cp/search.cc
> +++ b/gcc/cp/search.cc
> @@ -1109,7 +1109,7 @@ build_baselink (tree binfo, tree access_binfo, tree 
> functions, tree optype)
>  
>  tree
>  lookup_member (tree xbasetype, tree name, int protect, bool want_type,
> -tsubst_flags_t complain, access_failure_info *afi)
> +tsubst_flags_t complain, access_failure_info *afi /* = NULL */)
>  {
>tree rval, rval_binfo = NULL_TREE;
>tree type = NULL_TREE, basetype_path = NULL_TREE;
> diff --git a/gcc/testsuite/g++.dg/template/typename24.C 
> b/gcc/testsuite/g++.dg/template/typename24.C
> new file mode 100644
> index 000..4b1d5e5271b
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/template/typename24.C
> @@ -0,0 +1,16 @@
> +// PR c++/107773
> +
> +struct a {
> +  typedef void get;
> +};
> +
> +struct b : a {
> +  int get(int i) const;
> +};
> +
> +template
> +void f() {
> +  typedef typename T::get type; // { dg-error "'int b::get\\(int\\) const', 
> which is not a type" }
> +}
> +
> +template void f();
> diff --git a/gcc/testsuite/g++.dg/template/typename25.C 
> b/gcc/testsuite/g++.dg/template/typename25.C
> new file mode 100644
> index 000..4e6b764a97b
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/template/typename25.C
> @@ -0,0 +1,20 @@
> +// Example 4 from [temp.res.general]/3.
> +
> +struct A {
> +  

[PATCH] c++: TYPENAME_TYPE lookup ignoring non-types [PR107773]

2022-11-28 Thread Patrick Palka via Gcc-patches
[temp.res.general]/3 says, in a note, "the usual qualified name lookup
([basic.lookup.qual]) applies even in the presence of typename".  Thus
when resolving a TYPENAME_TYPE, it seems we shouldn't be looking past
non-type members.

This patch fixes this by passing want_type=false instead of =true during
the member lookup from make_typename_type.  An old nearby comment
mentions that we want to continue to set want_type=true when resolving a
nested typename type, but it appears that the nested case is handled by
resolve_typename_type instead (which passes want_type=true appropriately).

In passing, use lookup_member instead of lookup_field so that we give a
better diagnostic when a member function is found, and generalize the T
format specifier to D in the diagnostic.

Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for
trunk?

PR c++/107773

gcc/cp/ChangeLog:

* decl.cc (make_typename_type): Use lookup_member instead of
lookup_field.  Pass want_type=false instead of =true.  Use D
instead of T format specifier.
* search.cc (lookup_member): Document default argument.

gcc/testsuite/ChangeLog:

* g++.dg/template/typename24.C: New test.
* g++.dg/template/typename25.C: New test.
---
 gcc/cp/decl.cc |  7 +++
 gcc/cp/search.cc   |  2 +-
 gcc/testsuite/g++.dg/template/typename24.C | 16 
 gcc/testsuite/g++.dg/template/typename25.C | 20 
 4 files changed, 40 insertions(+), 5 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/template/typename24.C
 create mode 100644 gcc/testsuite/g++.dg/template/typename25.C

diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc
index 238e72f90da..673e10801a6 100644
--- a/gcc/cp/decl.cc
+++ b/gcc/cp/decl.cc
@@ -4303,9 +4303,8 @@ make_typename_type (tree context, tree name, enum 
tag_types tag_type,
  member of the current instantiation or a non-dependent base;
  lookup will stop when we hit a dependent base.  */
   if (!dependent_scope_p (context))
-/* We should only set WANT_TYPE when we're a nested typename type.
-   Then we can give better diagnostics if we find a non-type.  */
-t = lookup_field (context, name, 2, /*want_type=*/true);
+t = lookup_member (context, name, /*protect=*/2, /*want_type=*/false,
+  complain);
   else
 t = NULL_TREE;
 
@@ -4357,7 +4356,7 @@ make_typename_type (tree context, tree name, enum 
tag_types tag_type,
   else
{
  if (complain & tf_error)
-   error ("% names %q#T, which is not a type",
+   error ("% names %q#D, which is not a type",
   context, name, t);
  return error_mark_node;
}
diff --git a/gcc/cp/search.cc b/gcc/cp/search.cc
index 0dbb3be1ee7..e5848ebc620 100644
--- a/gcc/cp/search.cc
+++ b/gcc/cp/search.cc
@@ -1109,7 +1109,7 @@ build_baselink (tree binfo, tree access_binfo, tree 
functions, tree optype)
 
 tree
 lookup_member (tree xbasetype, tree name, int protect, bool want_type,
-  tsubst_flags_t complain, access_failure_info *afi)
+  tsubst_flags_t complain, access_failure_info *afi /* = NULL */)
 {
   tree rval, rval_binfo = NULL_TREE;
   tree type = NULL_TREE, basetype_path = NULL_TREE;
diff --git a/gcc/testsuite/g++.dg/template/typename24.C 
b/gcc/testsuite/g++.dg/template/typename24.C
new file mode 100644
index 000..4b1d5e5271b
--- /dev/null
+++ b/gcc/testsuite/g++.dg/template/typename24.C
@@ -0,0 +1,16 @@
+// PR c++/107773
+
+struct a {
+  typedef void get;
+};
+
+struct b : a {
+  int get(int i) const;
+};
+
+template
+void f() {
+  typedef typename T::get type; // { dg-error "'int b::get\\(int\\) const', 
which is not a type" }
+}
+
+template void f();
diff --git a/gcc/testsuite/g++.dg/template/typename25.C 
b/gcc/testsuite/g++.dg/template/typename25.C
new file mode 100644
index 000..4e6b764a97b
--- /dev/null
+++ b/gcc/testsuite/g++.dg/template/typename25.C
@@ -0,0 +1,20 @@
+// Example 4 from [temp.res.general]/3.
+
+struct A {
+  struct X { };
+  int X;
+};
+struct B {
+  struct X { };
+};
+template void f(T t) {
+  typename T::X x; // { dg-error "'int A::X', which is not a type" }
+}
+void foo() {
+  A a;
+  B b;
+  f(b); // OK, T::X refers to B::X
+  // { dg-bogus "" "" { target *-*-* } .-1 }
+  f(a); // error: T::X refers to the data member A::X not the struct A::X
+  // { dg-message "required from here" "" { target *-*-* } .-1 }
+}
-- 
2.39.0.rc0.33.g815c1e8202