Re: [PATCH] c++: Distinguish unsatisfaction vs errors during satisfaction [PR97093]
On Thu, 3 Dec 2020, Jason Merrill wrote: > On 12/3/20 9:24 AM, Patrick Palka wrote: > > During satisfaction, the flag info.noisy() controls three things: > > whether to diagnose fatal errors (such as the satisfaction value of an > > atom being non-bool); whether to diagnose unsatisfaction; and whether to > > bypass the satisfaction cache. > > > > This flag turns out to be too coarse however, for sometimes we need to > > diagnose fatal errors but not unsatisfaction, in particular when replaying > > an erroneous satisfaction result from constraint_satisfaction_value, > > evaluate_concept_check and tsubst_nested_requirement. > > > > And we sometimes need to bypass the satisfaction cache but not diagnose > > unsatisfaction, in particular when evaluating the branches of a > > disjunction when info.noisy() is true. Currently, satisfy_disjunction > > first quietly evaluates each branch, but doing so causes satisfy_atom > > to insert re-normalized atoms into the satisfaction cache when > > diagnosing unsatisfaction of the overall constraint. This is ultimately > > the source of PR97093. > > > > To that end, this patch adds the info.diagnose_unsatisfaction_p() flag > > which refines the info.noisy() flag. During satisfaction info.noisy() > > now controls whether to diagnose fatal errors, and > > info.diagnose_unsatisfaction_p() controls whether to additionally > > diagnose unsatisfaction. This enables us to address the above two > > issues straightforwardly. > > > This flag refinement also allows us to fold the diagnose_foo_requirement > > routines into the corresponding tsubst_foo_requirement ones. Here, the > > flags take on slightly different meanings: info.noisy() controls whether > > to diagnose invalid types and expressions inside the requires-expression, > > and info.diagnose_unsatisfaction_p() controls whether to diagnose the > > overall unsatisfaction of the requires-expression. > > > Bootstrapped and regtested on x86_64-pc-linux-gnu, and also tested on > > cmcstl2 and range-v3. Does this look OK for trunk? > > > > gcc/cp/ChangeLog: > > > > PR c++/97093 > > * constraint.cc (struct sat_info): Define. > > (tsubst_valid_expression_requirement): Take a sat_info instead > > of subst_info. Perform the substitution quietly first. Fold in > > error-replaying code from diagnose_valid_expression. > > (tsubst_simple_requirement): Take a sat_info instead of > > subst_info. > > (tsubst_type_requirement_1): New. Fold in error-replaying code > > from diagnose_valid_type. > > (tsubst_type_requirement): Use it. Take a sat_info instead of > > subst_info. > > (tsubst_compound_requirement): Likewise. Fold in > > error-replaying code from diagnose_compound_requirement. > > (tsubst_nested_requirement): Take a sat_info instead of > > subst_info. Perform the substitution quietly first. Fold in > > error-replaying code from diagnose_nested_requirement. > > (tsubst_requirement): Take a sat_info instead of subst_info. > > (tsubst_requirement_body): Likewise. > > (tsubst_requires_expr): Split into two versions, one that takes > > a sat_info argument and another that takes a complain and > > in_decl argument. Remove outdated documentation. Document he > > effects of the sat_info argument. > > (tsubst_parameter_mapping): Take a sat_info instead of a > > subst_info. > > (satisfy_conjunction): Likewise. > > (satisfy_disjunction): Likewise. Evaluate each branch with > > unsatisfaction diagnostics disabled rather than completely > > quietly, and short circuit when an erroneous branch is > > encountered. > > (satisfy_atom): Take a sat_info instead of a subst_info. Fix a > > comment. Use diagnose_unsatisfaction_p() instead of noisy() to > > guard replaying of satisfaction failure. Always check > > constantness quietly first and consistently return > > error_mark_node when the value is non-constant. > > (satisfy_constraint_r): Document the effects of the sat_info > > argument. Take a sat_info instead of a subst_info. > > (satisfy_constraint): Take a sat_info instead of a subst_info. > > (satisfy_associated_constraints): Likewise. > > (satisfy_constraint_expression): Likewise. > > (satisfy_declaration_constraints): Likewise. > > (constraint_satisfaction_value): Likewise. Adjust. XXX > > (constraints_satisfied_p): Adjust. > > (evaluate_concept_check): Adjust. > > (diagnose_trait_expr): Make static. Take a template args vector > > instead of a parameter mapping. > > (diagnose_atomic_constraint): Take a sat_info instead of a > > subst_info. Adjust call to diagnose_trait_expr. Call > > tsubst_requires_expr instead of diagnose_requires_expr. > > (diagnose_constraints): Adjust calls to > > constraint_satisfaction_value. > > (diagnose_valid_expression): Remove. > > (diagnose_valid_type): Likewise. > > (diagnos
Re: [PATCH] c++: Distinguish unsatisfaction vs errors during satisfaction [PR97093]
On 12/3/20 9:24 AM, Patrick Palka wrote: During satisfaction, the flag info.noisy() controls three things: whether to diagnose fatal errors (such as the satisfaction value of an atom being non-bool); whether to diagnose unsatisfaction; and whether to bypass the satisfaction cache. This flag turns out to be too coarse however, for sometimes we need to diagnose fatal errors but not unsatisfaction, in particular when replaying an erroneous satisfaction result from constraint_satisfaction_value, evaluate_concept_check and tsubst_nested_requirement. And we sometimes need to bypass the satisfaction cache but not diagnose unsatisfaction, in particular when evaluating the branches of a disjunction when info.noisy() is true. Currently, satisfy_disjunction first quietly evaluates each branch, but doing so causes satisfy_atom to insert re-normalized atoms into the satisfaction cache when diagnosing unsatisfaction of the overall constraint. This is ultimately the source of PR97093. To that end, this patch adds the info.diagnose_unsatisfaction_p() flag which refines the info.noisy() flag. During satisfaction info.noisy() now controls whether to diagnose fatal errors, and info.diagnose_unsatisfaction_p() controls whether to additionally diagnose unsatisfaction. This enables us to address the above two issues straightforwardly. This flag refinement also allows us to fold the diagnose_foo_requirement routines into the corresponding tsubst_foo_requirement ones. Here, the flags take on slightly different meanings: info.noisy() controls whether to diagnose invalid types and expressions inside the requires-expression, and info.diagnose_unsatisfaction_p() controls whether to diagnose the overall unsatisfaction of the requires-expression. Bootstrapped and regtested on x86_64-pc-linux-gnu, and also tested on cmcstl2 and range-v3. Does this look OK for trunk? gcc/cp/ChangeLog: PR c++/97093 * constraint.cc (struct sat_info): Define. (tsubst_valid_expression_requirement): Take a sat_info instead of subst_info. Perform the substitution quietly first. Fold in error-replaying code from diagnose_valid_expression. (tsubst_simple_requirement): Take a sat_info instead of subst_info. (tsubst_type_requirement_1): New. Fold in error-replaying code from diagnose_valid_type. (tsubst_type_requirement): Use it. Take a sat_info instead of subst_info. (tsubst_compound_requirement): Likewise. Fold in error-replaying code from diagnose_compound_requirement. (tsubst_nested_requirement): Take a sat_info instead of subst_info. Perform the substitution quietly first. Fold in error-replaying code from diagnose_nested_requirement. (tsubst_requirement): Take a sat_info instead of subst_info. (tsubst_requirement_body): Likewise. (tsubst_requires_expr): Split into two versions, one that takes a sat_info argument and another that takes a complain and in_decl argument. Remove outdated documentation. Document he effects of the sat_info argument. (tsubst_parameter_mapping): Take a sat_info instead of a subst_info. (satisfy_conjunction): Likewise. (satisfy_disjunction): Likewise. Evaluate each branch with unsatisfaction diagnostics disabled rather than completely quietly, and short circuit when an erroneous branch is encountered. (satisfy_atom): Take a sat_info instead of a subst_info. Fix a comment. Use diagnose_unsatisfaction_p() instead of noisy() to guard replaying of satisfaction failure. Always check constantness quietly first and consistently return error_mark_node when the value is non-constant. (satisfy_constraint_r): Document the effects of the sat_info argument. Take a sat_info instead of a subst_info. (satisfy_constraint): Take a sat_info instead of a subst_info. (satisfy_associated_constraints): Likewise. (satisfy_constraint_expression): Likewise. (satisfy_declaration_constraints): Likewise. (constraint_satisfaction_value): Likewise. Adjust. XXX (constraints_satisfied_p): Adjust. (evaluate_concept_check): Adjust. (diagnose_trait_expr): Make static. Take a template args vector instead of a parameter mapping. (diagnose_atomic_constraint): Take a sat_info instead of a subst_info. Adjust call to diagnose_trait_expr. Call tsubst_requires_expr instead of diagnose_requires_expr. (diagnose_constraints): Adjust calls to constraint_satisfaction_value. (diagnose_valid_expression): Remove. (diagnose_valid_type): Likewise. (diagnose_simple_requirement): Likewise. (diagnose_compound_requirement): Likewise. (diagnose_type_requirement): Likewise. (diagnose_nested_requirement): Likewise.
[PATCH] c++: Distinguish unsatisfaction vs errors during satisfaction [PR97093]
During satisfaction, the flag info.noisy() controls three things: whether to diagnose fatal errors (such as the satisfaction value of an atom being non-bool); whether to diagnose unsatisfaction; and whether to bypass the satisfaction cache. This flag turns out to be too coarse however, for sometimes we need to diagnose fatal errors but not unsatisfaction, in particular when replaying an erroneous satisfaction result from constraint_satisfaction_value, evaluate_concept_check and tsubst_nested_requirement. And we sometimes need to bypass the satisfaction cache but not diagnose unsatisfaction, in particular when evaluating the branches of a disjunction when info.noisy() is true. Currently, satisfy_disjunction first quietly evaluates each branch, but doing so causes satisfy_atom to insert re-normalized atoms into the satisfaction cache when diagnosing unsatisfaction of the overall constraint. This is ultimately the source of PR97093. To that end, this patch adds the info.diagnose_unsatisfaction_p() flag which refines the info.noisy() flag. During satisfaction info.noisy() now controls whether to diagnose fatal errors, and info.diagnose_unsatisfaction_p() controls whether to additionally diagnose unsatisfaction. This enables us to address the above two issues straightforwardly. This flag refinement also allows us to fold the diagnose_foo_requirement routines into the corresponding tsubst_foo_requirement ones. Here, the flags take on slightly different meanings: info.noisy() controls whether to diagnose invalid types and expressions inside the requires-expression, and info.diagnose_unsatisfaction_p() controls whether to diagnose the overall unsatisfaction of the requires-expression. Bootstrapped and regtested on x86_64-pc-linux-gnu, and also tested on cmcstl2 and range-v3. Does this look OK for trunk? gcc/cp/ChangeLog: PR c++/97093 * constraint.cc (struct sat_info): Define. (tsubst_valid_expression_requirement): Take a sat_info instead of subst_info. Perform the substitution quietly first. Fold in error-replaying code from diagnose_valid_expression. (tsubst_simple_requirement): Take a sat_info instead of subst_info. (tsubst_type_requirement_1): New. Fold in error-replaying code from diagnose_valid_type. (tsubst_type_requirement): Use it. Take a sat_info instead of subst_info. (tsubst_compound_requirement): Likewise. Fold in error-replaying code from diagnose_compound_requirement. (tsubst_nested_requirement): Take a sat_info instead of subst_info. Perform the substitution quietly first. Fold in error-replaying code from diagnose_nested_requirement. (tsubst_requirement): Take a sat_info instead of subst_info. (tsubst_requirement_body): Likewise. (tsubst_requires_expr): Split into two versions, one that takes a sat_info argument and another that takes a complain and in_decl argument. Remove outdated documentation. Document he effects of the sat_info argument. (tsubst_parameter_mapping): Take a sat_info instead of a subst_info. (satisfy_conjunction): Likewise. (satisfy_disjunction): Likewise. Evaluate each branch with unsatisfaction diagnostics disabled rather than completely quietly, and short circuit when an erroneous branch is encountered. (satisfy_atom): Take a sat_info instead of a subst_info. Fix a comment. Use diagnose_unsatisfaction_p() instead of noisy() to guard replaying of satisfaction failure. Always check constantness quietly first and consistently return error_mark_node when the value is non-constant. (satisfy_constraint_r): Document the effects of the sat_info argument. Take a sat_info instead of a subst_info. (satisfy_constraint): Take a sat_info instead of a subst_info. (satisfy_associated_constraints): Likewise. (satisfy_constraint_expression): Likewise. (satisfy_declaration_constraints): Likewise. (constraint_satisfaction_value): Likewise. Adjust. XXX (constraints_satisfied_p): Adjust. (evaluate_concept_check): Adjust. (diagnose_trait_expr): Make static. Take a template args vector instead of a parameter mapping. (diagnose_atomic_constraint): Take a sat_info instead of a subst_info. Adjust call to diagnose_trait_expr. Call tsubst_requires_expr instead of diagnose_requires_expr. (diagnose_constraints): Adjust calls to constraint_satisfaction_value. (diagnose_valid_expression): Remove. (diagnose_valid_type): Likewise. (diagnose_simple_requirement): Likewise. (diagnose_compound_requirement): Likewise. (diagnose_type_requirement): Likewise. (diagnose_nested_requirement): Likewise. (diagnose_requirement): Likewise.
