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