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