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>);


Reply via email to