[PATCH 1/2] c++: Fix push_access_scope and introduce RAII wrapper for it

[PATCH 1/2] c++: Fix push_access_scope and introduce RAII wrapper for it

[Patrick Palka]

When push_access_scope is passed a TYPE_DECL for a class type (which
can happen during e.g. satisfaction), we undesirably push only the
enclosing context of the class instead of the class itself.  This causes
us to mishandle e.g. testcase below due to us not entering the scope of
A before checking its constraints.

This patch adjusts push_access_scope accordingly, and introduces an
RAII wrapper for it.  We also make use of this wrapper right away by
replacing the only use of push_nested_class_guard with this new wrapper,
which means we can remove this old wrapper (whose functionality is
basically subsumed by the new wrapper).

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

gcc/cp/ChangeLog:

	* constraint.cc (get_normalized_constraints_from_decl): Use
	push_access_scope_guard instead of push_nested_class_guard.
	* cp-tree.h (struct push_nested_class_guard): Replace with ...
	(struct push_access_scope_guard): ... this.
	* pt.c (push_access_scope): When the argument corresponds to
	a class type, push the class instead of its context.
	(pop_access_scope): Adjust accordingly.

gcc/testsuite/ChangeLog:

	* g++.dg/cpp2a/concepts-access2.C: New test.
---
 gcc/cp/constraint.cc                          |  7 +-----
 gcc/cp/cp-tree.h                              | 23 +++++++++++--------
 gcc/cp/pt.c                                   |  9 +++++++-
 gcc/testsuite/g++.dg/cpp2a/concepts-access2.C | 13 +++++++++++
 4 files changed, 35 insertions(+), 17 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-access2.C

diff --git a/gcc/cp/constraint.cc b/gcc/cp/constraint.cc
index 6df3ca6ce32..99d3ccc6998 100644
--- a/gcc/cp/constraint.cc
+++ b/gcc/cp/constraint.cc
@@ -926,12 +926,7 @@ get_normalized_constraints_from_decl (tree d, bool diag = false)
   tree norm = NULL_TREE;
   if (tree ci = get_constraints (decl))
     {
-      push_nested_class_guard pncs (DECL_CONTEXT (d));
-
-      temp_override<tree> ovr (current_function_decl);
-      if (TREE_CODE (decl) == FUNCTION_DECL)
-	current_function_decl = decl;
-
+      push_access_scope_guard pas (decl);
       norm = get_normalized_constraints_from_info (ci, tmpl, diag);
     }
 
diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index 6f713719589..58da7460001 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -8463,21 +8463,24 @@ is_constrained_auto (const_tree t)
   return is_auto (t) && PLACEHOLDER_TYPE_CONSTRAINTS_INFO (t);
 }
 
-/* RAII class to push/pop class scope T; if T is not a class, do nothing.  */
+/* RAII class to push/pop the access scope for T.  */
 
-struct push_nested_class_guard
+struct push_access_scope_guard
 {
-  bool push;
-  push_nested_class_guard (tree t)
-    : push (t && CLASS_TYPE_P (t))
+  tree decl;
+  push_access_scope_guard (tree t)
+    : decl (t)
   {
-    if (push)
-      push_nested_class (t);
+    if (VAR_OR_FUNCTION_DECL_P (decl)
+	|| TREE_CODE (decl) == TYPE_DECL)
+      push_access_scope (decl);
+    else
+      decl = NULL_TREE;
   }
-  ~push_nested_class_guard ()
+  ~push_access_scope_guard ()
   {
-    if (push)
-      pop_nested_class ();
+    if (decl)
+      pop_access_scope (decl);
   }
 };
 
diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index f2039e09cd7..bd8b17ca047 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -224,7 +224,7 @@ static void instantiate_body (tree pattern, tree args, tree d, bool nested);
 /* Make the current scope suitable for access checking when we are
    processing T.  T can be FUNCTION_DECL for instantiated function
    template, VAR_DECL for static member variable, or TYPE_DECL for
-   alias template (needed by instantiate_decl).  */
+   for a class or alias template (needed by instantiate_decl).  */
 
 void
 push_access_scope (tree t)
@@ -234,6 +234,10 @@ push_access_scope (tree t)
 
   if (DECL_FRIEND_CONTEXT (t))
     push_nested_class (DECL_FRIEND_CONTEXT (t));
+  else if (TREE_CODE (t) == TYPE_DECL
+	   && CLASS_TYPE_P (TREE_TYPE (t))
+	   && DECL_ORIGINAL_TYPE (t) == NULL_TREE)
+    push_nested_class (TREE_TYPE (t));
   else if (DECL_CLASS_SCOPE_P (t))
     push_nested_class (DECL_CONTEXT (t));
   else if (deduction_guide_p (t) && DECL_ARTIFICIAL (t))
@@ -260,6 +264,9 @@ pop_access_scope (tree t)
     current_function_decl = saved_access_scope->pop();
 
   if (DECL_FRIEND_CONTEXT (t)
+      || (TREE_CODE (t) == TYPE_DECL
+	  && CLASS_TYPE_P (TREE_TYPE (t))
+	  && DECL_ORIGINAL_TYPE (t) == NULL_TREE)
       || DECL_CLASS_SCOPE_P (t)
       || (deduction_guide_p (t) && DECL_ARTIFICIAL (t)))
     pop_nested_class ();
diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-access2.C b/gcc/testsuite/g++.dg/cpp2a/concepts-access2.C
new file mode 100644
index 00000000000..8ddcad236e3
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp2a/concepts-access2.C
@@ -0,0 +1,13 @@
+// { dg-do compile { target concepts } }
+
+template<class T> requires T::value struct A { };
+template<class T> requires T::value struct B { }; // { dg-error "private" }
+
+struct S {
+private:
+  static constexpr bool value = true;
+  template<class T> requires T::value friend struct A;
+};
+
+A<S> x;
+B<S> y; // { dg-error "constraint" }
-- 
2.32.0.93.g670b81a890

[PATCH 2/2] c++: Extend PR96204 fix to variable templates

[Patrick Palka]

r12-1829 corrected the access scope during partial specialization
matching of class templates, but neglected the variable template case.
This patch moves the access scope adjustment to inside
most_specialized_partial_spec, so that all callers can benefit.

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

	PR c++/96204

gcc/cp/ChangeLog:

	* pt.c (instantiate_class_template_1): Remove call to
	push_nested_class and pop_nested_class added by r12-1829.
	(most_specialized_partial_spec): Use push_access_scope_guard
	and deferring_access_check_sentinel.

gcc/testsuite/ChangeLog:

	* g++.dg/template/access40b.C: New test.
---
 gcc/cp/pt.c                               | 12 +++++++----
 gcc/testsuite/g++.dg/template/access40b.C | 26 +++++++++++++++++++++++
 2 files changed, 34 insertions(+), 4 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/template/access40b.C

diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index bd8b17ca047..1e2e2ba5329 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -11776,11 +11776,8 @@ instantiate_class_template_1 (tree type)
   deferring_access_check_sentinel acs (dk_no_deferred);
 
   /* Determine what specialization of the original template to
-     instantiate; do this relative to the scope of the class for
-     sake of access checking.  */
-  push_nested_class (type);
+     instantiate.  */
   t = most_specialized_partial_spec (type, tf_warning_or_error);
-  pop_nested_class ();
   if (t == error_mark_node)
     return error_mark_node;
   else if (t)
@@ -24989,26 +24986,33 @@ most_specialized_partial_spec (tree target, tsubst_flags_t complain)
   tree outer_args = NULL_TREE;
   tree tmpl, args;
 
+  tree decl;
   if (TYPE_P (target))
     {
       tree tinfo = CLASSTYPE_TEMPLATE_INFO (target);
       tmpl = TI_TEMPLATE (tinfo);
       args = TI_ARGS (tinfo);
+      decl = TYPE_NAME (target);
     }
   else if (TREE_CODE (target) == TEMPLATE_ID_EXPR)
     {
       tmpl = TREE_OPERAND (target, 0);
       args = TREE_OPERAND (target, 1);
+      decl = DECL_TEMPLATE_RESULT (tmpl);
     }
   else if (VAR_P (target))
     {
       tree tinfo = DECL_TEMPLATE_INFO (target);
       tmpl = TI_TEMPLATE (tinfo);
       args = TI_ARGS (tinfo);
+      decl = target;
     }
   else
     gcc_unreachable ();
 
+  push_access_scope_guard pas (decl);
+  deferring_access_check_sentinel acs (dk_no_deferred);
+
   tree main_tmpl = most_general_template (tmpl);
 
   /* For determining which partial specialization to use, only the
diff --git a/gcc/testsuite/g++.dg/template/access40b.C b/gcc/testsuite/g++.dg/template/access40b.C
new file mode 100644
index 00000000000..040e3d18096
--- /dev/null
+++ b/gcc/testsuite/g++.dg/template/access40b.C
@@ -0,0 +1,26 @@
+// PR c++/96204
+// { dg-do compile { target c++14 } }
+// A variant of access40.C where has_type_member is a variable template instead
+// of a class template.
+
+template<class, class = void>
+constexpr bool has_type_member = false;
+
+template<class T>
+constexpr bool has_type_member<T, typename T::type> = true;
+
+struct Parent;
+
+struct Child {
+private:
+  friend struct Parent;
+  typedef void type;
+};
+
+struct Parent {
+  static void f() {
+    // The partial specialization does not match despite Child::type
+    // being accessible from the current scope.
+    static_assert(!has_type_member<Child>, "");
+  }
+};
-- 
2.32.0.93.g670b81a890

Re: [PATCH 2/2] c++: Extend PR96204 fix to variable templates

[Jason Merrill]

[-- Attachment #1: Type: text/plain, Size: 4740 bytes --]

On 6/29/21 1:57 PM, Patrick Palka wrote:
> r12-1829 corrected the access scope during partial specialization
> matching of class templates, but neglected the variable template case.
> This patch moves the access scope adjustment to inside
> most_specialized_partial_spec, so that all callers can benefit.
> 
> Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for
> trunk?
> 
> 	PR c++/96204
> 
> gcc/cp/ChangeLog:
> 
> 	* pt.c (instantiate_class_template_1): Remove call to
> 	push_nested_class and pop_nested_class added by r12-1829.
> 	(most_specialized_partial_spec): Use push_access_scope_guard
> 	and deferring_access_check_sentinel.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* g++.dg/template/access40b.C: New test.
> ---
>   gcc/cp/pt.c                               | 12 +++++++----
>   gcc/testsuite/g++.dg/template/access40b.C | 26 +++++++++++++++++++++++
>   2 files changed, 34 insertions(+), 4 deletions(-)
>   create mode 100644 gcc/testsuite/g++.dg/template/access40b.C
> 
> diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
> index bd8b17ca047..1e2e2ba5329 100644
> --- a/gcc/cp/pt.c
> +++ b/gcc/cp/pt.c
> @@ -11776,11 +11776,8 @@ instantiate_class_template_1 (tree type)
>     deferring_access_check_sentinel acs (dk_no_deferred);
>   
>     /* Determine what specialization of the original template to
> -     instantiate; do this relative to the scope of the class for
> -     sake of access checking.  */
> -  push_nested_class (type);
> +     instantiate.  */
>     t = most_specialized_partial_spec (type, tf_warning_or_error);
> -  pop_nested_class ();
>     if (t == error_mark_node)
>       return error_mark_node;
>     else if (t)
> @@ -24989,26 +24986,33 @@ most_specialized_partial_spec (tree target, tsubst_flags_t complain)
>     tree outer_args = NULL_TREE;
>     tree tmpl, args;
>   
> +  tree decl;
>     if (TYPE_P (target))
>       {
>         tree tinfo = CLASSTYPE_TEMPLATE_INFO (target);
>         tmpl = TI_TEMPLATE (tinfo);
>         args = TI_ARGS (tinfo);
> +      decl = TYPE_NAME (target);
>       }
>     else if (TREE_CODE (target) == TEMPLATE_ID_EXPR)
>       {
>         tmpl = TREE_OPERAND (target, 0);
>         args = TREE_OPERAND (target, 1);
> +      decl = DECL_TEMPLATE_RESULT (tmpl);

Hmm, this won't get us the right scope; we get here for the result of 
finish_template_variable, where tmpl is the most general template and 
args are args for it.  So in the below testcase, tmpl is outer<T>::N<T2,U>:

template <typename T> struct outer {
   template <typename T2, typename U>
   static constexpr int f() { return N<T,int>; };

   template <typename T2, typename U>
   static const int N = f<T2, U>();
};

template <typename T>
template <typename U>
const int outer<T>::N<T,U> = 1;

int i = outer<int>::N<double,int>;

Oddly, I notice that we also get here for static data members of class 
templates that are not themselves templates, as in mem-partial1.C that I 
adapted the above from.  Fixed by the attached patch.

Since the type of the variable depends on the specialization, we can't 
actually get the decl before doing the resolution, but we should be able 
to push into the right enclosing class.  Perhaps we should pass the 
partially instantiated template and its args to lookup_template_variable 
instead of the most general template and its args.

>       }
>     else if (VAR_P (target))
>       {
>         tree tinfo = DECL_TEMPLATE_INFO (target);
>         tmpl = TI_TEMPLATE (tinfo);
>         args = TI_ARGS (tinfo);
> +      decl = target;
>       }
>     else
>       gcc_unreachable ();
>   
> +  push_access_scope_guard pas (decl);
> +  deferring_access_check_sentinel acs (dk_no_deferred);
> +
>     tree main_tmpl = most_general_template (tmpl);
>   
>     /* For determining which partial specialization to use, only the
> diff --git a/gcc/testsuite/g++.dg/template/access40b.C b/gcc/testsuite/g++.dg/template/access40b.C
> new file mode 100644
> index 00000000000..040e3d18096
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/template/access40b.C
> @@ -0,0 +1,26 @@
> +// PR c++/96204
> +// { dg-do compile { target c++14 } }
> +// A variant of access40.C where has_type_member is a variable template instead
> +// of a class template.
> +
> +template<class, class = void>
> +constexpr bool has_type_member = false;
> +
> +template<class T>
> +constexpr bool has_type_member<T, typename T::type> = true;
> +
> +struct Parent;
> +
> +struct Child {
> +private:
> +  friend struct Parent;
> +  typedef void type;
> +};
> +
> +struct Parent {
> +  static void f() {
> +    // The partial specialization does not match despite Child::type
> +    // being accessible from the current scope.
> +    static_assert(!has_type_member<Child>, "");
> +  }
> +};
> 


[-- Attachment #2: 0001-vartmp.patch --]
[-- Type: text/x-patch, Size: 747 bytes --]

From b7b34f555b54f97a9d2315d6c6a552e27e2faa9c Mon Sep 17 00:00:00 2001
From: Jason Merrill <jason@redhat.com>
Date: Tue, 29 Jun 2021 15:11:25 -0400
Subject: [PATCH] vartmp
To: gcc-patches@gcc.gnu.org

---
 gcc/cp/pt.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index f2039e09cd7..d2936c106ba 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -26003,7 +26003,7 @@ instantiate_decl (tree d, bool defer_ok, bool expl_inst_class_mem_p)
   td = template_for_substitution (d);
   args = gen_args;
 
-  if (VAR_P (d))
+  if (variable_template_specialization_p (d))
     {
       /* Look up an explicit specialization, if any.  */
       tree tid = lookup_template_variable (gen_tmpl, gen_args);
-- 
2.27.0

Re: [PATCH 2/2] c++: Extend PR96204 fix to variable templates

[Patrick Palka]

On Tue, 29 Jun 2021, Jason Merrill wrote:

> On 6/29/21 1:57 PM, Patrick Palka wrote:
> > r12-1829 corrected the access scope during partial specialization
> > matching of class templates, but neglected the variable template case.
> > This patch moves the access scope adjustment to inside
> > most_specialized_partial_spec, so that all callers can benefit.
> > 
> > Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for
> > trunk?
> > 
> > 	PR c++/96204
> > 
> > gcc/cp/ChangeLog:
> > 
> > 	* pt.c (instantiate_class_template_1): Remove call to
> > 	push_nested_class and pop_nested_class added by r12-1829.
> > 	(most_specialized_partial_spec): Use push_access_scope_guard
> > 	and deferring_access_check_sentinel.
> > 
> > gcc/testsuite/ChangeLog:
> > 
> > 	* g++.dg/template/access40b.C: New test.
> > ---
> >   gcc/cp/pt.c                               | 12 +++++++----
> >   gcc/testsuite/g++.dg/template/access40b.C | 26 +++++++++++++++++++++++
> >   2 files changed, 34 insertions(+), 4 deletions(-)
> >   create mode 100644 gcc/testsuite/g++.dg/template/access40b.C
> > 
> > diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
> > index bd8b17ca047..1e2e2ba5329 100644
> > --- a/gcc/cp/pt.c
> > +++ b/gcc/cp/pt.c
> > @@ -11776,11 +11776,8 @@ instantiate_class_template_1 (tree type)
> >     deferring_access_check_sentinel acs (dk_no_deferred);
> >       /* Determine what specialization of the original template to
> > -     instantiate; do this relative to the scope of the class for
> > -     sake of access checking.  */
> > -  push_nested_class (type);
> > +     instantiate.  */
> >     t = most_specialized_partial_spec (type, tf_warning_or_error);
> > -  pop_nested_class ();
> >     if (t == error_mark_node)
> >       return error_mark_node;
> >     else if (t)
> > @@ -24989,26 +24986,33 @@ most_specialized_partial_spec (tree target,
> > tsubst_flags_t complain)
> >     tree outer_args = NULL_TREE;
> >     tree tmpl, args;
> >   +  tree decl;
> >     if (TYPE_P (target))
> >       {
> >         tree tinfo = CLASSTYPE_TEMPLATE_INFO (target);
> >         tmpl = TI_TEMPLATE (tinfo);
> >         args = TI_ARGS (tinfo);
> > +      decl = TYPE_NAME (target);
> >       }
> >     else if (TREE_CODE (target) == TEMPLATE_ID_EXPR)
> >       {
> >         tmpl = TREE_OPERAND (target, 0);
> >         args = TREE_OPERAND (target, 1);
> > +      decl = DECL_TEMPLATE_RESULT (tmpl);
> 
> Hmm, this won't get us the right scope; we get here for the result of
> finish_template_variable, where tmpl is the most general template and args are
> args for it.  So in the below testcase, tmpl is outer<T>::N<T2,U>:
> 
> template <typename T> struct outer {
>   template <typename T2, typename U>
>   static constexpr int f() { return N<T,int>; };
> 
>   template <typename T2, typename U>
>   static const int N = f<T2, U>();
> };
> 
> template <typename T>
> template <typename U>
> const int outer<T>::N<T,U> = 1;
> 
> int i = outer<int>::N<double,int>;
> 
> Oddly, I notice that we also get here for static data members of class
> templates that are not themselves templates, as in mem-partial1.C that I
> adapted the above from.  Fixed by the attached patch.

Makes sense.  I was wondering if the VAR_P (pattern) test in
instantiate_template_1 should be adjusted as well, but that doesn't seem
to be strictly necessary since a VAR_DECL there will always be a
variable template specialization.

> 
> Since the type of the variable depends on the specialization, we can't
> actually get the decl before doing the resolution, but we should be able to
> push into the right enclosing class.  Perhaps we should pass the partially
> instantiated template and its args to lookup_template_variable instead of the
> most general template and its args.
> 

It seems what works is passing the partially instantiated template and
the full set of args to lookup_template_variable, because the outer
args of the partial specialization may be dependent as in e.g. the
above testcase.  One would hope that 'tmpl' contains the partially
instantiated template, but that's not the case because
finish_template_variable passes only the most general template
to us.  So we need to adjust finish_template_variable to pass the
partially instantiated template instead.

And instantiate_decl needs an adjustment as well, since it too calls
most_specialized_partial_spec.  Here, we could just pass the VAR_DECL
'd' to most_specialized_partial_spec, which'll set up the right
context for us.

How does the following look?  Passes make check-c++, full testing in
progress.  The addded second testcase below should adequately test all
this IIUC..

-- >8 --

	PR c++/96204

gcc/cp/ChangeLog:

	* pt.c (finish_template_variable): Pass the partially
	instantiated template and args to instantiate_template.
	(instantiate_class_template_1): No need to call
	push_nested_class and pop_nested_class around the call to
	most_specialized_partial_spec.
	(instantiate_template_1): Pass the partially instantiated
	template to lookup_template_variable.
	(most_specialized_partial_spec):  Use push_access_scope_guard
	to set the access scope appropriately.  Use
	deferring_access_check_sentinel to disable deferment of access
	checking.
	(instantiate_decl): Just pass the VAR_DECL to
	most_specialized_partial_spec.

gcc/testsuite/ChangeLog:

	* g++.dg/template/access41.C: New test.
	* g++.dg/template/access41a.C: New test.
---
 gcc/cp/pt.c                               | 29 ++++++++++-----------
 gcc/testsuite/g++.dg/template/access41.C  | 24 ++++++++++++++++++
 gcc/testsuite/g++.dg/template/access41a.C | 31 +++++++++++++++++++++++
 3 files changed, 69 insertions(+), 15 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/template/access41.C
 create mode 100644 gcc/testsuite/g++.dg/template/access41a.C

diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index d66ae134580..bed41402801 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -10266,14 +10266,10 @@ finish_template_variable (tree var, tsubst_flags_t complain)
   tree templ = TREE_OPERAND (var, 0);
   tree arglist = TREE_OPERAND (var, 1);
 
-  tree tmpl_args = DECL_TI_ARGS (DECL_TEMPLATE_RESULT (templ));
-  arglist = add_outermost_template_args (tmpl_args, arglist);
-
-  templ = most_general_template (templ);
-  tree parms = DECL_TEMPLATE_PARMS (templ);
-  arglist = coerce_innermost_template_parms (parms, arglist, templ, complain,
-					     /*req_all*/true,
-					     /*use_default*/true);
+  tree parms = DECL_INNERMOST_TEMPLATE_PARMS (templ);
+  arglist = coerce_template_parms (parms, arglist, templ, complain,
+				   /*req_all*/true,
+				   /*use_default*/true);
   if (arglist == error_mark_node)
     return error_mark_node;
 
@@ -11772,11 +11768,8 @@ instantiate_class_template_1 (tree type)
   deferring_access_check_sentinel acs (dk_no_deferred);
 
   /* Determine what specialization of the original template to
-     instantiate; do this relative to the scope of the class for
-     sake of access checking.  */
-  push_nested_class (type);
+     instantiate.  */
   t = most_specialized_partial_spec (type, tf_warning_or_error);
-  pop_nested_class ();
   if (t == error_mark_node)
     return error_mark_node;
   else if (t)
@@ -21132,7 +21125,7 @@ instantiate_template_1 (tree tmpl, tree orig_args, tsubst_flags_t complain)
       /* We need to determine if we're using a partial or explicit
 	 specialization now, because the type of the variable could be
 	 different.  */
-      tree tid = lookup_template_variable (gen_tmpl, targ_ptr);
+      tree tid = lookup_template_variable (tmpl, targ_ptr);
       tree elt = most_specialized_partial_spec (tid, complain);
       if (elt == error_mark_node)
 	pattern = error_mark_node;
@@ -24985,26 +24978,33 @@ most_specialized_partial_spec (tree target, tsubst_flags_t complain)
   tree outer_args = NULL_TREE;
   tree tmpl, args;
 
+  tree decl;
   if (TYPE_P (target))
     {
       tree tinfo = CLASSTYPE_TEMPLATE_INFO (target);
       tmpl = TI_TEMPLATE (tinfo);
       args = TI_ARGS (tinfo);
+      decl = TYPE_NAME (target);
     }
   else if (TREE_CODE (target) == TEMPLATE_ID_EXPR)
     {
       tmpl = TREE_OPERAND (target, 0);
       args = TREE_OPERAND (target, 1);
+      decl = DECL_TEMPLATE_RESULT (tmpl);
     }
   else if (VAR_P (target))
     {
       tree tinfo = DECL_TEMPLATE_INFO (target);
       tmpl = TI_TEMPLATE (tinfo);
       args = TI_ARGS (tinfo);
+      decl = target;
     }
   else
     gcc_unreachable ();
 
+  push_access_scope_guard pas (decl);
+  deferring_access_check_sentinel acs (dk_no_deferred);
+
   tree main_tmpl = most_general_template (tmpl);
 
   /* For determining which partial specialization to use, only the
@@ -26009,8 +26009,7 @@ instantiate_decl (tree d, bool defer_ok, bool expl_inst_class_mem_p)
   if (variable_template_specialization_p (d))
     {
       /* Look up an explicit specialization, if any.  */
-      tree tid = lookup_template_variable (gen_tmpl, gen_args);
-      tree elt = most_specialized_partial_spec (tid, tf_warning_or_error);
+      tree elt = most_specialized_partial_spec (d, tf_warning_or_error);
       if (elt && elt != error_mark_node)
 	{
 	  td = TREE_VALUE (elt);
diff --git a/gcc/testsuite/g++.dg/template/access41.C b/gcc/testsuite/g++.dg/template/access41.C
new file mode 100644
index 00000000000..1ab9a1ab243
--- /dev/null
+++ b/gcc/testsuite/g++.dg/template/access41.C
@@ -0,0 +1,24 @@
+// PR c++/96204
+// { dg-do compile { target c++14 } }
+// A variant of access40.C where has_type_member is a variable template instead
+// of a class template.
+
+template<class, class = void>
+constexpr bool has_type_member = false;
+
+template<class T>
+constexpr bool has_type_member<T, typename T::type> = true;
+
+struct Parent;
+
+struct Child {
+private:
+  friend struct Parent;
+  typedef void type;
+};
+
+struct Parent {
+  // The partial specialization does not match despite Child::type
+  // being accessible from the current scope.
+  static_assert(!has_type_member<Child>, "");
+};
diff --git a/gcc/testsuite/g++.dg/template/access41a.C b/gcc/testsuite/g++.dg/template/access41a.C
new file mode 100644
index 00000000000..e3608e2dffc
--- /dev/null
+++ b/gcc/testsuite/g++.dg/template/access41a.C
@@ -0,0 +1,31 @@
+// PR c++/96204
+// { dg-do compile { target c++14 } }
+// A variant of access40a.C where has_type_member is a variable template instead
+// of a class template.
+
+template<class T>
+struct A {
+  template<class, class = void>
+  static constexpr bool has_type_member = false;
+};
+
+template<class T>
+template<class U>
+constexpr int A<T>::has_type_member<U, typename U::type> = true;
+
+struct Parent;
+
+struct Child {
+private:
+  friend struct A<int>;
+  typedef void type;
+};
+
+// The partial specialization matches because A<int> is a friend of Child.
+static_assert(A<int>::has_type_member<Child>, "");
+using type1 = const int;
+using type1 = decltype(A<int>::has_type_member<Child>);
+
+static_assert(!A<char>::has_type_member<Child>, "");
+using type2 = const bool;
+using type2 = decltype(A<char>::has_type_member<Child>);
-- 
2.32.0.93.g670b81a890

Re: [PATCH 2/2] c++: Extend PR96204 fix to variable templates

[Jason Merrill]

On 6/30/21 10:48 AM, Patrick Palka wrote:
> On Tue, 29 Jun 2021, Jason Merrill wrote:
> 
>> On 6/29/21 1:57 PM, Patrick Palka wrote:
>>> r12-1829 corrected the access scope during partial specialization
>>> matching of class templates, but neglected the variable template case.
>>> This patch moves the access scope adjustment to inside
>>> most_specialized_partial_spec, so that all callers can benefit.
>>>
>>> Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for
>>> trunk?
>>>
>>> 	PR c++/96204
>>>
>>> gcc/cp/ChangeLog:
>>>
>>> 	* pt.c (instantiate_class_template_1): Remove call to
>>> 	push_nested_class and pop_nested_class added by r12-1829.
>>> 	(most_specialized_partial_spec): Use push_access_scope_guard
>>> 	and deferring_access_check_sentinel.
>>>
>>> gcc/testsuite/ChangeLog:
>>>
>>> 	* g++.dg/template/access40b.C: New test.
>>> ---
>>>    gcc/cp/pt.c                               | 12 +++++++----
>>>    gcc/testsuite/g++.dg/template/access40b.C | 26 +++++++++++++++++++++++
>>>    2 files changed, 34 insertions(+), 4 deletions(-)
>>>    create mode 100644 gcc/testsuite/g++.dg/template/access40b.C
>>>
>>> diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
>>> index bd8b17ca047..1e2e2ba5329 100644
>>> --- a/gcc/cp/pt.c
>>> +++ b/gcc/cp/pt.c
>>> @@ -11776,11 +11776,8 @@ instantiate_class_template_1 (tree type)
>>>      deferring_access_check_sentinel acs (dk_no_deferred);
>>>        /* Determine what specialization of the original template to
>>> -     instantiate; do this relative to the scope of the class for
>>> -     sake of access checking.  */
>>> -  push_nested_class (type);
>>> +     instantiate.  */
>>>      t = most_specialized_partial_spec (type, tf_warning_or_error);
>>> -  pop_nested_class ();
>>>      if (t == error_mark_node)
>>>        return error_mark_node;
>>>      else if (t)
>>> @@ -24989,26 +24986,33 @@ most_specialized_partial_spec (tree target,
>>> tsubst_flags_t complain)
>>>      tree outer_args = NULL_TREE;
>>>      tree tmpl, args;
>>>    +  tree decl;
>>>      if (TYPE_P (target))
>>>        {
>>>          tree tinfo = CLASSTYPE_TEMPLATE_INFO (target);
>>>          tmpl = TI_TEMPLATE (tinfo);
>>>          args = TI_ARGS (tinfo);
>>> +      decl = TYPE_NAME (target);
>>>        }
>>>      else if (TREE_CODE (target) == TEMPLATE_ID_EXPR)
>>>        {
>>>          tmpl = TREE_OPERAND (target, 0);
>>>          args = TREE_OPERAND (target, 1);
>>> +      decl = DECL_TEMPLATE_RESULT (tmpl);
>>
>> Hmm, this won't get us the right scope; we get here for the result of
>> finish_template_variable, where tmpl is the most general template and args are
>> args for it.  So in the below testcase, tmpl is outer<T>::N<T2,U>:
>>
>> template <typename T> struct outer {
>>    template <typename T2, typename U>
>>    static constexpr int f() { return N<T,int>; };
>>
>>    template <typename T2, typename U>
>>    static const int N = f<T2, U>();
>> };
>>
>> template <typename T>
>> template <typename U>
>> const int outer<T>::N<T,U> = 1;
>>
>> int i = outer<int>::N<double,int>;
>>
>> Oddly, I notice that we also get here for static data members of class
>> templates that are not themselves templates, as in mem-partial1.C that I
>> adapted the above from.  Fixed by the attached patch.
> 
> Makes sense.  I was wondering if the VAR_P (pattern) test in
> instantiate_template_1 should be adjusted as well, but that doesn't seem
> to be strictly necessary since a VAR_DECL there will always be a
> variable template specialization.
> 
>>
>> Since the type of the variable depends on the specialization, we can't
>> actually get the decl before doing the resolution, but we should be able to
>> push into the right enclosing class.  Perhaps we should pass the partially
>> instantiated template and its args to lookup_template_variable instead of the
>> most general template and its args.
>>
> 
> It seems what works is passing the partially instantiated template and
> the full set of args to lookup_template_variable, because the outer
> args of the partial specialization may be dependent as in e.g. the
> above testcase.  One would hope that 'tmpl' contains the partially
> instantiated template, but that's not the case because
> finish_template_variable passes only the most general template
> to us.  So we need to adjust finish_template_variable to pass the
> partially instantiated template instead.
> 
> And instantiate_decl needs an adjustment as well, since it too calls
> most_specialized_partial_spec.  Here, we could just pass the VAR_DECL
> 'd' to most_specialized_partial_spec, which'll set up the right
> context for us.
> 
> How does the following look?  Passes make check-c++, full testing in
> progress.  The addded second testcase below should adequately test all
> this IIUC..
> 
> -- >8 --
> 
> 	PR c++/96204
> 
> gcc/cp/ChangeLog:
> 
> 	* pt.c (finish_template_variable): Pass the partially
> 	instantiated template and args to instantiate_template.
> 	(instantiate_class_template_1): No need to call
> 	push_nested_class and pop_nested_class around the call to
> 	most_specialized_partial_spec.
> 	(instantiate_template_1): Pass the partially instantiated
> 	template to lookup_template_variable.
> 	(most_specialized_partial_spec):  Use push_access_scope_guard
> 	to set the access scope appropriately.  Use
> 	deferring_access_check_sentinel to disable deferment of access
> 	checking.
> 	(instantiate_decl): Just pass the VAR_DECL to
> 	most_specialized_partial_spec.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* g++.dg/template/access41.C: New test.
> 	* g++.dg/template/access41a.C: New test.
> ---
>   gcc/cp/pt.c                               | 29 ++++++++++-----------
>   gcc/testsuite/g++.dg/template/access41.C  | 24 ++++++++++++++++++
>   gcc/testsuite/g++.dg/template/access41a.C | 31 +++++++++++++++++++++++
>   3 files changed, 69 insertions(+), 15 deletions(-)
>   create mode 100644 gcc/testsuite/g++.dg/template/access41.C
>   create mode 100644 gcc/testsuite/g++.dg/template/access41a.C
> 
> diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
> index d66ae134580..bed41402801 100644
> --- a/gcc/cp/pt.c
> +++ b/gcc/cp/pt.c
> @@ -10266,14 +10266,10 @@ finish_template_variable (tree var, tsubst_flags_t complain)
>     tree templ = TREE_OPERAND (var, 0);
>     tree arglist = TREE_OPERAND (var, 1);
>   
> -  tree tmpl_args = DECL_TI_ARGS (DECL_TEMPLATE_RESULT (templ));
> -  arglist = add_outermost_template_args (tmpl_args, arglist);
> -
> -  templ = most_general_template (templ);
> -  tree parms = DECL_TEMPLATE_PARMS (templ);
> -  arglist = coerce_innermost_template_parms (parms, arglist, templ, complain,
> -					     /*req_all*/true,
> -					     /*use_default*/true);
> +  tree parms = DECL_INNERMOST_TEMPLATE_PARMS (templ);
> +  arglist = coerce_template_parms (parms, arglist, templ, complain,
> +				   /*req_all*/true,
> +				   /*use_default*/true);

Is the change from DECL_TEMPLATE_PARMS/coerce_innermost to 
DECL_INNERMOST_TEMPLATE_PARMS/coerce_template_parms necessary?  I'd 
expect them to have the same effect.

OK either way.

>     if (arglist == error_mark_node)
>       return error_mark_node;
>   
> @@ -11772,11 +11768,8 @@ instantiate_class_template_1 (tree type)
>     deferring_access_check_sentinel acs (dk_no_deferred);
>   
>     /* Determine what specialization of the original template to
> -     instantiate; do this relative to the scope of the class for
> -     sake of access checking.  */
> -  push_nested_class (type);
> +     instantiate.  */
>     t = most_specialized_partial_spec (type, tf_warning_or_error);
> -  pop_nested_class ();
>     if (t == error_mark_node)
>       return error_mark_node;
>     else if (t)
> @@ -21132,7 +21125,7 @@ instantiate_template_1 (tree tmpl, tree orig_args, tsubst_flags_t complain)
>         /* We need to determine if we're using a partial or explicit
>   	 specialization now, because the type of the variable could be
>   	 different.  */
> -      tree tid = lookup_template_variable (gen_tmpl, targ_ptr);
> +      tree tid = lookup_template_variable (tmpl, targ_ptr);
>         tree elt = most_specialized_partial_spec (tid, complain);
>         if (elt == error_mark_node)
>   	pattern = error_mark_node;
> @@ -24985,26 +24978,33 @@ most_specialized_partial_spec (tree target, tsubst_flags_t complain)
>     tree outer_args = NULL_TREE;
>     tree tmpl, args;
>   
> +  tree decl;
>     if (TYPE_P (target))
>       {
>         tree tinfo = CLASSTYPE_TEMPLATE_INFO (target);
>         tmpl = TI_TEMPLATE (tinfo);
>         args = TI_ARGS (tinfo);
> +      decl = TYPE_NAME (target);
>       }
>     else if (TREE_CODE (target) == TEMPLATE_ID_EXPR)
>       {
>         tmpl = TREE_OPERAND (target, 0);
>         args = TREE_OPERAND (target, 1);
> +      decl = DECL_TEMPLATE_RESULT (tmpl);
>       }
>     else if (VAR_P (target))
>       {
>         tree tinfo = DECL_TEMPLATE_INFO (target);
>         tmpl = TI_TEMPLATE (tinfo);
>         args = TI_ARGS (tinfo);
> +      decl = target;
>       }
>     else
>       gcc_unreachable ();
>   
> +  push_access_scope_guard pas (decl);
> +  deferring_access_check_sentinel acs (dk_no_deferred);
> +
>     tree main_tmpl = most_general_template (tmpl);
>   
>     /* For determining which partial specialization to use, only the
> @@ -26009,8 +26009,7 @@ instantiate_decl (tree d, bool defer_ok, bool expl_inst_class_mem_p)
>     if (variable_template_specialization_p (d))
>       {
>         /* Look up an explicit specialization, if any.  */
> -      tree tid = lookup_template_variable (gen_tmpl, gen_args);
> -      tree elt = most_specialized_partial_spec (tid, tf_warning_or_error);
> +      tree elt = most_specialized_partial_spec (d, tf_warning_or_error);
>         if (elt && elt != error_mark_node)
>   	{
>   	  td = TREE_VALUE (elt);
> diff --git a/gcc/testsuite/g++.dg/template/access41.C b/gcc/testsuite/g++.dg/template/access41.C
> new file mode 100644
> index 00000000000..1ab9a1ab243
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/template/access41.C
> @@ -0,0 +1,24 @@
> +// PR c++/96204
> +// { dg-do compile { target c++14 } }
> +// A variant of access40.C where has_type_member is a variable template instead
> +// of a class template.
> +
> +template<class, class = void>
> +constexpr bool has_type_member = false;
> +
> +template<class T>
> +constexpr bool has_type_member<T, typename T::type> = true;
> +
> +struct Parent;
> +
> +struct Child {
> +private:
> +  friend struct Parent;
> +  typedef void type;
> +};
> +
> +struct Parent {
> +  // The partial specialization does not match despite Child::type
> +  // being accessible from the current scope.
> +  static_assert(!has_type_member<Child>, "");
> +};
> diff --git a/gcc/testsuite/g++.dg/template/access41a.C b/gcc/testsuite/g++.dg/template/access41a.C
> new file mode 100644
> index 00000000000..e3608e2dffc
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/template/access41a.C
> @@ -0,0 +1,31 @@
> +// PR c++/96204
> +// { dg-do compile { target c++14 } }
> +// A variant of access40a.C where has_type_member is a variable template instead
> +// of a class template.
> +
> +template<class T>
> +struct A {
> +  template<class, class = void>
> +  static constexpr bool has_type_member = false;
> +};
> +
> +template<class T>
> +template<class U>
> +constexpr int A<T>::has_type_member<U, typename U::type> = true;
> +
> +struct Parent;
> +
> +struct Child {
> +private:
> +  friend struct A<int>;
> +  typedef void type;
> +};
> +
> +// The partial specialization matches because A<int> is a friend of Child.
> +static_assert(A<int>::has_type_member<Child>, "");
> +using type1 = const int;
> +using type1 = decltype(A<int>::has_type_member<Child>);
> +
> +static_assert(!A<char>::has_type_member<Child>, "");
> +using type2 = const bool;
> +using type2 = decltype(A<char>::has_type_member<Child>);
> 

Re: [PATCH 2/2] c++: Extend PR96204 fix to variable templates

[Patrick Palka]

On Wed, 30 Jun 2021, Jason Merrill wrote:

> On 6/30/21 10:48 AM, Patrick Palka wrote:
> > On Tue, 29 Jun 2021, Jason Merrill wrote:
> > 
> > > On 6/29/21 1:57 PM, Patrick Palka wrote:
> > > > r12-1829 corrected the access scope during partial specialization
> > > > matching of class templates, but neglected the variable template case.
> > > > This patch moves the access scope adjustment to inside
> > > > most_specialized_partial_spec, so that all callers can benefit.
> > > > 
> > > > Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for
> > > > trunk?
> > > > 
> > > > 	PR c++/96204
> > > > 
> > > > gcc/cp/ChangeLog:
> > > > 
> > > > 	* pt.c (instantiate_class_template_1): Remove call to
> > > > 	push_nested_class and pop_nested_class added by r12-1829.
> > > > 	(most_specialized_partial_spec): Use push_access_scope_guard
> > > > 	and deferring_access_check_sentinel.
> > > > 
> > > > gcc/testsuite/ChangeLog:
> > > > 
> > > > 	* g++.dg/template/access40b.C: New test.
> > > > ---
> > > >    gcc/cp/pt.c                               | 12 +++++++----
> > > >    gcc/testsuite/g++.dg/template/access40b.C | 26
> > > > +++++++++++++++++++++++
> > > >    2 files changed, 34 insertions(+), 4 deletions(-)
> > > >    create mode 100644 gcc/testsuite/g++.dg/template/access40b.C
> > > > 
> > > > diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
> > > > index bd8b17ca047..1e2e2ba5329 100644
> > > > --- a/gcc/cp/pt.c
> > > > +++ b/gcc/cp/pt.c
> > > > @@ -11776,11 +11776,8 @@ instantiate_class_template_1 (tree type)
> > > >      deferring_access_check_sentinel acs (dk_no_deferred);
> > > >        /* Determine what specialization of the original template to
> > > > -     instantiate; do this relative to the scope of the class for
> > > > -     sake of access checking.  */
> > > > -  push_nested_class (type);
> > > > +     instantiate.  */
> > > >      t = most_specialized_partial_spec (type, tf_warning_or_error);
> > > > -  pop_nested_class ();
> > > >      if (t == error_mark_node)
> > > >        return error_mark_node;
> > > >      else if (t)
> > > > @@ -24989,26 +24986,33 @@ most_specialized_partial_spec (tree target,
> > > > tsubst_flags_t complain)
> > > >      tree outer_args = NULL_TREE;
> > > >      tree tmpl, args;
> > > >    +  tree decl;
> > > >      if (TYPE_P (target))
> > > >        {
> > > >          tree tinfo = CLASSTYPE_TEMPLATE_INFO (target);
> > > >          tmpl = TI_TEMPLATE (tinfo);
> > > >          args = TI_ARGS (tinfo);
> > > > +      decl = TYPE_NAME (target);
> > > >        }
> > > >      else if (TREE_CODE (target) == TEMPLATE_ID_EXPR)
> > > >        {
> > > >          tmpl = TREE_OPERAND (target, 0);
> > > >          args = TREE_OPERAND (target, 1);
> > > > +      decl = DECL_TEMPLATE_RESULT (tmpl);
> > > 
> > > Hmm, this won't get us the right scope; we get here for the result of
> > > finish_template_variable, where tmpl is the most general template and args
> > > are
> > > args for it.  So in the below testcase, tmpl is outer<T>::N<T2,U>:
> > > 
> > > template <typename T> struct outer {
> > >    template <typename T2, typename U>
> > >    static constexpr int f() { return N<T,int>; };
> > > 
> > >    template <typename T2, typename U>
> > >    static const int N = f<T2, U>();
> > > };
> > > 
> > > template <typename T>
> > > template <typename U>
> > > const int outer<T>::N<T,U> = 1;
> > > 
> > > int i = outer<int>::N<double,int>;
> > > 
> > > Oddly, I notice that we also get here for static data members of class
> > > templates that are not themselves templates, as in mem-partial1.C that I
> > > adapted the above from.  Fixed by the attached patch.
> > 
> > Makes sense.  I was wondering if the VAR_P (pattern) test in
> > instantiate_template_1 should be adjusted as well, but that doesn't seem
> > to be strictly necessary since a VAR_DECL there will always be a
> > variable template specialization.
> > 
> > > 
> > > Since the type of the variable depends on the specialization, we can't
> > > actually get the decl before doing the resolution, but we should be able
> > > to
> > > push into the right enclosing class.  Perhaps we should pass the partially
> > > instantiated template and its args to lookup_template_variable instead of
> > > the
> > > most general template and its args.
> > > 
> > 
> > It seems what works is passing the partially instantiated template and
> > the full set of args to lookup_template_variable, because the outer
> > args of the partial specialization may be dependent as in e.g. the
> > above testcase.  One would hope that 'tmpl' contains the partially
> > instantiated template, but that's not the case because
> > finish_template_variable passes only the most general template
> > to us.  So we need to adjust finish_template_variable to pass the
> > partially instantiated template instead.
> > 
> > And instantiate_decl needs an adjustment as well, since it too calls
> > most_specialized_partial_spec.  Here, we could just pass the VAR_DECL
> > 'd' to most_specialized_partial_spec, which'll set up the right
> > context for us.
> > 
> > How does the following look?  Passes make check-c++, full testing in
> > progress.  The addded second testcase below should adequately test all
> > this IIUC..
> > 
> > -- >8 --
> > 
> > 	PR c++/96204
> > 
> > gcc/cp/ChangeLog:
> > 
> > 	* pt.c (finish_template_variable): Pass the partially
> > 	instantiated template and args to instantiate_template.
> > 	(instantiate_class_template_1): No need to call
> > 	push_nested_class and pop_nested_class around the call to
> > 	most_specialized_partial_spec.
> > 	(instantiate_template_1): Pass the partially instantiated
> > 	template to lookup_template_variable.
> > 	(most_specialized_partial_spec):  Use push_access_scope_guard
> > 	to set the access scope appropriately.  Use
> > 	deferring_access_check_sentinel to disable deferment of access
> > 	checking.
> > 	(instantiate_decl): Just pass the VAR_DECL to
> > 	most_specialized_partial_spec.
> > 
> > gcc/testsuite/ChangeLog:
> > 
> > 	* g++.dg/template/access41.C: New test.
> > 	* g++.dg/template/access41a.C: New test.
> > ---
> >   gcc/cp/pt.c                               | 29 ++++++++++-----------
> >   gcc/testsuite/g++.dg/template/access41.C  | 24 ++++++++++++++++++
> >   gcc/testsuite/g++.dg/template/access41a.C | 31 +++++++++++++++++++++++
> >   3 files changed, 69 insertions(+), 15 deletions(-)
> >   create mode 100644 gcc/testsuite/g++.dg/template/access41.C
> >   create mode 100644 gcc/testsuite/g++.dg/template/access41a.C
> > 
> > diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
> > index d66ae134580..bed41402801 100644
> > --- a/gcc/cp/pt.c
> > +++ b/gcc/cp/pt.c
> > @@ -10266,14 +10266,10 @@ finish_template_variable (tree var, tsubst_flags_t
> > complain)
> >     tree templ = TREE_OPERAND (var, 0);
> >     tree arglist = TREE_OPERAND (var, 1);
> >   -  tree tmpl_args = DECL_TI_ARGS (DECL_TEMPLATE_RESULT (templ));
> > -  arglist = add_outermost_template_args (tmpl_args, arglist);
> > -
> > -  templ = most_general_template (templ);
> > -  tree parms = DECL_TEMPLATE_PARMS (templ);
> > -  arglist = coerce_innermost_template_parms (parms, arglist, templ,
> > complain,
> > -					     /*req_all*/true,
> > -					     /*use_default*/true);
> > +  tree parms = DECL_INNERMOST_TEMPLATE_PARMS (templ);
> > +  arglist = coerce_template_parms (parms, arglist, templ, complain,
> > +				   /*req_all*/true,
> > +				   /*use_default*/true);
> 
> Is the change from DECL_TEMPLATE_PARMS/coerce_innermost to
> DECL_INNERMOST_TEMPLATE_PARMS/coerce_template_parms necessary?  I'd expect
> them to have the same effect.
> 
> OK either way.

Ah yeah, the effect seems to be the same.  I thought I ran into a crash
before changing that part of finish_template_parm, but I can't reproduce
that anymore.  I'll change it back to using DECL_TEMPLATE_PARMS /
coerce_innermost_template_parms and commit the patch after another round
of testing.  Thanks a lot!

> 
> >     if (arglist == error_mark_node)
> >       return error_mark_node;
> >   @@ -11772,11 +11768,8 @@ instantiate_class_template_1 (tree type)
> >     deferring_access_check_sentinel acs (dk_no_deferred);
> >       /* Determine what specialization of the original template to
> > -     instantiate; do this relative to the scope of the class for
> > -     sake of access checking.  */
> > -  push_nested_class (type);
> > +     instantiate.  */
> >     t = most_specialized_partial_spec (type, tf_warning_or_error);
> > -  pop_nested_class ();
> >     if (t == error_mark_node)
> >       return error_mark_node;
> >     else if (t)
> > @@ -21132,7 +21125,7 @@ instantiate_template_1 (tree tmpl, tree orig_args,
> > tsubst_flags_t complain)
> >         /* We need to determine if we're using a partial or explicit
> >   	 specialization now, because the type of the variable could be
> >   	 different.  */
> > -      tree tid = lookup_template_variable (gen_tmpl, targ_ptr);
> > +      tree tid = lookup_template_variable (tmpl, targ_ptr);
> >         tree elt = most_specialized_partial_spec (tid, complain);
> >         if (elt == error_mark_node)
> >   	pattern = error_mark_node;
> > @@ -24985,26 +24978,33 @@ most_specialized_partial_spec (tree target,
> > tsubst_flags_t complain)
> >     tree outer_args = NULL_TREE;
> >     tree tmpl, args;
> >   +  tree decl;
> >     if (TYPE_P (target))
> >       {
> >         tree tinfo = CLASSTYPE_TEMPLATE_INFO (target);
> >         tmpl = TI_TEMPLATE (tinfo);
> >         args = TI_ARGS (tinfo);
> > +      decl = TYPE_NAME (target);
> >       }
> >     else if (TREE_CODE (target) == TEMPLATE_ID_EXPR)
> >       {
> >         tmpl = TREE_OPERAND (target, 0);
> >         args = TREE_OPERAND (target, 1);
> > +      decl = DECL_TEMPLATE_RESULT (tmpl);
> >       }
> >     else if (VAR_P (target))
> >       {
> >         tree tinfo = DECL_TEMPLATE_INFO (target);
> >         tmpl = TI_TEMPLATE (tinfo);
> >         args = TI_ARGS (tinfo);
> > +      decl = target;
> >       }
> >     else
> >       gcc_unreachable ();
> >   +  push_access_scope_guard pas (decl);
> > +  deferring_access_check_sentinel acs (dk_no_deferred);
> > +
> >     tree main_tmpl = most_general_template (tmpl);
> >       /* For determining which partial specialization to use, only the
> > @@ -26009,8 +26009,7 @@ instantiate_decl (tree d, bool defer_ok, bool
> > expl_inst_class_mem_p)
> >     if (variable_template_specialization_p (d))
> >       {
> >         /* Look up an explicit specialization, if any.  */
> > -      tree tid = lookup_template_variable (gen_tmpl, gen_args);
> > -      tree elt = most_specialized_partial_spec (tid, tf_warning_or_error);
> > +      tree elt = most_specialized_partial_spec (d, tf_warning_or_error);
> >         if (elt && elt != error_mark_node)
> >   	{
> >   	  td = TREE_VALUE (elt);
> > diff --git a/gcc/testsuite/g++.dg/template/access41.C
> > b/gcc/testsuite/g++.dg/template/access41.C
> > new file mode 100644
> > index 00000000000..1ab9a1ab243
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/template/access41.C
> > @@ -0,0 +1,24 @@
> > +// PR c++/96204
> > +// { dg-do compile { target c++14 } }
> > +// A variant of access40.C where has_type_member is a variable template
> > instead
> > +// of a class template.
> > +
> > +template<class, class = void>
> > +constexpr bool has_type_member = false;
> > +
> > +template<class T>
> > +constexpr bool has_type_member<T, typename T::type> = true;
> > +
> > +struct Parent;
> > +
> > +struct Child {
> > +private:
> > +  friend struct Parent;
> > +  typedef void type;
> > +};
> > +
> > +struct Parent {
> > +  // The partial specialization does not match despite Child::type
> > +  // being accessible from the current scope.
> > +  static_assert(!has_type_member<Child>, "");
> > +};
> > diff --git a/gcc/testsuite/g++.dg/template/access41a.C
> > b/gcc/testsuite/g++.dg/template/access41a.C
> > new file mode 100644
> > index 00000000000..e3608e2dffc
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/template/access41a.C
> > @@ -0,0 +1,31 @@
> > +// PR c++/96204
> > +// { dg-do compile { target c++14 } }
> > +// A variant of access40a.C where has_type_member is a variable template
> > instead
> > +// of a class template.
> > +
> > +template<class T>
> > +struct A {
> > +  template<class, class = void>
> > +  static constexpr bool has_type_member = false;
> > +};
> > +
> > +template<class T>
> > +template<class U>
> > +constexpr int A<T>::has_type_member<U, typename U::type> = true;
> > +
> > +struct Parent;
> > +
> > +struct Child {
> > +private:
> > +  friend struct A<int>;
> > +  typedef void type;
> > +};
> > +
> > +// The partial specialization matches because A<int> is a friend of Child.
> > +static_assert(A<int>::has_type_member<Child>, "");
> > +using type1 = const int;
> > +using type1 = decltype(A<int>::has_type_member<Child>);
> > +
> > +static_assert(!A<char>::has_type_member<Child>, "");
> > +using type2 = const bool;
> > +using type2 = decltype(A<char>::has_type_member<Child>);
> > 
> 
> 

Re: [PATCH 1/2] c++: Fix push_access_scope and introduce RAII wrapper for it

[Jason Merrill]

On 6/29/21 1:57 PM, Patrick Palka wrote:
> When push_access_scope is passed a TYPE_DECL for a class type (which
> can happen during e.g. satisfaction), we undesirably push only the
> enclosing context of the class instead of the class itself.  This causes
> us to mishandle e.g. testcase below due to us not entering the scope of
> A before checking its constraints.
> 
> This patch adjusts push_access_scope accordingly, and introduces an
> RAII wrapper for it.  We also make use of this wrapper right away by
> replacing the only use of push_nested_class_guard with this new wrapper,
> which means we can remove this old wrapper (whose functionality is
> basically subsumed by the new wrapper).
> 
> Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for
> trunk?
> 
> gcc/cp/ChangeLog:
> 
> 	* constraint.cc (get_normalized_constraints_from_decl): Use
> 	push_access_scope_guard instead of push_nested_class_guard.
> 	* cp-tree.h (struct push_nested_class_guard): Replace with ...
> 	(struct push_access_scope_guard): ... this.
> 	* pt.c (push_access_scope): When the argument corresponds to
> 	a class type, push the class instead of its context.
> 	(pop_access_scope): Adjust accordingly.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* g++.dg/cpp2a/concepts-access2.C: New test.
> ---
>   gcc/cp/constraint.cc                          |  7 +-----
>   gcc/cp/cp-tree.h                              | 23 +++++++++++--------
>   gcc/cp/pt.c                                   |  9 +++++++-
>   gcc/testsuite/g++.dg/cpp2a/concepts-access2.C | 13 +++++++++++
>   4 files changed, 35 insertions(+), 17 deletions(-)
>   create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-access2.C
> 
> diff --git a/gcc/cp/constraint.cc b/gcc/cp/constraint.cc
> index 6df3ca6ce32..99d3ccc6998 100644
> --- a/gcc/cp/constraint.cc
> +++ b/gcc/cp/constraint.cc
> @@ -926,12 +926,7 @@ get_normalized_constraints_from_decl (tree d, bool diag = false)
>     tree norm = NULL_TREE;
>     if (tree ci = get_constraints (decl))
>       {
> -      push_nested_class_guard pncs (DECL_CONTEXT (d));
> -
> -      temp_override<tree> ovr (current_function_decl);
> -      if (TREE_CODE (decl) == FUNCTION_DECL)
> -	current_function_decl = decl;
> -
> +      push_access_scope_guard pas (decl);
>         norm = get_normalized_constraints_from_info (ci, tmpl, diag);
>       }
>   
> diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
> index 6f713719589..58da7460001 100644
> --- a/gcc/cp/cp-tree.h
> +++ b/gcc/cp/cp-tree.h
> @@ -8463,21 +8463,24 @@ is_constrained_auto (const_tree t)
>     return is_auto (t) && PLACEHOLDER_TYPE_CONSTRAINTS_INFO (t);
>   }
>   
> -/* RAII class to push/pop class scope T; if T is not a class, do nothing.  */
> +/* RAII class to push/pop the access scope for T.  */
>   
> -struct push_nested_class_guard
> +struct push_access_scope_guard
>   {
> -  bool push;
> -  push_nested_class_guard (tree t)
> -    : push (t && CLASS_TYPE_P (t))
> +  tree decl;
> +  push_access_scope_guard (tree t)
> +    : decl (t)
>     {
> -    if (push)
> -      push_nested_class (t);
> +    if (VAR_OR_FUNCTION_DECL_P (decl)
> +	|| TREE_CODE (decl) == TYPE_DECL)
> +      push_access_scope (decl);
> +    else
> +      decl = NULL_TREE;
>     }
> -  ~push_nested_class_guard ()
> +  ~push_access_scope_guard ()
>     {
> -    if (push)
> -      pop_nested_class ();
> +    if (decl)
> +      pop_access_scope (decl);
>     }
>   };
>   
> diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
> index f2039e09cd7..bd8b17ca047 100644
> --- a/gcc/cp/pt.c
> +++ b/gcc/cp/pt.c
> @@ -224,7 +224,7 @@ static void instantiate_body (tree pattern, tree args, tree d, bool nested);
>   /* Make the current scope suitable for access checking when we are
>      processing T.  T can be FUNCTION_DECL for instantiated function
>      template, VAR_DECL for static member variable, or TYPE_DECL for
> -   alias template (needed by instantiate_decl).  */
> +   for a class or alias template (needed by instantiate_decl).  */
>   
>   void
>   push_access_scope (tree t)
> @@ -234,6 +234,10 @@ push_access_scope (tree t)
>   
>     if (DECL_FRIEND_CONTEXT (t))
>       push_nested_class (DECL_FRIEND_CONTEXT (t));
> +  else if (TREE_CODE (t) == TYPE_DECL
> +	   && CLASS_TYPE_P (TREE_TYPE (t))
> +	   && DECL_ORIGINAL_TYPE (t) == NULL_TREE)

I suspect DECL_IMPLICIT_TYPEDEF_P is a better test for this case.

> +    push_nested_class (TREE_TYPE (t));
>     else if (DECL_CLASS_SCOPE_P (t))
>       push_nested_class (DECL_CONTEXT (t));
>     else if (deduction_guide_p (t) && DECL_ARTIFICIAL (t))
> @@ -260,6 +264,9 @@ pop_access_scope (tree t)
>       current_function_decl = saved_access_scope->pop();
>   
>     if (DECL_FRIEND_CONTEXT (t)
> +      || (TREE_CODE (t) == TYPE_DECL
> +	  && CLASS_TYPE_P (TREE_TYPE (t))
> +	  && DECL_ORIGINAL_TYPE (t) == NULL_TREE)
>         || DECL_CLASS_SCOPE_P (t)
>         || (deduction_guide_p (t) && DECL_ARTIFICIAL (t)))
>       pop_nested_class ();
> diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-access2.C b/gcc/testsuite/g++.dg/cpp2a/concepts-access2.C
> new file mode 100644
> index 00000000000..8ddcad236e3
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp2a/concepts-access2.C
> @@ -0,0 +1,13 @@
> +// { dg-do compile { target concepts } }
> +
> +template<class T> requires T::value struct A { };
> +template<class T> requires T::value struct B { }; // { dg-error "private" }
> +
> +struct S {
> +private:
> +  static constexpr bool value = true;
> +  template<class T> requires T::value friend struct A;
> +};
> +
> +A<S> x;
> +B<S> y; // { dg-error "constraint" }
> 

Re: [PATCH 1/2] c++: Fix push_access_scope and introduce RAII wrapper for it

[Patrick Palka]

On Tue, 29 Jun 2021, Jason Merrill wrote:

> On 6/29/21 1:57 PM, Patrick Palka wrote:
> > When push_access_scope is passed a TYPE_DECL for a class type (which
> > can happen during e.g. satisfaction), we undesirably push only the
> > enclosing context of the class instead of the class itself.  This causes
> > us to mishandle e.g. testcase below due to us not entering the scope of
> > A before checking its constraints.
> > 
> > This patch adjusts push_access_scope accordingly, and introduces an
> > RAII wrapper for it.  We also make use of this wrapper right away by
> > replacing the only use of push_nested_class_guard with this new wrapper,
> > which means we can remove this old wrapper (whose functionality is
> > basically subsumed by the new wrapper).
> > 
> > Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for
> > trunk?
> > 
> > gcc/cp/ChangeLog:
> > 
> > 	* constraint.cc (get_normalized_constraints_from_decl): Use
> > 	push_access_scope_guard instead of push_nested_class_guard.
> > 	* cp-tree.h (struct push_nested_class_guard): Replace with ...
> > 	(struct push_access_scope_guard): ... this.
> > 	* pt.c (push_access_scope): When the argument corresponds to
> > 	a class type, push the class instead of its context.
> > 	(pop_access_scope): Adjust accordingly.
> > 
> > gcc/testsuite/ChangeLog:
> > 
> > 	* g++.dg/cpp2a/concepts-access2.C: New test.
> > ---
> >   gcc/cp/constraint.cc                          |  7 +-----
> >   gcc/cp/cp-tree.h                              | 23 +++++++++++--------
> >   gcc/cp/pt.c                                   |  9 +++++++-
> >   gcc/testsuite/g++.dg/cpp2a/concepts-access2.C | 13 +++++++++++
> >   4 files changed, 35 insertions(+), 17 deletions(-)
> >   create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-access2.C
> > 
> > diff --git a/gcc/cp/constraint.cc b/gcc/cp/constraint.cc
> > index 6df3ca6ce32..99d3ccc6998 100644
> > --- a/gcc/cp/constraint.cc
> > +++ b/gcc/cp/constraint.cc
> > @@ -926,12 +926,7 @@ get_normalized_constraints_from_decl (tree d, bool diag
> > = false)
> >     tree norm = NULL_TREE;
> >     if (tree ci = get_constraints (decl))
> >       {
> > -      push_nested_class_guard pncs (DECL_CONTEXT (d));
> > -
> > -      temp_override<tree> ovr (current_function_decl);
> > -      if (TREE_CODE (decl) == FUNCTION_DECL)
> > -	current_function_decl = decl;
> > -
> > +      push_access_scope_guard pas (decl);
> >         norm = get_normalized_constraints_from_info (ci, tmpl, diag);
> >       }
> >   diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
> > index 6f713719589..58da7460001 100644
> > --- a/gcc/cp/cp-tree.h
> > +++ b/gcc/cp/cp-tree.h
> > @@ -8463,21 +8463,24 @@ is_constrained_auto (const_tree t)
> >     return is_auto (t) && PLACEHOLDER_TYPE_CONSTRAINTS_INFO (t);
> >   }
> >   -/* RAII class to push/pop class scope T; if T is not a class, do nothing.
> > */
> > +/* RAII class to push/pop the access scope for T.  */
> >   -struct push_nested_class_guard
> > +struct push_access_scope_guard
> >   {
> > -  bool push;
> > -  push_nested_class_guard (tree t)
> > -    : push (t && CLASS_TYPE_P (t))
> > +  tree decl;
> > +  push_access_scope_guard (tree t)
> > +    : decl (t)
> >     {
> > -    if (push)
> > -      push_nested_class (t);
> > +    if (VAR_OR_FUNCTION_DECL_P (decl)
> > +	|| TREE_CODE (decl) == TYPE_DECL)
> > +      push_access_scope (decl);
> > +    else
> > +      decl = NULL_TREE;
> >     }
> > -  ~push_nested_class_guard ()
> > +  ~push_access_scope_guard ()
> >     {
> > -    if (push)
> > -      pop_nested_class ();
> > +    if (decl)
> > +      pop_access_scope (decl);
> >     }
> >   };
> >   diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
> > index f2039e09cd7..bd8b17ca047 100644
> > --- a/gcc/cp/pt.c
> > +++ b/gcc/cp/pt.c
> > @@ -224,7 +224,7 @@ static void instantiate_body (tree pattern, tree args,
> > tree d, bool nested);
> >   /* Make the current scope suitable for access checking when we are
> >      processing T.  T can be FUNCTION_DECL for instantiated function
> >      template, VAR_DECL for static member variable, or TYPE_DECL for
> > -   alias template (needed by instantiate_decl).  */
> > +   for a class or alias template (needed by instantiate_decl).  */
> >     void
> >   push_access_scope (tree t)
> > @@ -234,6 +234,10 @@ push_access_scope (tree t)
> >       if (DECL_FRIEND_CONTEXT (t))
> >       push_nested_class (DECL_FRIEND_CONTEXT (t));
> > +  else if (TREE_CODE (t) == TYPE_DECL
> > +	   && CLASS_TYPE_P (TREE_TYPE (t))
> > +	   && DECL_ORIGINAL_TYPE (t) == NULL_TREE)
> 
> I suspect DECL_IMPLICIT_TYPEDEF_P is a better test for this case.

That works nicely.  How does the following look?  Bootstrapped and
regtested on x86_64-pc-linux-gnu.

-- >8 --

gcc/cp/ChangeLog:

	* constraint.cc (get_normalized_constraints_from_decl): Use
	push_access_scope_guard instead of push_nested_class_guard.
	* cp-tree.h (struct push_nested_class_guard): Replace with ...
	(struct push_access_scope_guard): ... this.
	* pt.c (push_access_scope): When the argument corresponds to
	a class type, push the class instead of its context.
	(pop_access_scope): Adjust accordingly.

gcc/testsuite/ChangeLog:

	* g++.dg/cpp2a/concepts-access2.C: New test.
---
 gcc/cp/constraint.cc                          |  7 +-----
 gcc/cp/cp-tree.h                              | 23 +++++++++++--------
 gcc/cp/pt.c                                   |  7 +++++-
 gcc/testsuite/g++.dg/cpp2a/concepts-access2.C | 13 +++++++++++
 4 files changed, 33 insertions(+), 17 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-access2.C

diff --git a/gcc/cp/constraint.cc b/gcc/cp/constraint.cc
index 6df3ca6ce32..99d3ccc6998 100644
--- a/gcc/cp/constraint.cc
+++ b/gcc/cp/constraint.cc
@@ -926,12 +926,7 @@ get_normalized_constraints_from_decl (tree d, bool diag = false)
   tree norm = NULL_TREE;
   if (tree ci = get_constraints (decl))
     {
-      push_nested_class_guard pncs (DECL_CONTEXT (d));
-
-      temp_override<tree> ovr (current_function_decl);
-      if (TREE_CODE (decl) == FUNCTION_DECL)
-	current_function_decl = decl;
-
+      push_access_scope_guard pas (decl);
       norm = get_normalized_constraints_from_info (ci, tmpl, diag);
     }
 
diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index 6f713719589..58da7460001 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -8463,21 +8463,24 @@ is_constrained_auto (const_tree t)
   return is_auto (t) && PLACEHOLDER_TYPE_CONSTRAINTS_INFO (t);
 }
 
-/* RAII class to push/pop class scope T; if T is not a class, do nothing.  */
+/* RAII class to push/pop the access scope for T.  */
 
-struct push_nested_class_guard
+struct push_access_scope_guard
 {
-  bool push;
-  push_nested_class_guard (tree t)
-    : push (t && CLASS_TYPE_P (t))
+  tree decl;
+  push_access_scope_guard (tree t)
+    : decl (t)
   {
-    if (push)
-      push_nested_class (t);
+    if (VAR_OR_FUNCTION_DECL_P (decl)
+	|| TREE_CODE (decl) == TYPE_DECL)
+      push_access_scope (decl);
+    else
+      decl = NULL_TREE;
   }
-  ~push_nested_class_guard ()
+  ~push_access_scope_guard ()
   {
-    if (push)
-      pop_nested_class ();
+    if (decl)
+      pop_access_scope (decl);
   }
 };
 
diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index d2936c106ba..f8f7616bf2a 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -224,7 +224,7 @@ static void instantiate_body (tree pattern, tree args, tree d, bool nested);
 /* Make the current scope suitable for access checking when we are
    processing T.  T can be FUNCTION_DECL for instantiated function
    template, VAR_DECL for static member variable, or TYPE_DECL for
-   alias template (needed by instantiate_decl).  */
+   for a class or alias template (needed by instantiate_decl).  */
 
 void
 push_access_scope (tree t)
@@ -234,6 +234,9 @@ push_access_scope (tree t)
 
   if (DECL_FRIEND_CONTEXT (t))
     push_nested_class (DECL_FRIEND_CONTEXT (t));
+  else if (DECL_IMPLICIT_TYPEDEF_P (t)
+	   && CLASS_TYPE_P (TREE_TYPE (t)))
+    push_nested_class (TREE_TYPE (t));
   else if (DECL_CLASS_SCOPE_P (t))
     push_nested_class (DECL_CONTEXT (t));
   else if (deduction_guide_p (t) && DECL_ARTIFICIAL (t))
@@ -260,6 +263,8 @@ pop_access_scope (tree t)
     current_function_decl = saved_access_scope->pop();
 
   if (DECL_FRIEND_CONTEXT (t)
+      || (DECL_IMPLICIT_TYPEDEF_P (t)
+	  && CLASS_TYPE_P (TREE_TYPE (t)))
       || DECL_CLASS_SCOPE_P (t)
       || (deduction_guide_p (t) && DECL_ARTIFICIAL (t)))
     pop_nested_class ();
diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-access2.C b/gcc/testsuite/g++.dg/cpp2a/concepts-access2.C
new file mode 100644
index 00000000000..8ddcad236e3
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp2a/concepts-access2.C
@@ -0,0 +1,13 @@
+// { dg-do compile { target concepts } }
+
+template<class T> requires T::value struct A { };
+template<class T> requires T::value struct B { }; // { dg-error "private" }
+
+struct S {
+private:
+  static constexpr bool value = true;
+  template<class T> requires T::value friend struct A;
+};
+
+A<S> x;
+B<S> y; // { dg-error "constraint" }
-- 
2.32.0.93.g670b81a890

Re: [PATCH 1/2] c++: Fix push_access_scope and introduce RAII wrapper for it

[Jason Merrill]

On 6/30/21 11:03 AM, Patrick Palka wrote:
> On Tue, 29 Jun 2021, Jason Merrill wrote:
> 
>> On 6/29/21 1:57 PM, Patrick Palka wrote:
>>> When push_access_scope is passed a TYPE_DECL for a class type (which
>>> can happen during e.g. satisfaction), we undesirably push only the
>>> enclosing context of the class instead of the class itself.  This causes
>>> us to mishandle e.g. testcase below due to us not entering the scope of
>>> A before checking its constraints.
>>>
>>> This patch adjusts push_access_scope accordingly, and introduces an
>>> RAII wrapper for it.  We also make use of this wrapper right away by
>>> replacing the only use of push_nested_class_guard with this new wrapper,
>>> which means we can remove this old wrapper (whose functionality is
>>> basically subsumed by the new wrapper).
>>>
>>> Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for
>>> trunk?
>>>
>>> gcc/cp/ChangeLog:
>>>
>>> 	* constraint.cc (get_normalized_constraints_from_decl): Use
>>> 	push_access_scope_guard instead of push_nested_class_guard.
>>> 	* cp-tree.h (struct push_nested_class_guard): Replace with ...
>>> 	(struct push_access_scope_guard): ... this.
>>> 	* pt.c (push_access_scope): When the argument corresponds to
>>> 	a class type, push the class instead of its context.
>>> 	(pop_access_scope): Adjust accordingly.
>>>
>>> gcc/testsuite/ChangeLog:
>>>
>>> 	* g++.dg/cpp2a/concepts-access2.C: New test.
>>> ---
>>>    gcc/cp/constraint.cc                          |  7 +-----
>>>    gcc/cp/cp-tree.h                              | 23 +++++++++++--------
>>>    gcc/cp/pt.c                                   |  9 +++++++-
>>>    gcc/testsuite/g++.dg/cpp2a/concepts-access2.C | 13 +++++++++++
>>>    4 files changed, 35 insertions(+), 17 deletions(-)
>>>    create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-access2.C
>>>
>>> diff --git a/gcc/cp/constraint.cc b/gcc/cp/constraint.cc
>>> index 6df3ca6ce32..99d3ccc6998 100644
>>> --- a/gcc/cp/constraint.cc
>>> +++ b/gcc/cp/constraint.cc
>>> @@ -926,12 +926,7 @@ get_normalized_constraints_from_decl (tree d, bool diag
>>> = false)
>>>      tree norm = NULL_TREE;
>>>      if (tree ci = get_constraints (decl))
>>>        {
>>> -      push_nested_class_guard pncs (DECL_CONTEXT (d));
>>> -
>>> -      temp_override<tree> ovr (current_function_decl);
>>> -      if (TREE_CODE (decl) == FUNCTION_DECL)
>>> -	current_function_decl = decl;
>>> -
>>> +      push_access_scope_guard pas (decl);
>>>          norm = get_normalized_constraints_from_info (ci, tmpl, diag);
>>>        }
>>>    diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
>>> index 6f713719589..58da7460001 100644
>>> --- a/gcc/cp/cp-tree.h
>>> +++ b/gcc/cp/cp-tree.h
>>> @@ -8463,21 +8463,24 @@ is_constrained_auto (const_tree t)
>>>      return is_auto (t) && PLACEHOLDER_TYPE_CONSTRAINTS_INFO (t);
>>>    }
>>>    -/* RAII class to push/pop class scope T; if T is not a class, do nothing.
>>> */
>>> +/* RAII class to push/pop the access scope for T.  */
>>>    -struct push_nested_class_guard
>>> +struct push_access_scope_guard
>>>    {
>>> -  bool push;
>>> -  push_nested_class_guard (tree t)
>>> -    : push (t && CLASS_TYPE_P (t))
>>> +  tree decl;
>>> +  push_access_scope_guard (tree t)
>>> +    : decl (t)
>>>      {
>>> -    if (push)
>>> -      push_nested_class (t);
>>> +    if (VAR_OR_FUNCTION_DECL_P (decl)
>>> +	|| TREE_CODE (decl) == TYPE_DECL)
>>> +      push_access_scope (decl);
>>> +    else
>>> +      decl = NULL_TREE;
>>>      }
>>> -  ~push_nested_class_guard ()
>>> +  ~push_access_scope_guard ()
>>>      {
>>> -    if (push)
>>> -      pop_nested_class ();
>>> +    if (decl)
>>> +      pop_access_scope (decl);
>>>      }
>>>    };
>>>    diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
>>> index f2039e09cd7..bd8b17ca047 100644
>>> --- a/gcc/cp/pt.c
>>> +++ b/gcc/cp/pt.c
>>> @@ -224,7 +224,7 @@ static void instantiate_body (tree pattern, tree args,
>>> tree d, bool nested);
>>>    /* Make the current scope suitable for access checking when we are
>>>       processing T.  T can be FUNCTION_DECL for instantiated function
>>>       template, VAR_DECL for static member variable, or TYPE_DECL for
>>> -   alias template (needed by instantiate_decl).  */
>>> +   for a class or alias template (needed by instantiate_decl).  */
>>>      void
>>>    push_access_scope (tree t)
>>> @@ -234,6 +234,10 @@ push_access_scope (tree t)
>>>        if (DECL_FRIEND_CONTEXT (t))
>>>        push_nested_class (DECL_FRIEND_CONTEXT (t));
>>> +  else if (TREE_CODE (t) == TYPE_DECL
>>> +	   && CLASS_TYPE_P (TREE_TYPE (t))
>>> +	   && DECL_ORIGINAL_TYPE (t) == NULL_TREE)
>>
>> I suspect DECL_IMPLICIT_TYPEDEF_P is a better test for this case.
> 
> That works nicely.  How does the following look?  Bootstrapped and
> regtested on x86_64-pc-linux-gnu.

OK.

> -- >8 --
> 
> gcc/cp/ChangeLog:
> 
> 	* constraint.cc (get_normalized_constraints_from_decl): Use
> 	push_access_scope_guard instead of push_nested_class_guard.
> 	* cp-tree.h (struct push_nested_class_guard): Replace with ...
> 	(struct push_access_scope_guard): ... this.
> 	* pt.c (push_access_scope): When the argument corresponds to
> 	a class type, push the class instead of its context.
> 	(pop_access_scope): Adjust accordingly.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* g++.dg/cpp2a/concepts-access2.C: New test.
> ---
>   gcc/cp/constraint.cc                          |  7 +-----
>   gcc/cp/cp-tree.h                              | 23 +++++++++++--------
>   gcc/cp/pt.c                                   |  7 +++++-
>   gcc/testsuite/g++.dg/cpp2a/concepts-access2.C | 13 +++++++++++
>   4 files changed, 33 insertions(+), 17 deletions(-)
>   create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-access2.C
> 
> diff --git a/gcc/cp/constraint.cc b/gcc/cp/constraint.cc
> index 6df3ca6ce32..99d3ccc6998 100644
> --- a/gcc/cp/constraint.cc
> +++ b/gcc/cp/constraint.cc
> @@ -926,12 +926,7 @@ get_normalized_constraints_from_decl (tree d, bool diag = false)
>     tree norm = NULL_TREE;
>     if (tree ci = get_constraints (decl))
>       {
> -      push_nested_class_guard pncs (DECL_CONTEXT (d));
> -
> -      temp_override<tree> ovr (current_function_decl);
> -      if (TREE_CODE (decl) == FUNCTION_DECL)
> -	current_function_decl = decl;
> -
> +      push_access_scope_guard pas (decl);
>         norm = get_normalized_constraints_from_info (ci, tmpl, diag);
>       }
>   
> diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
> index 6f713719589..58da7460001 100644
> --- a/gcc/cp/cp-tree.h
> +++ b/gcc/cp/cp-tree.h
> @@ -8463,21 +8463,24 @@ is_constrained_auto (const_tree t)
>     return is_auto (t) && PLACEHOLDER_TYPE_CONSTRAINTS_INFO (t);
>   }
>   
> -/* RAII class to push/pop class scope T; if T is not a class, do nothing.  */
> +/* RAII class to push/pop the access scope for T.  */
>   
> -struct push_nested_class_guard
> +struct push_access_scope_guard
>   {
> -  bool push;
> -  push_nested_class_guard (tree t)
> -    : push (t && CLASS_TYPE_P (t))
> +  tree decl;
> +  push_access_scope_guard (tree t)
> +    : decl (t)
>     {
> -    if (push)
> -      push_nested_class (t);
> +    if (VAR_OR_FUNCTION_DECL_P (decl)
> +	|| TREE_CODE (decl) == TYPE_DECL)
> +      push_access_scope (decl);
> +    else
> +      decl = NULL_TREE;
>     }
> -  ~push_nested_class_guard ()
> +  ~push_access_scope_guard ()
>     {
> -    if (push)
> -      pop_nested_class ();
> +    if (decl)
> +      pop_access_scope (decl);
>     }
>   };
>   
> diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
> index d2936c106ba..f8f7616bf2a 100644
> --- a/gcc/cp/pt.c
> +++ b/gcc/cp/pt.c
> @@ -224,7 +224,7 @@ static void instantiate_body (tree pattern, tree args, tree d, bool nested);
>   /* Make the current scope suitable for access checking when we are
>      processing T.  T can be FUNCTION_DECL for instantiated function
>      template, VAR_DECL for static member variable, or TYPE_DECL for
> -   alias template (needed by instantiate_decl).  */
> +   for a class or alias template (needed by instantiate_decl).  */
>   
>   void
>   push_access_scope (tree t)
> @@ -234,6 +234,9 @@ push_access_scope (tree t)
>   
>     if (DECL_FRIEND_CONTEXT (t))
>       push_nested_class (DECL_FRIEND_CONTEXT (t));
> +  else if (DECL_IMPLICIT_TYPEDEF_P (t)
> +	   && CLASS_TYPE_P (TREE_TYPE (t)))
> +    push_nested_class (TREE_TYPE (t));
>     else if (DECL_CLASS_SCOPE_P (t))
>       push_nested_class (DECL_CONTEXT (t));
>     else if (deduction_guide_p (t) && DECL_ARTIFICIAL (t))
> @@ -260,6 +263,8 @@ pop_access_scope (tree t)
>       current_function_decl = saved_access_scope->pop();
>   
>     if (DECL_FRIEND_CONTEXT (t)
> +      || (DECL_IMPLICIT_TYPEDEF_P (t)
> +	  && CLASS_TYPE_P (TREE_TYPE (t)))
>         || DECL_CLASS_SCOPE_P (t)
>         || (deduction_guide_p (t) && DECL_ARTIFICIAL (t)))
>       pop_nested_class ();
> diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-access2.C b/gcc/testsuite/g++.dg/cpp2a/concepts-access2.C
> new file mode 100644
> index 00000000000..8ddcad236e3
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp2a/concepts-access2.C
> @@ -0,0 +1,13 @@
> +// { dg-do compile { target concepts } }
> +
> +template<class T> requires T::value struct A { };
> +template<class T> requires T::value struct B { }; // { dg-error "private" }
> +
> +struct S {
> +private:
> +  static constexpr bool value = true;
> +  template<class T> requires T::value friend struct A;
> +};
> +
> +A<S> x;
> +B<S> y; // { dg-error "constraint" }
>