Re: [PATCH] c++: partial spec constraint checking context [PR105220]
On 1/14/25 9:46 PM, Patrick Palka wrote:
On Tue, 14 Jan 2025, Jason Merrill wrote:
On 1/10/25 1:36 PM, Patrick Palka wrote:
On Tue, 1 Oct 2024, Patrick Palka wrote:
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 is A or just A:
template
concept fooable = requires(T t) { t.foo(); };
template
struct A { };
struct B {
private:
friend struct A; // satisfaction failure at this point
void foo();
};
template struct A;
... so in light of the above, I wonder if the original patch can go in
as-is?
Ping.
Ping.
Ping.
As I commented on the PR, we probably want to stop considering friendship at
all in this testcase, so the value of fooable does not depend on where it
appears. The resolution for 2589 is still under review, but there seems to be
general agreement on that point.
Sorry I didn't previously point to that in this thread.
Whoops, I remember seeing that comment but forgot about it as well
(Bugzilla is down for me so I can't reply to the PR ATM).
Ignoring context/friendship when checking an atomic constraint
normalized from a concept-id such as fooable seems reasonable, but
I'm surprised that the latest revision of the CWG 2589 PR would also
make us ignore friendship for "inline/non-shareable" atomic constraints,
e.g.
template
struct A;
template requires requires(T t) { t.foo(); }
struct A { };
Here the atomic constraint is unique to where it was written, so it
seems we might as well consider friendship in this case?
Yeah, a reply on the list made the same point, there should be further
revision.
Jason
Re: [PATCH] c++: partial spec constraint checking context [PR105220]
On Tue, 14 Jan 2025, Jason Merrill wrote:
> On 1/10/25 1:36 PM, Patrick Palka wrote:
> > On Tue, 1 Oct 2024, Patrick Palka wrote:
> > > 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 is A or just A:
> > > > > >
> > > > > > template
> > > > > > concept fooable = requires(T t) { t.foo(); };
> > > > > >
> > > > > > template
> > > > > > struct A { };
> > > > > >
> > > > > > struct B {
> > > > > > private:
> > > > > >friend struct A; // satisfaction failure at this point
> > > > > >void foo();
> > > > > > };
> > > > > >
> > > > > > template struct A;
> > > > >
> > > > > ... so in light of the above, I wonder if the original patch can go in
> > > > > as-is?
> > > >
> > > > Ping.
> > >
> > > Ping.
> >
> > Ping.
>
> As I commented on the PR, we probably want to stop considering friendship at
> all in this testcase, so the value of fooable does not depend on where it
> appears. The resolution for 2589 is still under review, but there seems to be
> general agreement on that point.
>
> Sorry I didn't previously point to that in this thread.
Whoops, I remember seeing that comment but forgot about it as well
(Bugzilla is down for me so I can't reply to the PR ATM).
Ignoring context/friendship when checking an atomic constraint
normalized from a concept-id such as fooable seems reasonable, but
I'm surprised that the latest revision of the CWG 2589 PR would also
make us ignore friendship for "inline/non-shareable" atomic constraints,
e.g.
template
struct A;
template requires requires(T t) { t.foo(); }
struct A { };
Here the atomic constraint is unique to where it was written, so it
seems we might as well consider friendship in this case?
Re: [PATCH] c++: partial spec constraint checking context [PR105220]
On 1/10/25 1:36 PM, Patrick Palka wrote:
On Tue, 1 Oct 2024, Patrick Palka wrote:
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 is A or just A:
template
concept fooable = requires(T t) { t.foo(); };
template
struct A { };
struct B {
private:
friend struct A; // satisfaction failure at this point
void foo();
};
template struct A;
... so in light of the above, I wonder if the original patch can go in
as-is?
Ping.
Ping.
Ping.
As I commented on the PR, we probably want to stop considering
friendship at all in this testcase, so the value of fooable does not
depend on where it appears. The resolution for 2589 is still under
review, but there seems to be general agreement on that point.
Sorry I didn't previously point to that in this thread.
Jason
Re: [PATCH] c++: partial spec constraint checking context [PR105220]
On Tue, 1 Oct 2024, Patrick Palka wrote:
> 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 is A or just A:
> > > >
> > > > template
> > > > concept fooable = requires(T t) { t.foo(); };
> > > >
> > > > template
> > > > struct A { };
> > > >
> > > > struct B {
> > > > private:
> > > > friend struct A; // satisfaction failure at this point
> > > > void foo();
> > > > };
> > > >
> > > > template struct A;
> > >
> > > ... so in light of the above, I wonder if the original patch can go in
> > > as-is?
> >
> > Ping.
>
> 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
Re: [PATCH] c++: partial spec constraint checking context [PR105220]
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 is A or just A:
> > >
> > > template
> > > concept fooable = requires(T t) { t.foo(); };
> > >
> > > template
> > > struct A { };
> > >
> > > struct B {
> > > private:
> > > friend struct A; // satisfaction failure at this point
> > > void foo();
> > > };
> > >
> > > template struct A;
> >
> > ... 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 un
Re: [PATCH] c++: partial spec constraint checking context [PR105220]
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 is A or just A:
> >
> > template
> > concept fooable = requires(T t) { t.foo(); };
> >
> > template
> > struct A { };
> >
> > struct B {
> > private:
> > friend struct A; // satisfaction failure at this point
> > void foo();
> > };
> >
> > template struct A;
>
> ... so in light of the above, I wonder if the original patch can go in
> as-is?
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
Re: [PATCH] c++: partial spec constraint checking context [PR105220]
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 is A or just A:
>
> template
> concept fooable = requires(T t) { t.foo(); };
>
> template
> struct A { };
>
> struct B {
> private:
> friend struct A; // satisfaction failure at this point
> void foo();
> };
>
> template struct A;
... so in light of the above, I wonder if the original patch can go in
as-is?
>
>
> >
> > > 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 000..641d456722d
Re: [PATCH] c++: partial spec constraint checking context [PR105220]
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 is A or just A:
template
concept fooable = requires(T t) { t.foo(); };
template
struct A { };
struct B {
private:
friend struct A; // satisfaction failure at this point
void foo();
};
template struct A;
>
> > 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 000..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
> > +concept fooable = requires(T t) { t.foo(); };
> > +
> > +template
> > +struct A;// #1, incomplete
> > +
> > +template
> > +struct A {
Re: [PATCH] c++: partial spec constraint checking context [PR105220]
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.
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 000..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
+concept fooable = requires(T t) { t.foo(); };
+
+template
+struct A;// #1, incomplete
+
+template
+struct A { }; // #2
+
+struct B {
+private:
+ friend struct A;
+ void foo();
+};
+
+template struct A; // OK, B::foo() is accessible from #2
