Re: [PATCH 4/4] c++: dependent constraint on placeholder 'auto' [PR96443]
On 2/28/21 12:55 PM, Patrick Palka wrote: On Fri, 12 Feb 2021, Jason Merrill wrote: On 2/11/21 5:14 PM, Patrick Palka wrote: On Thu, 11 Feb 2021, Jason Merrill wrote: On 2/8/21 2:03 PM, Patrick Palka wrote: This fixes the way we check satisfaction of constraints on placeholder types in various contexts, and in particular when the constraint is dependent. Firstly, when evaluating the return type requirement of a compound requirement, we currently substitute the outer template arguments into the constraint before checking satisfaction. But we should instead be passing in the complete set of template arguments to satisfaction and not do a prior separate substitution. Our current approach leads to us incorrectly rejecting the testcase concepts-return-req2.C below. Secondly, when checking the constraints on a placeholder variable or return type, we don't substitute the template arguments of the enclosing context at all. This leads to bogus errors during satisfaction when the constraint is dependent as in the testcase concepts-placeholder3.C below. In order to fix these two issues, we need to be able to properly normalize the constraints on a placeholder 'auto', which in turn requires us to know the template parameters that were in-scope where an 'auto' was introduced. This information currently doesn't seem to be easily available when we need it, so this patch adds an auxiliary hash table that keeps track of the value of current_template_parms when each constrained 'auto' was formed. This patch also removes some seemingly wrong handling of placeholder type arguments from tsubst_parameter_mapping. The code doesn't trigger with the example used in the comments, because type_uses_auto doesn't look inside non-deduced contexts such as the operand of decltype. And the call to do_auto_deduction seems confused because if 'arg' is a type, then so is 'parm', and therefore 'init' too is a type, but do_auto_deduction expects it to be an expression. Before this patch, this code was dead (as far as our testsuite can tell), but now it breaks other parts of this patch, so let's remove it. gcc/cp/ChangeLog: PR c++/96443 * constraint.cc (type_deducible_p): Don't substitute into the constraints, and instead just pass 'args' to do_auto_deduction as the outer template arguments. (tsubst_parameter_mapping): Remove confused code for handling placeholder type arguments. (normalize_placeholder_type_constraint): Define. (satisfy_constraint_expression): Use it to handle placeholder 'auto' types. * cp-tree.h (get_constrained_auto_context): Declare. * pt.c (constrained_auto_context_map): Define. (get_placeholder_type_constraint_context): Define. (set_placeholder_type_constraints): Define. (copy_placeholder_type_constraints): Define. (tsubst) : Use copy_placeholder_type_constraints. (make_constrained_placeholder_type): Use set_placeholder_type_constraints. (do_auto_deduction): Clarify comments about the outer_targs parameter. Rework satisfaction of a placeholder type constraint to pass in the complete set of template arguments directly to constraints_satisfied_p. (splice_late_return_type): Use copy_placeholder_type_constraints. gcc/testsuite/ChangeLog: PR c++/96443 * g++.dg/cpp2a/concepts-placeholder3.C: New test. * g++.dg/cpp2a/concepts-return-req2.C: New test. * g++.dg/concepts-ts1.C: Add dg-bogus directive to the call to f15 that we expect to accept. --- gcc/cp/constraint.cc | 106 -- gcc/cp/cp-tree.h | 1 + gcc/cp/pt.c | 101 +++-- .../g++.dg/cpp2a/concepts-placeholder3.C | 19 .../g++.dg/cpp2a/concepts-return-req2.C | 13 +++ gcc/testsuite/g++.dg/cpp2a/concepts-ts1.C | 2 +- 6 files changed, 146 insertions(+), 96 deletions(-) create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-placeholder3.C create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-return-req2.C diff --git a/gcc/cp/constraint.cc b/gcc/cp/constraint.cc index 56134f8b2bf..53588047d44 100644 --- a/gcc/cp/constraint.cc +++ b/gcc/cp/constraint.cc @@ -2007,39 +2007,19 @@ type_deducible_p (tree expr, tree type, tree placeholder, tree args, references are preserved in the result. */ expr = force_paren_expr_uneval (expr); - /* Replace the constraints with the instantiated constraints. This - substitutes args into any template parameters in the trailing - result type. */ - tree saved_constr = PLACEHOLDER_TYPE_CONSTRAINTS (placeholder); - tree subst_constr -= tsubst_constraint (saved_constr, -args, -info.complain | tf_partial, -info.in_decl); - - if
Re: [PATCH 4/4] c++: dependent constraint on placeholder 'auto' [PR96443]
On Fri, 12 Feb 2021, Jason Merrill wrote: > On 2/11/21 5:14 PM, Patrick Palka wrote: > > On Thu, 11 Feb 2021, Jason Merrill wrote: > > > > > On 2/8/21 2:03 PM, Patrick Palka wrote: > > > > This fixes the way we check satisfaction of constraints on placeholder > > > > types in various contexts, and in particular when the constraint is > > > > dependent. > > > > > > > > Firstly, when evaluating the return type requirement of a compound > > > > requirement, we currently substitute the outer template arguments into > > > > the constraint before checking satisfaction. But we should instead be > > > > passing in the complete set of template arguments to satisfaction and > > > > not do a prior separate substitution. Our current approach leads to us > > > > incorrectly rejecting the testcase concepts-return-req2.C below. > > > > > > > > Secondly, when checking the constraints on a placeholder variable or > > > > return type, we don't substitute the template arguments of the enclosing > > > > context at all. This leads to bogus errors during satisfaction when the > > > > constraint is dependent as in the testcase concepts-placeholder3.C > > > > below. > > > > > > > > In order to fix these two issues, we need to be able to properly > > > > normalize the constraints on a placeholder 'auto', which in turn > > > > requires us to know the template parameters that were in-scope where an > > > > 'auto' was introduced. This information currently doesn't seem to be > > > > easily available when we need it, so this patch adds an auxiliary hash > > > > table that keeps track of the value of current_template_parms when each > > > > constrained 'auto' was formed. > > > > > > > > This patch also removes some seemingly wrong handling of placeholder > > > > type arguments from tsubst_parameter_mapping. The code doesn't trigger > > > > with the example used in the comments, because type_uses_auto doesn't > > > > look inside non-deduced contexts such as the operand of decltype. And > > > > the call to do_auto_deduction seems confused because if 'arg' is a type, > > > > then so is 'parm', and therefore 'init' too is a type, but > > > > do_auto_deduction expects it to be an expression. Before this patch, > > > > this code was dead (as far as our testsuite can tell), but now it breaks > > > > other parts of this patch, so let's remove it. > > > > > > > > gcc/cp/ChangeLog: > > > > > > > > PR c++/96443 > > > > * constraint.cc (type_deducible_p): Don't substitute into the > > > > constraints, and instead just pass 'args' to do_auto_deduction > > > > as the outer template arguments. > > > > (tsubst_parameter_mapping): Remove confused code for handling > > > > placeholder type arguments. > > > > (normalize_placeholder_type_constraint): Define. > > > > (satisfy_constraint_expression): Use it to handle placeholder > > > > 'auto' types. > > > > * cp-tree.h (get_constrained_auto_context): Declare. > > > > * pt.c (constrained_auto_context_map): Define. > > > > (get_placeholder_type_constraint_context): Define. > > > > (set_placeholder_type_constraints): Define. > > > > (copy_placeholder_type_constraints): Define. > > > > (tsubst) : Use > > > > copy_placeholder_type_constraints. > > > > (make_constrained_placeholder_type): Use > > > > set_placeholder_type_constraints. > > > > (do_auto_deduction): Clarify comments about the outer_targs > > > > parameter. Rework satisfaction of a placeholder type constraint > > > > to pass in the complete set of template arguments directly to > > > > constraints_satisfied_p. > > > > (splice_late_return_type): Use > > > > copy_placeholder_type_constraints. > > > > > > > > gcc/testsuite/ChangeLog: > > > > > > > > PR c++/96443 > > > > * g++.dg/cpp2a/concepts-placeholder3.C: New test. > > > > * g++.dg/cpp2a/concepts-return-req2.C: New test. > > > > * g++.dg/concepts-ts1.C: Add dg-bogus directive to the call to > > > > f15 that we expect to accept. > > > > --- > > > >gcc/cp/constraint.cc | 106 > > > > -- > > > >gcc/cp/cp-tree.h | 1 + > > > >gcc/cp/pt.c | 101 +++-- > > > >.../g++.dg/cpp2a/concepts-placeholder3.C | 19 > > > >.../g++.dg/cpp2a/concepts-return-req2.C | 13 +++ > > > >gcc/testsuite/g++.dg/cpp2a/concepts-ts1.C | 2 +- > > > >6 files changed, 146 insertions(+), 96 deletions(-) > > > >create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-placeholder3.C > > > >create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-return-req2.C > > > > > > > > diff --git a/gcc/cp/constraint.cc b/gcc/cp/constraint.cc > > > > index 56134f8b2bf..53588047d44 100644 > > > > --- a/gcc/cp/constraint.cc > > > > +++
Re: [PATCH 4/4] c++: dependent constraint on placeholder 'auto' [PR96443]
On 2/11/21 5:14 PM, Patrick Palka wrote: On Thu, 11 Feb 2021, Jason Merrill wrote: On 2/8/21 2:03 PM, Patrick Palka wrote: This fixes the way we check satisfaction of constraints on placeholder types in various contexts, and in particular when the constraint is dependent. Firstly, when evaluating the return type requirement of a compound requirement, we currently substitute the outer template arguments into the constraint before checking satisfaction. But we should instead be passing in the complete set of template arguments to satisfaction and not do a prior separate substitution. Our current approach leads to us incorrectly rejecting the testcase concepts-return-req2.C below. Secondly, when checking the constraints on a placeholder variable or return type, we don't substitute the template arguments of the enclosing context at all. This leads to bogus errors during satisfaction when the constraint is dependent as in the testcase concepts-placeholder3.C below. In order to fix these two issues, we need to be able to properly normalize the constraints on a placeholder 'auto', which in turn requires us to know the template parameters that were in-scope where an 'auto' was introduced. This information currently doesn't seem to be easily available when we need it, so this patch adds an auxiliary hash table that keeps track of the value of current_template_parms when each constrained 'auto' was formed. This patch also removes some seemingly wrong handling of placeholder type arguments from tsubst_parameter_mapping. The code doesn't trigger with the example used in the comments, because type_uses_auto doesn't look inside non-deduced contexts such as the operand of decltype. And the call to do_auto_deduction seems confused because if 'arg' is a type, then so is 'parm', and therefore 'init' too is a type, but do_auto_deduction expects it to be an expression. Before this patch, this code was dead (as far as our testsuite can tell), but now it breaks other parts of this patch, so let's remove it. gcc/cp/ChangeLog: PR c++/96443 * constraint.cc (type_deducible_p): Don't substitute into the constraints, and instead just pass 'args' to do_auto_deduction as the outer template arguments. (tsubst_parameter_mapping): Remove confused code for handling placeholder type arguments. (normalize_placeholder_type_constraint): Define. (satisfy_constraint_expression): Use it to handle placeholder 'auto' types. * cp-tree.h (get_constrained_auto_context): Declare. * pt.c (constrained_auto_context_map): Define. (get_placeholder_type_constraint_context): Define. (set_placeholder_type_constraints): Define. (copy_placeholder_type_constraints): Define. (tsubst) : Use copy_placeholder_type_constraints. (make_constrained_placeholder_type): Use set_placeholder_type_constraints. (do_auto_deduction): Clarify comments about the outer_targs parameter. Rework satisfaction of a placeholder type constraint to pass in the complete set of template arguments directly to constraints_satisfied_p. (splice_late_return_type): Use copy_placeholder_type_constraints. gcc/testsuite/ChangeLog: PR c++/96443 * g++.dg/cpp2a/concepts-placeholder3.C: New test. * g++.dg/cpp2a/concepts-return-req2.C: New test. * g++.dg/concepts-ts1.C: Add dg-bogus directive to the call to f15 that we expect to accept. --- gcc/cp/constraint.cc | 106 -- gcc/cp/cp-tree.h | 1 + gcc/cp/pt.c | 101 +++-- .../g++.dg/cpp2a/concepts-placeholder3.C | 19 .../g++.dg/cpp2a/concepts-return-req2.C | 13 +++ gcc/testsuite/g++.dg/cpp2a/concepts-ts1.C | 2 +- 6 files changed, 146 insertions(+), 96 deletions(-) create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-placeholder3.C create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-return-req2.C diff --git a/gcc/cp/constraint.cc b/gcc/cp/constraint.cc index 56134f8b2bf..53588047d44 100644 --- a/gcc/cp/constraint.cc +++ b/gcc/cp/constraint.cc @@ -2007,39 +2007,19 @@ type_deducible_p (tree expr, tree type, tree placeholder, tree args, references are preserved in the result. */ expr = force_paren_expr_uneval (expr); - /* Replace the constraints with the instantiated constraints. This - substitutes args into any template parameters in the trailing - result type. */ - tree saved_constr = PLACEHOLDER_TYPE_CONSTRAINTS (placeholder); - tree subst_constr -= tsubst_constraint (saved_constr, -args, -info.complain | tf_partial, -info.in_decl); - - if (subst_constr == error_mark_node) -return false; - - PLACEHOLDER_TYPE_CONSTRAINTS (placeholder) =
Re: [PATCH 4/4] c++: dependent constraint on placeholder 'auto' [PR96443]
On Thu, 11 Feb 2021, Jason Merrill wrote: > On 2/8/21 2:03 PM, Patrick Palka wrote: > > This fixes the way we check satisfaction of constraints on placeholder > > types in various contexts, and in particular when the constraint is > > dependent. > > > > Firstly, when evaluating the return type requirement of a compound > > requirement, we currently substitute the outer template arguments into > > the constraint before checking satisfaction. But we should instead be > > passing in the complete set of template arguments to satisfaction and > > not do a prior separate substitution. Our current approach leads to us > > incorrectly rejecting the testcase concepts-return-req2.C below. > > > > Secondly, when checking the constraints on a placeholder variable or > > return type, we don't substitute the template arguments of the enclosing > > context at all. This leads to bogus errors during satisfaction when the > > constraint is dependent as in the testcase concepts-placeholder3.C > > below. > > > > In order to fix these two issues, we need to be able to properly > > normalize the constraints on a placeholder 'auto', which in turn > > requires us to know the template parameters that were in-scope where an > > 'auto' was introduced. This information currently doesn't seem to be > > easily available when we need it, so this patch adds an auxiliary hash > > table that keeps track of the value of current_template_parms when each > > constrained 'auto' was formed. > > > > This patch also removes some seemingly wrong handling of placeholder > > type arguments from tsubst_parameter_mapping. The code doesn't trigger > > with the example used in the comments, because type_uses_auto doesn't > > look inside non-deduced contexts such as the operand of decltype. And > > the call to do_auto_deduction seems confused because if 'arg' is a type, > > then so is 'parm', and therefore 'init' too is a type, but > > do_auto_deduction expects it to be an expression. Before this patch, > > this code was dead (as far as our testsuite can tell), but now it breaks > > other parts of this patch, so let's remove it. > > > > gcc/cp/ChangeLog: > > > > PR c++/96443 > > * constraint.cc (type_deducible_p): Don't substitute into the > > constraints, and instead just pass 'args' to do_auto_deduction > > as the outer template arguments. > > (tsubst_parameter_mapping): Remove confused code for handling > > placeholder type arguments. > > (normalize_placeholder_type_constraint): Define. > > (satisfy_constraint_expression): Use it to handle placeholder > > 'auto' types. > > * cp-tree.h (get_constrained_auto_context): Declare. > > * pt.c (constrained_auto_context_map): Define. > > (get_placeholder_type_constraint_context): Define. > > (set_placeholder_type_constraints): Define. > > (copy_placeholder_type_constraints): Define. > > (tsubst) : Use > > copy_placeholder_type_constraints. > > (make_constrained_placeholder_type): Use > > set_placeholder_type_constraints. > > (do_auto_deduction): Clarify comments about the outer_targs > > parameter. Rework satisfaction of a placeholder type constraint > > to pass in the complete set of template arguments directly to > > constraints_satisfied_p. > > (splice_late_return_type): Use copy_placeholder_type_constraints. > > > > gcc/testsuite/ChangeLog: > > > > PR c++/96443 > > * g++.dg/cpp2a/concepts-placeholder3.C: New test. > > * g++.dg/cpp2a/concepts-return-req2.C: New test. > > * g++.dg/concepts-ts1.C: Add dg-bogus directive to the call to > > f15 that we expect to accept. > > --- > > gcc/cp/constraint.cc | 106 -- > > gcc/cp/cp-tree.h | 1 + > > gcc/cp/pt.c | 101 +++-- > > .../g++.dg/cpp2a/concepts-placeholder3.C | 19 > > .../g++.dg/cpp2a/concepts-return-req2.C | 13 +++ > > gcc/testsuite/g++.dg/cpp2a/concepts-ts1.C | 2 +- > > 6 files changed, 146 insertions(+), 96 deletions(-) > > create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-placeholder3.C > > create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-return-req2.C > > > > diff --git a/gcc/cp/constraint.cc b/gcc/cp/constraint.cc > > index 56134f8b2bf..53588047d44 100644 > > --- a/gcc/cp/constraint.cc > > +++ b/gcc/cp/constraint.cc > > @@ -2007,39 +2007,19 @@ type_deducible_p (tree expr, tree type, tree > > placeholder, tree args, > >references are preserved in the result. */ > > expr = force_paren_expr_uneval (expr); > > - /* Replace the constraints with the instantiated constraints. This > > - substitutes args into any template parameters in the trailing > > - result type. */ > > - tree saved_constr = PLACEHOLDER_TYPE_CONSTRAINTS (placeholder); > > - tree subst_constr > > -= tsubst_constraint (saved_constr, > > -args, > > -
Re: [PATCH 4/4] c++: dependent constraint on placeholder 'auto' [PR96443]
On 2/8/21 2:03 PM, Patrick Palka wrote: This fixes the way we check satisfaction of constraints on placeholder types in various contexts, and in particular when the constraint is dependent. Firstly, when evaluating the return type requirement of a compound requirement, we currently substitute the outer template arguments into the constraint before checking satisfaction. But we should instead be passing in the complete set of template arguments to satisfaction and not do a prior separate substitution. Our current approach leads to us incorrectly rejecting the testcase concepts-return-req2.C below. Secondly, when checking the constraints on a placeholder variable or return type, we don't substitute the template arguments of the enclosing context at all. This leads to bogus errors during satisfaction when the constraint is dependent as in the testcase concepts-placeholder3.C below. In order to fix these two issues, we need to be able to properly normalize the constraints on a placeholder 'auto', which in turn requires us to know the template parameters that were in-scope where an 'auto' was introduced. This information currently doesn't seem to be easily available when we need it, so this patch adds an auxiliary hash table that keeps track of the value of current_template_parms when each constrained 'auto' was formed. This patch also removes some seemingly wrong handling of placeholder type arguments from tsubst_parameter_mapping. The code doesn't trigger with the example used in the comments, because type_uses_auto doesn't look inside non-deduced contexts such as the operand of decltype. And the call to do_auto_deduction seems confused because if 'arg' is a type, then so is 'parm', and therefore 'init' too is a type, but do_auto_deduction expects it to be an expression. Before this patch, this code was dead (as far as our testsuite can tell), but now it breaks other parts of this patch, so let's remove it. gcc/cp/ChangeLog: PR c++/96443 * constraint.cc (type_deducible_p): Don't substitute into the constraints, and instead just pass 'args' to do_auto_deduction as the outer template arguments. (tsubst_parameter_mapping): Remove confused code for handling placeholder type arguments. (normalize_placeholder_type_constraint): Define. (satisfy_constraint_expression): Use it to handle placeholder 'auto' types. * cp-tree.h (get_constrained_auto_context): Declare. * pt.c (constrained_auto_context_map): Define. (get_placeholder_type_constraint_context): Define. (set_placeholder_type_constraints): Define. (copy_placeholder_type_constraints): Define. (tsubst) : Use copy_placeholder_type_constraints. (make_constrained_placeholder_type): Use set_placeholder_type_constraints. (do_auto_deduction): Clarify comments about the outer_targs parameter. Rework satisfaction of a placeholder type constraint to pass in the complete set of template arguments directly to constraints_satisfied_p. (splice_late_return_type): Use copy_placeholder_type_constraints. gcc/testsuite/ChangeLog: PR c++/96443 * g++.dg/cpp2a/concepts-placeholder3.C: New test. * g++.dg/cpp2a/concepts-return-req2.C: New test. * g++.dg/concepts-ts1.C: Add dg-bogus directive to the call to f15 that we expect to accept. --- gcc/cp/constraint.cc | 106 -- gcc/cp/cp-tree.h | 1 + gcc/cp/pt.c | 101 +++-- .../g++.dg/cpp2a/concepts-placeholder3.C | 19 .../g++.dg/cpp2a/concepts-return-req2.C | 13 +++ gcc/testsuite/g++.dg/cpp2a/concepts-ts1.C | 2 +- 6 files changed, 146 insertions(+), 96 deletions(-) create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-placeholder3.C create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-return-req2.C diff --git a/gcc/cp/constraint.cc b/gcc/cp/constraint.cc index 56134f8b2bf..53588047d44 100644 --- a/gcc/cp/constraint.cc +++ b/gcc/cp/constraint.cc @@ -2007,39 +2007,19 @@ type_deducible_p (tree expr, tree type, tree placeholder, tree args, references are preserved in the result. */ expr = force_paren_expr_uneval (expr); - /* Replace the constraints with the instantiated constraints. This - substitutes args into any template parameters in the trailing - result type. */ - tree saved_constr = PLACEHOLDER_TYPE_CONSTRAINTS (placeholder); - tree subst_constr -= tsubst_constraint (saved_constr, -args, -info.complain | tf_partial, -info.in_decl); - - if (subst_constr == error_mark_node) -return false; - - PLACEHOLDER_TYPE_CONSTRAINTS (placeholder) = subst_constr; - - /* Temporarily unlink the canonical type. */ - tree saved_type = TYPE_CANONICAL
Re: [PATCH 4/4] c++: dependent constraint on placeholder 'auto' [PR96443]
On Mon, 8 Feb 2021, Patrick Palka wrote: > This fixes the way we check satisfaction of constraints on placeholder > types in various contexts, and in particular when the constraint is > dependent. > > Firstly, when evaluating the return type requirement of a compound > requirement, we currently substitute the outer template arguments into > the constraint before checking satisfaction. But we should instead be > passing in the complete set of template arguments to satisfaction and > not do a prior separate substitution. Our current approach leads to us > incorrectly rejecting the testcase concepts-return-req2.C below. > > Secondly, when checking the constraints on a placeholder variable or > return type, we don't substitute the template arguments of the enclosing > context at all. This leads to bogus errors during satisfaction when the > constraint is dependent as in the testcase concepts-placeholder3.C > below. > > In order to fix these two issues, we need to be able to properly > normalize the constraints on a placeholder 'auto', which in turn > requires us to know the template parameters that were in-scope where an > 'auto' was introduced. This information currently doesn't seem to be > easily available when we need it, so this patch adds an auxiliary hash > table that keeps track of the value of current_template_parms when each > constrained 'auto' was formed. > > This patch also removes some seemingly wrong handling of placeholder > type arguments from tsubst_parameter_mapping. The code doesn't trigger > with the example used in the comments, because type_uses_auto doesn't > look inside non-deduced contexts such as the operand of decltype. And > the call to do_auto_deduction seems confused because if 'arg' is a type, > then so is 'parm', and therefore 'init' too is a type, but > do_auto_deduction expects it to be an expression. Before this patch, > this code was dead (as far as our testsuite can tell), but now it breaks > other parts of this patch, so let's remove it. This series was bootstrapped and regtested on x86_64-pc-linux-gnu and tested on range-v3 and cmcstl2, with and without --enable-checking=release.
