On Mon, 16 Sep 2024, Patrick Palka wrote:
> On Thu, 30 Nov 2023, Patrick Palka wrote:
>
> > On Fri, 3 Nov 2023, Patrick Palka wrote:
> >
> > > On Tue, 3 May 2022, Jason Merrill wrote:
> > >
> > > > On 5/2/22 14:50, Patrick Palka wrote:
> > > > > Currently when checking the constraints of a class template, we do so
> > > > > in
> > > > > the context of the template, not the specialized type. This is the
> > > > > best
> > > > > we can do for a primary template since the specialized type is valid
> > > > > only if the primary template's constraints are satisfied.
> > > >
> > > > Hmm, that's unfortunate. It ought to be possible, if awkward, to form
> > > > the
> > > > type long enough to check its constraints.
> > >
> > > (Sorry, lost track of this patch...)
> > >
> > > Seems doable, but I'm not sure if would make any difference in practice?
> > >
> > > If the access context during satisfaction of a primary class template's
> > > constraints is the specialization rather than the primary template,
> > > then that should only make a difference if there's some friend declaration
> > > naming the specialization. But that'd mean the specialization's
> > > constraints had to have been satisfied at that point, before the friend
> > > declaration went into effect. So either the constraints don't depend on
> > > the access granted by the friend declaration anyway, or they do and the
> > > program is ill-formed (due to either satifaction failure or instability)
> > > IIUC.
> > >
> > > For example, I don't think an adapted version of the testcase without a
> > > partial specialization is valid, regardless of whether the access context
> > > during satisfaction of A<B> is A<B> or just A:
> > >
> > > template<class T>
> > > concept fooable = requires(T t) { t.foo(); };
> > >
> > > template<fooable T>
> > > struct A { };
> > >
> > > struct B {
> > > private:
> > > friend struct A<B>; // satisfaction failure at this point
> > > void foo();
> > > };
> > >
> > > template struct A<B>;
> >
> > ... so in light of the above, I wonder if the original patch can go in
> > as-is?
>
> Ping.
Ping.
>
> >
> > >
> > >
> > > >
> > > > > But for a
> > > > > partial specialization, we can assume the specialized type is valid
> > > > > (as
> > > > > a consequence of constraints being checked only when necessary), so we
> > > > > arguably should check the constraints on a partial specialization more
> > > > > specifically in the context of the specialized type, not the template.
> > > > >
> > > > > This patch implements this by substituting and setting the access
> > > > > context appropriately in satisfy_declaration_constraints. Note that
> > > > > setting the access context in this case is somewhat redundant since
> > > > > the
> > > > > relevant caller most_specialized_partial_spec will already have set
> > > > > the
> > > > > access context to the specialiation, but this redundancy should be
> > > > > harmless.
> > > > >
> > > > > Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK
> > > > > for
> > > > > trunk and perhaps 12.2 (after the branch is thawed)?
> > > > >
> > > > > PR c++/105220
> > > > >
> > > > > gcc/cp/ChangeLog:
> > > > >
> > > > > * constraint.cc (satisfy_declaration_constraints): When checking
> > > > > the constraints of a partial template specialization, do so in
> > > > > the context of the specialized type not the template.
> > > > >
> > > > > gcc/testsuite/ChangeLog:
> > > > >
> > > > > * g++.dg/cpp2a/concepts-partial-spec12.C: New test.
> > > > > ---
> > > > > gcc/cp/constraint.cc | 17 ++++++++++++++---
> > > > > .../g++.dg/cpp2a/concepts-partial-spec12.C | 19
> > > > > +++++++++++++++++++
> > > > > 2 files changed, 33 insertions(+), 3 deletions(-)
> > > > > create mode 100644
> > > > > gcc/testsuite/g++.dg/cpp2a/concepts-partial-spec12.C
> > > > >
> > > > > diff --git a/gcc/cp/constraint.cc b/gcc/cp/constraint.cc
> > > > > index 94f6222b436..772f8532b47 100644
> > > > > --- a/gcc/cp/constraint.cc
> > > > > +++ b/gcc/cp/constraint.cc
> > > > > @@ -3253,11 +3253,22 @@ satisfy_declaration_constraints (tree t, tree
> > > > > args,
> > > > > sat_info info)
> > > > > {
> > > > > if (!push_tinst_level (t, args))
> > > > > return result;
> > > > > - tree pattern = DECL_TEMPLATE_RESULT (t);
> > > > > + tree ascope = DECL_TEMPLATE_RESULT (t);
> > > > > + if (CLASS_TYPE_P (TREE_TYPE (t))
> > > > > + && CLASSTYPE_TEMPLATE_SPECIALIZATION (TREE_TYPE (t)))
> > > > > + {
> > > > > + gcc_checking_assert (t == most_general_template (t));
> > > > > + /* When checking the constraints on a partial specialization,
> > > > > + do so in the context of the specialized type, not the
> > > > > template.
> > > > > + This substitution should always succeed since we shouldn't
> > > > > + be checking constraints thereof unless the specialized type
> > > > > + is valid. */
> > > > > + ascope = tsubst (ascope, args, tf_none, info.in_decl);
> > > > > + }
> > > > > push_to_top_level ();
> > > > > - push_access_scope (pattern);
> > > > > + push_access_scope (ascope);
> > > > > result = satisfy_normalized_constraints (norm, args, info);
> > > > > - pop_access_scope (pattern);
> > > > > + pop_access_scope (ascope);
> > > > > pop_from_top_level ();
> > > > > pop_tinst_level ();
> > > > > }
> > > > > diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-partial-spec12.C
> > > > > b/gcc/testsuite/g++.dg/cpp2a/concepts-partial-spec12.C
> > > > > new file mode 100644
> > > > > index 00000000000..641d456722d
> > > > > --- /dev/null
> > > > > +++ b/gcc/testsuite/g++.dg/cpp2a/concepts-partial-spec12.C
> > > > > @@ -0,0 +1,19 @@
> > > > > +// PR c++/105220
> > > > > +// { dg-do compile { target c++20 } }
> > > > > +
> > > > > +template<class T>
> > > > > +concept fooable = requires(T t) { t.foo(); };
> > > > > +
> > > > > +template<class>
> > > > > +struct A; // #1, incomplete
> > > > > +
> > > > > +template<fooable T>
> > > > > +struct A<T> { }; // #2
> > > > > +
> > > > > +struct B {
> > > > > +private:
> > > > > + friend struct A<B>;
> > > > > + void foo();
> > > > > +};
> > > > > +
> > > > > +template struct A<B>; // OK, B::foo() is accessible from #2
> > > >
> > > >
> > >
> >
>
