| Message ID | 20210629175723.1009233-2-ppalka@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | [1/2] c++: Fix push_access_scope and introduce RAII wrapper for it | expand |
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>, ""); > + } > +}; >
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>);
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>); >
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>); > > > >
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>, ""); + } +};