On Thu, 10 Mar 2022, Jason Merrill wrote:
> On 2/16/22 15:56, Patrick Palka wrote:
> > On Tue, 15 Feb 2022, Jason Merrill wrote:
> >
> > > On 2/14/22 11:32, Patrick Palka wrote:
> > > > Here the template context for the atomic constraint has two levels of
> > > > template arguments, but since it depends only on the innermost argument
> > > > T we use a single-level argument vector during substitution into the
> > > > constraint (built by get_mapped_args). We eventually pass this vector
> > > > to do_auto_deduction as part of checking the return-type-requirement
> > > > inside the atom, but do_auto_deduction expects outer_targs to be a full
> > > > set of arguments for sake of satisfaction.
> > >
> > > Could we note the current number of levels in the map and use that in
> > > get_mapped_args instead of the highest level parameter we happened to use?
> >
> > Ah yeah, that seems to work nicely. IIUC it should suffice to remember
> > whether the atomic constraint expression came from a concept definition.
> > If it did, then the depth of the argument vector returned by
> > get_mapped_args must be one, otherwise (as in the testcase) it must be
> > the same as the template depth of the constrained entity, which is the
> > depth of ARGS.
> >
> > How does the following look? Bootstrapped and regtested on
> > x86_64-pc-linux-gnu and also on cmcstl2 and range-v3.
> >
> > -- >8 --
> >
> > Subject: [PATCH] c++: return-type-req in constraint using only outer tparms
> > [PR104527]
> >
> > Here the template context for the atomic constraint has two levels of
> > template parameters, but since it depends only on the innermost parameter
> > T we use a single-level argument vector (built by get_mapped_args) during
> > substitution into the atom. We eventually pass this vector to
> > do_auto_deduction as part of checking the return-type-requirement within
> > the atom, but do_auto_deduction expects outer_targs to be a full set of
> > arguments for sake of satisfaction.
> >
> > This patch fixes this by making get_mapped_args always return an
> > argument vector whose depth corresponds to the template depth of the
> > context in which the atomic constraint expression was written, instead
> > of the highest parameter level that the expression happens to use.
> >
> > PR c++/104527
> >
> > gcc/cp/ChangeLog:
> >
> > * constraint.cc (normalize_atom): Set
> > ATOMIC_CONSTR_EXPR_FROM_CONCEPT_P appropriately.
> > (get_mapped_args): Make static, adjust parameters. Always
> > return a vector whose depth corresponds to the template depth of
> > the context of the atomic constraint expression. Micro-optimize
> > by passing false as exact to safe_grow_cleared and by collapsing
> > a multi-level depth-one argument vector.
> > (satisfy_atom): Adjust call to get_mapped_args and
> > diagnose_atomic_constraint.
> > (diagnose_atomic_constraint): Replace map parameter with an args
> > parameter.
> > * cp-tree.h (ATOMIC_CONSTR_EXPR_FROM_CONCEPT_P): Define.
> > (get_mapped_args): Remove declaration.
> >
> > gcc/testsuite/ChangeLog:
> >
> > * g++.dg/cpp2a/concepts-return-req4.C: New test.
> > ---
> > gcc/cp/constraint.cc | 64 +++++++++++--------
> > gcc/cp/cp-tree.h | 7 +-
> > .../g++.dg/cpp2a/concepts-return-req4.C | 24 +++++++
> > 3 files changed, 69 insertions(+), 26 deletions(-)
> > create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-return-req4.C
> >
> > diff --git a/gcc/cp/constraint.cc b/gcc/cp/constraint.cc
> > index 12db7e5cf14..306e28955c6 100644
> > --- a/gcc/cp/constraint.cc
> > +++ b/gcc/cp/constraint.cc
> > @@ -764,6 +764,8 @@ normalize_atom (tree t, tree args, norm_info info)
> > tree ci = build_tree_list (t, info.context);
> > tree atom = build1 (ATOMIC_CONSTR, ci, map);
> > + if (info.in_decl && concept_definition_p (info.in_decl))
> > + ATOMIC_CONSTR_EXPR_FROM_CONCEPT_P (atom) = true;
>
> I'm a bit nervous about relying on in_decl, given that we support normalizing
> when it isn't set; I don't remember the circumstances for that. Maybe make
> the flag indicate that ctx_parms had depth 1?
in_decl gets reliably updated by norm_info::update_context whenever we
recurse inside a concept-id during normalization. And I think the only
other situation we have to worry about is when starting out with a
concept-id, which is handled by normalize_concept_definition where we
also set in_decl appropriately.
AFAICT, in_decl is not set (at the start) only when normalizing a
placeholder type constraint or nested-requirement, and from some
subsumption entrypoints. And we shouldn't see an atom that belongs to a
concept in these cases unless we recurse into a concept-id, in which
case norm_info::update_context will update in_decl appropriately.
So IMHO it should be safe to rely on in_decl here to detect if the atom
belongs to a concept, at least given the current entrypoints to
subsumption/satisfaction..
>
> > if (!info.generate_diagnostics ())
> > {
> > /* Cache the ATOMIC_CONSTRs that we return, so that
> > sat_hasher::equal
> > @@ -2826,33 +2828,37 @@ satisfaction_value (tree t)
> > return boolean_true_node;
> > }
> > -/* Build a new template argument list with template arguments
> > corresponding
> > - to the parameters used in an atomic constraint. */
> > +/* Build a new template argument vector according to the parameter
> > + mapping of the atomic constraint T, using arguments from ARGS. */
> > -tree
> > -get_mapped_args (tree map)
> > +static tree
> > +get_mapped_args (tree t, tree args)
> > {
> > + tree map = ATOMIC_CONSTR_MAP (t);
> > +
> > /* No map, no arguments. */
> > if (!map)
> > return NULL_TREE;
> > - /* Find the mapped parameter with the highest level. */
> > - int count = 0;
> > - for (tree p = map; p; p = TREE_CHAIN (p))
> > - {
> > - int level;
> > - int index;
> > - template_parm_level_and_index (TREE_VALUE (p), &level, &index);
> > - if (level > count)
> > - count = level;
> > - }
> > + /* Determine the depth of the resulting argument vector. */
> > + int depth;
> > + if (ATOMIC_CONSTR_EXPR_FROM_CONCEPT_P (t))
> > + /* The expression of this atomic constraint comes from a concept
> > definition,
> > + whose template depth is always one, so the resulting argument vector
> > + will also have depth one. */
> > + depth = 1;
> > + else
> > + /* Otherwise, the expression of this atomic constraint was written in
> > + the context of the constrained entity, whose template depth is that
> > + of ARGS. */
> > + depth = TMPL_ARGS_DEPTH (args);
> > /* Place each argument at its corresponding position in the argument
> > list. Note that the list will be sparse (not all arguments supplied),
> > but instantiation is guaranteed to only use the parameters in the
> > mapping, so null arguments would never be used. */
> > - auto_vec< vec<tree> > lists (count);
> > - lists.quick_grow_cleared (count);
> > + auto_vec< vec<tree> > lists (depth);
> > + lists.quick_grow_cleared (depth);
> > for (tree p = map; p; p = TREE_CHAIN (p))
> > {
> > int level;
> > @@ -2862,12 +2868,12 @@ get_mapped_args (tree map)
> > /* Insert the argument into its corresponding position. */
> > vec<tree> &list = lists[level - 1];
> > if (index >= (int)list.length ())
> > - list.safe_grow_cleared (index + 1, true);
> > + list.safe_grow_cleared (index + 1, /*exact=*/false);
> > list[index] = TREE_PURPOSE (p);
> > }
> > /* Build the new argument list. */
> > - tree args = make_tree_vec (lists.length ());
> > + args = make_tree_vec (lists.length ());
> > for (unsigned i = 0; i != lists.length (); ++i)
> > {
> > vec<tree> &list = lists[i];
> > @@ -2879,6 +2885,16 @@ get_mapped_args (tree map)
> > }
> > SET_NON_DEFAULT_TEMPLATE_ARGS_COUNT (args, 0);
> > + if (TMPL_ARGS_HAVE_MULTIPLE_LEVELS (args)
> > + && TMPL_ARGS_DEPTH (args) == 1)
> > + {
> > + /* Micro-optimization: represent a depth-one argument vector
> > + using a single level. */
> > + tree level = TMPL_ARGS_LEVEL (args, 1);
> > + ggc_free (args);
> > + args = level;
> > + }
> > +
> > return args;
> > }
> > @@ -2933,7 +2949,7 @@ satisfy_atom (tree t, tree args, sat_info info)
> > }
> > /* Rebuild the argument vector from the parameter mapping. */
> > - args = get_mapped_args (map);
> > + args = get_mapped_args (t, args);
> > /* Apply the parameter mapping (i.e., just substitute). */
> > tree expr = ATOMIC_CONSTR_EXPR (t);
> > @@ -2955,7 +2971,7 @@ satisfy_atom (tree t, tree args, sat_info info)
> > if (!same_type_p (TREE_TYPE (result), boolean_type_node))
> > {
> > if (info.noisy ())
> > - diagnose_atomic_constraint (t, map, result, info);
> > + diagnose_atomic_constraint (t, args, result, info);
> > return cache.save (inst_cache.save (error_mark_node));
> > }
> > @@ -2974,7 +2990,7 @@ satisfy_atom (tree t, tree args, sat_info info)
> > }
> > result = satisfaction_value (result);
> > if (result == boolean_false_node && info.diagnose_unsatisfaction_p ())
> > - diagnose_atomic_constraint (t, map, result, info);
> > + diagnose_atomic_constraint (t, args, result, info);
> > return cache.save (inst_cache.save (result));
> > }
> > @@ -3642,11 +3658,10 @@ diagnose_trait_expr (tree expr, tree args)
> > }
> > }
> > -/* Diagnose a substitution failure in the atomic constraint T when
> > applied
> > - with the instantiated parameter mapping MAP. */
> > +/* Diagnose a substitution failure in the atomic constraint T using ARGS.
> > */
> > static void
> > -diagnose_atomic_constraint (tree t, tree map, tree result, sat_info info)
> > +diagnose_atomic_constraint (tree t, tree args, tree result, sat_info info)
> > {
> > /* If the constraint is already ill-formed, we've previously diagnosed
> > the reason. We should still say why the constraints aren't satisfied.
> > */
> > @@ -3667,7 +3682,6 @@ diagnose_atomic_constraint (tree t, tree map, tree
> > result, sat_info info)
> > /* Generate better diagnostics for certain kinds of expressions. */
> > tree expr = ATOMIC_CONSTR_EXPR (t);
> > STRIP_ANY_LOCATION_WRAPPER (expr);
> > - tree args = get_mapped_args (map);
> > switch (TREE_CODE (expr))
> > {
> > case TRAIT_EXPR:
> > diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
> > index f253b32c3f2..dc2429a8406 100644
> > --- a/gcc/cp/cp-tree.h
> > +++ b/gcc/cp/cp-tree.h
> > @@ -466,6 +466,7 @@ extern GTY(()) tree cp_global_trees[CPTI_MAX];
> > IMPLICIT_CONV_EXPR_NONTYPE_ARG (in IMPLICIT_CONV_EXPR)
> > BASELINK_FUNCTIONS_MAYBE_INCOMPLETE_P (in BASELINK)
> > BIND_EXPR_VEC_DTOR (in BIND_EXPR)
> > + ATOMIC_CONSTR_EXPR_FROM_CONCEPT_P (in ATOMIC_CONSTR)
> > 2: IDENTIFIER_KIND_BIT_2 (in IDENTIFIER_NODE)
> > ICS_THIS_FLAG (in _CONV)
> > DECL_INITIALIZED_BY_CONSTANT_EXPRESSION_P (in VAR_DECL)
> > @@ -1679,6 +1680,11 @@ check_constraint_info (tree t)
> > #define ATOMIC_CONSTR_MAP_INSTANTIATED_P(NODE) \
> > TREE_LANG_FLAG_0 (ATOMIC_CONSTR_CHECK (NODE))
> > +/* Whether the expression for this atomic constraint belongs to a
> > + concept definition. */
> > +#define ATOMIC_CONSTR_EXPR_FROM_CONCEPT_P(NODE) \
> > + TREE_LANG_FLAG_1 (ATOMIC_CONSTR_CHECK (NODE))
> > +
> > /* The expression of an atomic constraint. */
> > #define ATOMIC_CONSTR_EXPR(NODE) \
> > CONSTR_EXPR (ATOMIC_CONSTR_CHECK (NODE))
> > @@ -8306,7 +8312,6 @@ extern tree evaluate_requires_expr
> > (tree);
> > extern tree tsubst_constraint (tree, tree,
> > tsubst_flags_t, tree);
> > extern tree tsubst_constraint_info (tree, tree,
> > tsubst_flags_t, tree);
> > extern tree tsubst_parameter_mapping (tree, tree,
> > tsubst_flags_t, tree);
> > -extern tree get_mapped_args (tree);
> > struct processing_constraint_expression_sentinel
> > {
> > diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-return-req4.C
> > b/gcc/testsuite/g++.dg/cpp2a/concepts-return-req4.C
> > new file mode 100644
> > index 00000000000..471946bc8eb
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/cpp2a/concepts-return-req4.C
> > @@ -0,0 +1,24 @@
> > +// PR c++/104527
> > +// { dg-do compile { target c++20 } }
> > +
> > +template<class T, class U>
> > +concept is_same = __is_same(T, U);
> > +
> > +template<class T>
> > +struct A {
> > + template<class...>
> > + requires requires { { 0 } -> is_same<T>; }
> > + struct B {};
> > +
> > + template<class...>
> > + requires requires { { 1 } -> is_same<T>; }
> > + static void f();
> > +};
> > +
> > +A<int>::B<> a1;
> > +A<bool>::B<> a2; // { dg-error "constraint" }
> > +
> > +int main() {
> > + A<int>::f();
> > + A<bool>::f(); // { dg-error "no match" }
> > +}
>
>
