Re: [PATCH] c++: partial spec constraint checking context [PR105220]

[Patrick Palka <ppalka@redhat.com>]

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


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