On 2/10/21 9:41 AM, Patrick Palka wrote:
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.
True. And if we change REQUIRES_EXPR handling to not go through here, it should be just the two. OK, let's leave this alone.
++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.
Sounds good.
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_checkSounds good. Some ideas: satisfy_constraint could be renamed to something like satisfy_normalized_constraint or satisfy_normal_form.
Let's go with the former.
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.
Yes, it seems like the callers should check for dependent template args sooner, before normalization.
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).
nested-requirements, placeholders, requires-expressions, and concept checks. A set of things that (now) carry all the information they need for their own normalization. Unless it's ever called for a concept check with a non-null args argument. Maybe let's move the null args test to an assert inside the concept-check case.
The one-parameter version of satisfy_constraint_expression has just one caller, and this use could be replaced with tsubst_requires_expr I think.
Agreed; a requires-expression is already atomic, it isn't useful to normalize it, and better not to clutter up the hash table with them.
Incidentally, though, I don't mind overloads like this that just forward to the main implementation. The ones that bother me are the ones that duplicate a lot of the code, like constraint_satisfaction_value.
It seems we could straightforwardly merge the two constraint_satisfaction_value overloads, which would also let us merge the constraints_satisfied_p overloads.
Sounds good.
We could also merge tsubst_requires_expr routines with the diagnose_requires_expr ones.
Sounds good.
/* 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))
