On Tue, 9 Feb 2021, Jason Merrill wrote:
> On 2/8/21 2:03 PM, Patrick Palka wrote:
> > This sets up the functionality for controlling the initial set of
> > template parameters to pass to normalization when dealing with a
> > constraint-expression that is not associated with some constrained
> > declaration, for instance when normalizing a nested requirement of a
> > requires expression, or the constraints on a placeholder type.
> >
> > The main new ingredient here is the data member norm_info::initial_parms
> > which can be set by callers of the normalization routines to communicate
> > the in-scope template parameters for the supplied constraint-expression,
> > rather than always falling back to using current_template_parms.
> >
> > This patch then uses this functionality in our handling of nested
> > requirements so that we can delay normalizing them until needed for
> > satisfaction. We currently immediately normalize nested requirements at
> > parse time, where we have the necessary template context, and cache the
> > normal form in their TREE_TYPE node. With this patch, we now delay
> > normalization until needed (as with other constraint expressions), and
> > instead store the current value of current_template_parms in their
> > TREE_TYPE node (which we use to restore the template context at
> > normalization time).
> >
> > In the subsequent patch, this functionality will also be used to
> > normalize placeholder type constraints during auto deduction.
> >
> > gcc/cp/ChangeLog:
> >
> > * constraint.cc (build_parameter_mapping): Rely on the caller to
> > determine the in-scope template parameters.
> > (norm_info::norm_info): Delegate the one-parameter constructor
> > to the two-parameter constructor. In the two-parameter
> > constructor, fold in the definition of make_context, set
> > initial_parms appropriately, and don't set the now-removed
> > orig_decl member.
> > (norm_info::make_context): Remove, now that its only use is
> > inlined into the caller.
> > (norm_info::update_context): Adjust call to
> > build_parameter_mapping to pass in the relevant set of in-scope
> > template parameters.
> > (norm_info::ctx_parms): Define this member function.
> > (norm_info::context): Initialize to NULL_TREE.
> > (norm_info::orig_decl): Remove this data member.
> > (norm_info::initial_parms): Define this data member.
> > (normalize_atom): Adjust call to build_parameter_mapping to pass
> > in the relevant set of in-scope template parameters. Use
> > info.initial_parms instead of info.orig_decl.
> > (normalize_constraint_expression): Define an overload that takes
> > a norm_info object. Cache the result of normalization. Define
> > the other overload in terms of this one, and handle a NESTED_REQ
> > argument by setting info.initial_parms appropriately.
> > (tsubst_nested_requirement): Go through
> > satisfy_constraint_expression so that we normalize on demand.
> > (finish_nested_requirement): Set the TREE_TYPE of the NESTED_REQ
> > to current_template_parms.
> > (diagnose_nested_requirements): Go through
> > satisfy_constraint_expression, as with tsubst_nested_requirement.
> > ---
> > gcc/cp/constraint.cc | 140 +++++++++++++++++++++++--------------------
> > gcc/cp/cp-tree.h | 4 +-
> > 2 files changed, 78 insertions(+), 66 deletions(-)
> >
> > diff --git a/gcc/cp/constraint.cc b/gcc/cp/constraint.cc
> > index 39c97986082..56134f8b2bf 100644
> > --- a/gcc/cp/constraint.cc
> > +++ b/gcc/cp/constraint.cc
> > @@ -133,7 +133,7 @@ struct sat_info : subst_info
> > bool diagnose_unsatisfaction;
> > };
> > -static tree satisfy_constraint (tree, tree, sat_info);
> > +static tree satisfy_constraint_expression (tree, tree, sat_info);
> > /* True if T is known to be some type other than bool. Note that this
> > is false for dependent types and errors. */
> > @@ -594,26 +594,12 @@ map_arguments (tree parms, tree args)
> > return parms;
> > }
> > -/* Build the parameter mapping for EXPR using ARGS. */
> > +/* Build the parameter mapping for EXPR using ARGS, where CTX_PARMS
> > + are the template parameters in scope for EXPR. */
> > static tree
> > -build_parameter_mapping (tree expr, tree args, tree decl)
> > +build_parameter_mapping (tree expr, tree args, tree ctx_parms)
> > {
> > - tree ctx_parms = NULL_TREE;
> > - if (decl)
> > - {
> > - gcc_assert (TREE_CODE (decl) == TEMPLATE_DECL);
> > - ctx_parms = DECL_TEMPLATE_PARMS (decl);
> > - }
> > - else if (current_template_parms)
> > - {
> > - /* TODO: This should probably be the only case, but because the
> > - point of declaration of concepts is currently set after the
> > - initializer, the template parameter lists are not available
> > - when normalizing concept definitions, hence the case above. */
> > - ctx_parms = current_template_parms;
> > - }
> > -
> > tree parms = find_template_parameters (expr, ctx_parms);
> > tree map = map_arguments (parms, args);
> > return map;
> > @@ -645,53 +631,63 @@ parameter_mapping_equivalent_p (tree t1, tree t2)
> > struct norm_info : subst_info
> > {
> > - explicit norm_info (tsubst_flags_t complain)
> > - : subst_info (tf_warning_or_error | complain, NULL_TREE),
> > - context()
> > + explicit norm_info (tsubst_flags_t cmp)
> > + : norm_info (NULL_TREE, cmp)
> > {}
> > /* Construct a top-level context for DECL. */
> > norm_info (tree in_decl, tsubst_flags_t complain)
> > - : subst_info (tf_warning_or_error | complain, in_decl),
> > - context (make_context (in_decl)),
> > - orig_decl (in_decl)
> > - {}
> > -
> > - bool generate_diagnostics() const
> > + : subst_info (tf_warning_or_error | complain, in_decl)
> > {
> > - return complain & tf_norm;
> > + if (in_decl)
> > + {
> > + initial_parms = DECL_TEMPLATE_PARMS (in_decl);
> > + if (generate_diagnostics ())
> > + context = build_tree_list (NULL_TREE, in_decl);
> > + }
> > + else
> > + initial_parms = current_template_parms;
> > }
> > - tree make_context(tree in_decl)
> > + bool generate_diagnostics() const
> > {
> > - if (generate_diagnostics ())
> > - return build_tree_list (NULL_TREE, in_decl);
> > - return NULL_TREE;
> > + return complain & tf_norm;
> > }
> > void update_context(tree expr, tree args)
> > {
> > if (generate_diagnostics ())
> > {
> > - tree map = build_parameter_mapping (expr, args, in_decl);
> > + tree map = build_parameter_mapping (expr, args, ctx_parms ());
> > context = tree_cons (map, expr, context);
> > }
> > in_decl = get_concept_check_template (expr);
> > }
> > + /* Returns the template parameters that are in scope for the current
> > + normalization context. */
> > +
> > + tree ctx_parms()
> > + {
> > + if (in_decl)
> > + return DECL_TEMPLATE_PARMS (in_decl);
> > + else
> > + return initial_parms;
> > + }
> > +
> > /* Provides information about the source of a constraint. This is a
> > TREE_LIST whose VALUE is either a concept check or a constrained
> > declaration. The PURPOSE, for concept checks is a parameter mapping
> > for that check. */
> > - tree context;
> > + tree context = NULL_TREE;
> > /* The declaration whose constraints we're normalizing. The targets
> > of the parameter mapping of each atom will be in terms of the
> > template parameters of ORIG_DECL. */
> > - tree orig_decl = NULL_TREE;
> > + tree initial_parms = NULL_TREE;
> > };
> > static tree normalize_expression (tree, tree, norm_info);
> > @@ -773,7 +769,7 @@ normalize_atom (tree t, tree args, norm_info info)
> > return normalize_concept_check (t, args, info);
> > /* Build the parameter mapping for the atom. */
> > - tree map = build_parameter_mapping (t, args, info.in_decl);
> > + tree map = build_parameter_mapping (t, args, info.ctx_parms ());
> > /* Build a new info object for the atom. */
> > tree ci = build_tree_list (t, info.context);
> > @@ -803,10 +799,8 @@ normalize_atom (tree t, tree args, norm_info info)
> > tree target = TREE_PURPOSE (node);
> > TREE_VEC_ELT (targets, i++) = target;
> > }
> > - tree ctx_parms = (info.orig_decl
> > - ? DECL_TEMPLATE_PARMS (info.orig_decl)
> > - : current_template_parms);
> > - tree target_parms = find_template_parameters (targets, ctx_parms);
> > + tree target_parms = find_template_parameters (targets,
> > + info.initial_parms);
> > TREE_TYPE (map) = target_parms;
> > }
> > @@ -983,17 +977,43 @@ normalize_nontemplate_requirements (tree decl, bool
> > diag = false)
> > /* Normalize an EXPR as a constraint. */
> > static tree
> > -normalize_constraint_expression (tree expr, bool diag)
> > +normalize_constraint_expression (tree expr, norm_info info)
> > {
> > if (!expr || expr == error_mark_node)
> > return expr;
> > +
> > + if (!info.generate_diagnostics ())
> > + if (tree *p = hash_map_safe_get (normalized_map, expr))
> > + return *p;
>
> It seems like we only want this for NESTED_REQ.
I figured it'd also be beneficial to cache the normal form of a
placeholder type constraint, which will also goes through this overload.
I haven't done any careful benchmarking though.
>
> > ++processing_template_decl;
> > - norm_info info (diag ? tf_norm : tf_none);
> > tree norm = get_normalized_constraints (expr, info);
> > --processing_template_decl;
> > +
> > + if (!info.generate_diagnostics ())
> > + hash_map_safe_put<hm_ggc> (normalized_map, expr, norm);
> > +
> > return norm;
> > }
> > +/* High-level wrapper for the above. */
> > +
> > +static tree
> > +normalize_constraint_expression (tree expr, bool diag)
> > +{
> > + norm_info info (diag ? tf_norm : tf_none);
>
> I wonder if we want to add a norm_info constructor taking a sat_info so we
> don't need to mediate passing from one into the other with bool "diag"
> parameters in various functions. That doesn't need to happen in this patch.
Sounds good. I think such a constructor would let us eliminate the
bool-taking overload of normalize_constraint_expression altogether, if
we move the special handling of NESTED_REQ to the norm_info-taking
overload or to satisfy_constraint_expression.
>
> > + if (TREE_CODE (expr) == NESTED_REQ)
> > + {
> > + /* The TREE_TYPE contains the set of template parameters that were
> > + in scope for this nested requiremen; use them as the initial template
> > + parameters for normalization. */
> > + info.initial_parms = TREE_TYPE (expr);
> > + return normalize_constraint_expression (TREE_OPERAND (expr, 0),
> > info);
> > + }
> > + else
> > + return normalize_constraint_expression (expr, info);
> > +}
>
> It seems like you don't want any other code to call the first overload, so
> let's give the first one a different name to make that clearer. The
> differences in functionality don't follow naturally from the parameter types.
In the next patch, a new function normalize_placeholder_type_constraints
will also call this norm_info-taking overload. But I think we could
remove the bool-taking overload at least, as mentioned above.
>
> Maybe this year we can cull/refactor the confusing set of overlapping and
> ambiguously named functions that the concepts code already has:
>
> satisfy_constraint
> satisfy_associated_constraints
> satisfy_constraint_expression x2
> constraint_satisfaction_value x2
> constraints_satisfied_p x2
> satisfy_declaration_constraints x2
> evaluate_concept_check
Sounds good. Some ideas:
satisfy_constraint could be renamed to something like
satisfy_normalized_constraint or satisfy_normal_form.
satisfy_associated_constraints is just a small wrapper over
satisfy_constraint, so we could remove it and have its two callers could
check for type-dependence of the supplied template arguments themselves.
satisfy_constraint_expression should probably be renamed to something
like satisfy_nondeclaration_constraints since it'd now handle more than
just constraint-expressions (e.g. NESTED_REQs and placeholder types).
The one-parameter version of satisfy_constraint_expression has just one
caller, and this use could be replaced with tsubst_requires_expr I think.
It seems we could straightforwardly merge the two
constraint_satisfaction_value overloads, which would also let us merge
the constraints_satisfied_p overloads.
We could also merge tsubst_requires_expr routines with the
diagnose_requires_expr ones.
>
> > /* 17.4.1.2p2. Two constraints are identical if they are formed
> > from the same expression and the targets of the parameter mapping
> > are equivalent. */
> > @@ -2086,16 +2106,14 @@ tsubst_compound_requirement (tree t, tree args,
> > subst_info info)
> > static tree
> > tsubst_nested_requirement (tree t, tree args, subst_info info)
> > {
> > - /* Perform satisfaction quietly with the regular normal form. */
> > + /* Perform satisfaction quietly first. */
> > sat_info quiet (tf_none, info.in_decl);
> > - tree norm = TREE_VALUE (TREE_TYPE (t));
> > - tree diag_norm = TREE_PURPOSE (TREE_TYPE (t));
> > - tree result = satisfy_constraint (norm, args, quiet);
> > + tree result = satisfy_constraint_expression (t, args, quiet);
> > if (result == error_mark_node)
> > {
> > - /* Replay the error using the diagnostic normal form. */
> > + /* Replay the error. */
> > sat_info noisy (tf_warning_or_error, info.in_decl);
> > - satisfy_constraint (diag_norm, args, noisy);
> > + satisfy_constraint_expression (t, args, noisy);
> > }
> > if (result != boolean_true_node)
> > return error_mark_node;
> > @@ -3301,15 +3319,9 @@ finish_compound_requirement (location_t loc, tree
> > expr, tree type, bool noexcept
> > tree
> > finish_nested_requirement (location_t loc, tree expr)
> > {
> > - /* We need to normalize the constraints now, at parse time, while
> > - we have the necessary template context. We normalize twice,
> > - once without diagnostic information and once with, which we'll
> > - later use for quiet and noisy satisfaction respectively. */
> > - tree norm = normalize_constraint_expression (expr, /*diag=*/false);
> > - tree diag_norm = normalize_constraint_expression (expr, /*diag=*/true);
> > -
> > - /* Build the constraint, saving its two normalizations as its type. */
> > - tree r = build1 (NESTED_REQ, build_tree_list (diag_norm, norm), expr);
> > + /* Build the requirement, saving the set of in-scope template
> > + parameters as its type. */
> > + tree r = build1 (NESTED_REQ, current_template_parms, expr);
> > SET_EXPR_LOCATION (r, loc);
> > return r;
> > }
> > @@ -3710,12 +3722,9 @@ diagnose_type_requirement (tree req, tree args, tree
> > in_decl)
> > static void
> > diagnose_nested_requirement (tree req, tree args)
> > {
> > - /* Quietly check for satisfaction first using the regular normal form.
> > - We can elaborate details later if needed. */
> > - tree norm = TREE_VALUE (TREE_TYPE (req));
> > - tree diag_norm = TREE_PURPOSE (TREE_TYPE (req));
> > - sat_info info (tf_none, NULL_TREE);
> > - tree result = satisfy_constraint (norm, args, info);
> > + /* Quietly check for satisfaction first. */
> > + sat_info quiet (tf_none, NULL_TREE);
> > + tree result = satisfy_constraint_expression (req, args, quiet);
> > if (result == boolean_true_node)
> > return;
> > @@ -3723,10 +3732,11 @@ diagnose_nested_requirement (tree req, tree args)
> > location_t loc = cp_expr_location (expr);
> > if (diagnosing_failed_constraint::replay_errors_p ())
> > {
> > - /* Replay the substitution error using the diagnostic normal form.
> > */
> > + /* Replay the substitution error with re-normalized requirements. */
> > inform (loc, "nested requirement %qE is not satisfied, because",
> > expr);
> > +
> > sat_info noisy (tf_warning_or_error, NULL_TREE,
> > /*diag_unsat=*/true);
> > - satisfy_constraint (diag_norm, args, noisy);
> > + satisfy_constraint_expression (req, args, noisy);
> > }
> > else
> > inform (loc, "nested requirement %qE is not satisfied", expr);
> > diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
> > index 970ed5e77bb..26fbf1eb663 100644
> > --- a/gcc/cp/cp-tree.h
> > +++ b/gcc/cp/cp-tree.h
> > @@ -1587,7 +1587,9 @@ check_constraint_info (tree t)
> > TREE_LANG_FLAG_0 (TREE_CHECK (NODE, COMPOUND_REQ))
> > /* The constraints on an 'auto' placeholder type, used in an argument
> > deduction
> > - constraint. */
> > + constraint. This should usually be set via
> > set_placeholder_type_constraints
> > + since we also need to record the relevant set of in-scope template
> > parameters
> > + for later normalization. */
> > #define PLACEHOLDER_TYPE_CONSTRAINTS(NODE) \
> > DECL_SIZE_UNIT (TYPE_NAME (NODE))
> >
>
>
