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

[Jason Merrill <jason@redhat.com>]

[-- 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