| Message ID | 20200309222358.688229-1-ppalka@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | [1/2] c++: Replay errors during diagnosis of constraint satisfaction failures | expand |
On 3/9/20 6:23 PM, Patrick Palka wrote: > This patch adds a new flag -fconcepts-diagnostics-depth to the C++ frontend > which controls how deeply we replay errors when diagnosing a constraint > satisfaction failure. The default is -fconcepts-diagnostics-depth=1 which > diagnoses only the topmost constraint satisfaction failure and is consistent > with our behavior before this patch. By increasing this flag's value, the user > can control how deeply they want the compiler to explain a constraint > satisfaction error. > > For example, if the unsatisfied constraint is a disjunction, then the default > behavior is to just say "no branch in the disjunction is satisfied", but with > -fconcepts-diagnostics-depth=2 we will additionally replay and diagnose the > error in each branch of the disjunction. And if the unsatisfied constraint is a > requires expression, then we will replay the error in the requires expression, > etc. This proceeds recursively until there is nothing more to replay or we > reached the exceeded the maximum depth specified by the flag. > > Implementation wise, this patch essentially just uncomments the existing > commented-out code that performs the error-replaying, adding logic to keep track > of the current replay depth along the way. Besides that, there is a new routine > collect_operands_of_disjunction which flattens a disjunction and collects all of > its operands into a vector. > > Here are some examples of diagnostics with the two patches in this series. > > For the simple test case in which we call ranges::begin() on something that's > not a range: > > #include <ranges> > > struct S { } s; > auto x = std::ranges::begin(s); > > we get the following diagnostics with -fconcepts-diagnostics-depth={1,2,3} > respectively: > > https://pppalka.github.io/ranges-begin-depth-1.html > https://pppalka.github.io/ranges-begin-depth-2.html > https://pppalka.github.io/ranges-begin-depth-3.html > > And for the new test g++.dg/concepts/diagnostic5.C, we get: > > https://pppalka.github.io/diagnostic5-depth-1.html > https://pppalka.github.io/diagnostic5-depth-2.html > https://pppalka.github.io/diagnostic5-depth-3.html > https://pppalka.github.io/diagnostic5-depth-4.html > > The extra diagnostics enabled by this flag are at times longer than they need to > be (e.g. "the operand is_array_v<...> is unsatisfied because \n the expression > is_array_v<...> [with ...] evaluated to false") and not immediately easy to > follow (especially when there are nested disjunctions), but the transparency > provided by these optional diagnostics seems to be pretty helpful in practice. > > Does this seem like a sensible approach? Thoughts and ideas for improvement > welcome. Wording and naming suggestions would be much appreciated. This does seem like a good approach, thanks. > gcc/c-family/ChangeLog: > > * c.opt: Add -fconcepts-diagnostics-depth. > > gcc/cp/ChangeLog: > > * constraint.cc (finish_constraint_binary_op): Set the location of EXPR > as well as its range, because build_x_binary_op doesn't always do so. > (current_constraint_diagnosis_depth): New. > (concepts_diagnostics_max_depth_exceeded_p): New. > (collect_operands_of_disjunction): New. > (satisfy_disjunction): When diagnosing a satisfaction failure, maybe > replay each branch of the disjunction, subject to the current diagnosis > depth. > (diagnose_valid_expression): When diagnosing a satisfaction failure, > maybe replay the substitution error, subject to the current diagnosis > recursion. > (diagnose_valid_type): Likewise. > (diagnose_nested_requiremnet): Likewise. > (diagnosing_failed_constraint::diagnosing_failed_constraint): Increment > current_constraint_diagnosis_depth when diagnosing. > (diagnosing_failed_constraint::~diagnosing_failed_constraint): Decrement > current_constraint_diagnosis_depth when diagnosing. > (diagnose_constraints): Don't diagnose if concepts_diagnostics_max_depth > is 0. Emit a one-off note to increase -fconcepts-diagnostics-depth if > the limit was exceeded. > > gcc/testsuite/ChangeLog: > > * g++.dg/concepts/diagnostic2.C: Expect "no operand" instead of > "neither operand". > * g++.dg/concepts/diagnostic5.C: New test. > --- > gcc/c-family/c.opt | 4 + > gcc/cp/constraint.cc | 146 +++++++++++++++++--- > gcc/testsuite/g++.dg/concepts/diagnostic2.C | 2 +- > gcc/testsuite/g++.dg/concepts/diagnostic5.C | 46 ++++++ > 4 files changed, 177 insertions(+), 21 deletions(-) > create mode 100644 gcc/testsuite/g++.dg/concepts/diagnostic5.C > > diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt > index 1cd585fa71d..97ef488931d 100644 > --- a/gcc/c-family/c.opt > +++ b/gcc/c-family/c.opt > @@ -1453,6 +1453,10 @@ fconcepts-ts > C++ ObjC++ Var(flag_concepts_ts) Init(0) > Enable certain features present in the Concepts TS. > > +fconcepts-diagnostics-depth= > +C++ ObjC++ Joined RejectNegative UInteger Var(concepts_diagnostics_max_depth) Init(1) > +Specify maximum error replay depth during recursive diagnosis of a constraint satisfaction failure. > + > fcond-mismatch > C ObjC C++ ObjC++ > Allow the arguments of the '?' operator to have different types. > diff --git a/gcc/cp/constraint.cc b/gcc/cp/constraint.cc > index 4bb4a3f7252..c5e3d64daa6 100644 > --- a/gcc/cp/constraint.cc > +++ b/gcc/cp/constraint.cc > @@ -162,6 +162,7 @@ finish_constraint_binary_op (location_t loc, > /* When either operand is dependent, the overload set may be non-empty. */ > if (expr == error_mark_node) > return error_mark_node; > + expr.set_location (loc); > expr.set_range (lhs.get_start (), rhs.get_finish ()); > return expr; > } > @@ -2386,6 +2387,50 @@ satisfy_conjunction (tree t, tree args, subst_info info) > return satisfy_constraint_r (TREE_OPERAND (t, 1), args, info); > } > > +/* The current depth at which we're replaying an error during recursive > + diagnosis of a constraint satisfaction failure. */ > + > +static int current_constraint_diagnosis_depth; > + > +/* Whether CURRENT_CONSTRAINT_DIAGNOSIS_DEPTH has ever exceeded > + CONCEPTS_DIAGNOSTICS_MAX_DEPTH during recursive diagnosis of a constraint > + satisfaction error. */ > + > +static bool concepts_diagnostics_max_depth_exceeded_p; > + > +/* Recursive subroutine of collect_operands_of_disjunction. T is a normalized > + subexpression of a constraint (composed of CONJ_CONSTRs and DISJ_CONSTRs) > + and E is the corresponding unnormalized subexpression (composed of > + TRUTH_ANDIF_EXPRs and TRUTH_ORIF_EXPRs). */ > + > +static void > +collect_operands_of_disjunction_r (tree t, tree e, > + auto_vec<std::pair<tree, tree> > *operands) > +{ > + if (TREE_CODE (e) == TRUTH_ORIF_EXPR) > + { > + collect_operands_of_disjunction_r (TREE_OPERAND (t, 0), > + TREE_OPERAND (e, 0), operands); > + collect_operands_of_disjunction_r (TREE_OPERAND (t, 1), > + TREE_OPERAND (e, 1), operands); > + } > + else > + { > + std::pair<tree, tree> p = {t, e}; > + operands->safe_push (p); > + } > +} > + > +/* Recursively collect the normalized and unnormalized operands of the > + disjunction T and append them to OPERANDS in order. */ > + > +static void > +collect_operands_of_disjunction (tree t, > + auto_vec<std::pair<tree, tree> > *operands) > +{ > + collect_operands_of_disjunction_r (t, CONSTR_EXPR (t), operands); > +} > + > /* Compute the satisfaction of a disjunction. */ > > static tree > @@ -2403,11 +2448,27 @@ satisfy_disjunction (tree t, tree args, subst_info info) > tree rhs = satisfy_constraint_r (TREE_OPERAND (t, 1), args, quiet); > if (rhs != boolean_true_node && info.noisy ()) > { > - location_t loc = cp_expr_location (CONSTR_EXPR (t)); > - inform (loc, "neither operand of the disjunction is satisfied"); > - /* TODO: Replay the LHS and RHS to find failures in both branches. */ > - // satisfy_constraint_r (TREE_OPERAND (t, 0), args, info); > - // satisfy_constraint_r (TREE_OPERAND (t, 1), args, info); > + cp_expr disj_expr = CONSTR_EXPR (t); > + inform (disj_expr.get_location (), > + "no operand of the disjunction is satisfied"); > + if (current_constraint_diagnosis_depth < concepts_diagnostics_max_depth) Could we localize all the tracking of current_constraint_diagnosis_depth vs. max_depth and max_depth_exceeded_p in diagnosing_failed_constraint, where you're already doing the increment/decrement? > + { > + /* Replay the error in each branch of the disjunction. */ > + auto_vec<std::pair<tree, tree> > operands; > + collect_operands_of_disjunction (t, &operands); > + for (unsigned i = 0; i < operands.length (); i++) > + { > + tree norm_op = operands[i].first; > + tree op = operands[i].second; > + location_t loc = make_location (cp_expr_location (op), > + disj_expr.get_start (), > + disj_expr.get_finish ()); > + inform (loc, "the operand %qE is unsatisfied because", op); > + satisfy_constraint_r (norm_op, args, info); > + } > + } > + else > + concepts_diagnostics_max_depth_exceeded_p = true; > } > return rhs; > } > @@ -3147,10 +3208,17 @@ diagnose_valid_expression (tree expr, tree args, tree in_decl) > return result; > > location_t loc = cp_expr_loc_or_input_loc (expr); > - inform (loc, "the required expression %qE is invalid", expr); > - > - /* TODO: Replay the substitution to diagnose the error? */ > - // tsubst_expr (expr, args, tf_error, in_decl, false); > + if (current_constraint_diagnosis_depth < concepts_diagnostics_max_depth) > + { > + /* Replay the substitution error. */ > + inform (loc, "the required expression %qE is invalid, because", expr); > + tsubst_expr (expr, args, tf_error, in_decl, false); > + } > + else > + { > + inform (loc, "the required expression %qE is invalid", expr); > + concepts_diagnostics_max_depth_exceeded_p = true; > + } > > return error_mark_node; > } > @@ -3163,10 +3231,17 @@ diagnose_valid_type (tree type, tree args, tree in_decl) > return result; > > location_t loc = cp_expr_loc_or_input_loc (type); > - inform (loc, "the required type %qT is invalid", type); > - > - /* TODO: Replay the substitution to diagnose the error? */ > - // tsubst (type, args, tf_error, in_decl); > + if (current_constraint_diagnosis_depth < concepts_diagnostics_max_depth) > + { > + /* Replay the substitution error. */ > + inform (loc, "the required type %qT is invalid, because", type); > + tsubst (type, args, tf_error, in_decl); > + } > + else > + { > + inform (loc, "the required type %qT is invalid", type); > + concepts_diagnostics_max_depth_exceeded_p = true; > + } > > return error_mark_node; > } > @@ -3245,11 +3320,19 @@ diagnose_nested_requirement (tree req, tree args) > > tree expr = TREE_OPERAND (req, 0); > location_t loc = cp_expr_location (expr); > - inform (loc, "nested requirement %qE is not satisfied", expr); > + if (current_constraint_diagnosis_depth < concepts_diagnostics_max_depth) > + { > + /* Replay the substitution error. */ > + inform (loc, "nested requirement %qE is not satisfied, because", expr); > + subst_info noisy (tf_warning_or_error, NULL_TREE); > + satisfy_constraint_expression (expr, args, noisy); > + } > + else > + { > + inform (loc, "nested requirement %qE is not satisfied", expr); > + concepts_diagnostics_max_depth_exceeded_p = true; > + } > > - /* TODO: Replay the substitution to diagnose the error? */ > - // subst_info noisy (tf_warning_or_error, NULL_TREE); > - // satisfy_constraint (norm, args, info); > } > > static void > @@ -3346,14 +3429,23 @@ diagnosing_failed_constraint (tree t, tree args, bool diag) > : diagnosing_error (diag) > { > if (diagnosing_error) > - current_failed_constraint = tree_cons (args, t, current_failed_constraint); > + { > + current_failed_constraint > + = tree_cons (args, t, current_failed_constraint); > + ++current_constraint_diagnosis_depth; > + } > } > > diagnosing_failed_constraint:: > ~diagnosing_failed_constraint () > { > - if (diagnosing_error && current_failed_constraint) > - current_failed_constraint = TREE_CHAIN (current_failed_constraint); > + if (diagnosing_error) > + { > + --current_constraint_diagnosis_depth; > + if (current_failed_constraint) > + current_failed_constraint = TREE_CHAIN (current_failed_constraint); > + } > + > } > > /* Emit diagnostics detailing the failure ARGS to satisfy the constraints > @@ -3364,11 +3456,25 @@ diagnose_constraints (location_t loc, tree t, tree args) > { > inform (loc, "constraints not satisfied"); > > + if (concepts_diagnostics_max_depth == 0) > + return; > + > /* Replay satisfaction, but diagnose errors. */ > if (!args) > constraint_satisfaction_value (t, tf_warning_or_error); > else > constraint_satisfaction_value (t, args, tf_warning_or_error); > + > + static bool suggested_p; > + if (concepts_diagnostics_max_depth_exceeded_p > + && current_constraint_diagnosis_depth == 0 > + && !suggested_p) > + { > + inform (UNKNOWN_LOCATION, > + "set -fconcepts-diagnostics-depth= to at least %d for more detail", > + concepts_diagnostics_max_depth + 1); > + suggested_p = true; > + } > } > > #include "gt-cp-constraint.h" > diff --git a/gcc/testsuite/g++.dg/concepts/diagnostic2.C b/gcc/testsuite/g++.dg/concepts/diagnostic2.C > index ce51b71fa8b..47accb8366e 100644 > --- a/gcc/testsuite/g++.dg/concepts/diagnostic2.C > +++ b/gcc/testsuite/g++.dg/concepts/diagnostic2.C > @@ -5,7 +5,7 @@ template<typename T> > inline constexpr bool foo_v = false; > > template<typename T> > - concept foo = foo_v<T> || foo_v<T&>; // { dg-message "neither operand" } > + concept foo = foo_v<T> || foo_v<T&>; // { dg-message "no operand" } > /* { dg-begin-multiline-output "" } > concept foo = foo_v<T> || foo_v<T&>; > ~~~~~~~~~^~~~~~~~~~~~ > diff --git a/gcc/testsuite/g++.dg/concepts/diagnostic5.C b/gcc/testsuite/g++.dg/concepts/diagnostic5.C > new file mode 100644 > index 00000000000..3c3b42f566c > --- /dev/null > +++ b/gcc/testsuite/g++.dg/concepts/diagnostic5.C > @@ -0,0 +1,46 @@ > +// { dg-do compile { target c++2a } } > +// { dg-additional-options "-fconcepts-diagnostics-depth=2" } > + > +template<typename T> > + concept c1 = requires { typename T::blah; }; > +// { dg-message "satisfaction of .c1<char>." "" { target *-*-* } .-1 } > +// { dg-message "satisfaction of .c1<char\\*>." "" { target *-*-* } .-2 } > +// { dg-message ".typename T::blah. is invalid" "" { target *-*-* } .-3 } > + > +template<typename T> > + concept c2 = requires (T x) { *x; }; > +// { dg-message "satisfaction of .c2<char>." "" { target *-*-* } .-1 } > +// { dg-message "in requirements with .char x." "" { target *-*-* } .-2 } > +// { dg-message "required expression .* is invalid" "" { target *-*-* } .-3 } > + > +template<typename T> > + concept c3 = __is_same(T, const T) || __is_same(T, int); > +// { dg-message "satisfaction of .c3<char>." "" { target *-*-* } .-1 } > +// { dg-message "no operand of the disjunction is satisfied" "" { target *-*-* } .-2 } > + > +template<typename T> > + concept c4 = requires (T x) { requires c2<const T> || c2<volatile T>; }; > +// { dg-message "satisfaction of .c4<char>." "" { target *-*-* } .-1 } > +// { dg-message "nested requirement" "" { target *-*-* } .-2 } > + > +template<typename T> > + concept c5 = requires (T x) { { &x } -> c1; }; > +// { dg-message "satisfaction of .c5<char>." "" { target *-*-* } .-1 } > +// { dg-message "in requirements with .char x." "" { target *-*-* } .-2 } > +// { dg-message "does not satisfy return-type-requirement" "" { target *-*-* } .-3 } > +// { dg-error "deduced expression type does not satisfy" "" { target *-*-* } .-4 } > + > +template<typename T> > + requires (c1<T> || c2<T>) || (c3<T> || c4<T>) || c5<T> // { dg-message "49: no operand" } > + // { dg-message ".c1<T>. is unsatisfied because" "" { target *-*-* } .-1 } > + // { dg-message ".c2<T>. is unsatisfied because" "" { target *-*-* } .-2 } > + // { dg-message ".c3<T>. is unsatisfied because" "" { target *-*-* } .-3 } > + // { dg-message ".c4<T>. is unsatisfied because" "" { target *-*-* } .-4 } > + // { dg-message ".c5<T>. is unsatisfied because" "" { target *-*-* } .-5 } > + void foo() { } > + > +void > +bar() > +{ > + foo<char>(); // { dg-error "use of" } > +} >
On Wed, 11 Mar 2020, Jason Merrill wrote: > On 3/9/20 6:23 PM, Patrick Palka wrote: > > This patch adds a new flag -fconcepts-diagnostics-depth to the C++ frontend > > which controls how deeply we replay errors when diagnosing a constraint > > satisfaction failure. The default is -fconcepts-diagnostics-depth=1 which > > diagnoses only the topmost constraint satisfaction failure and is consistent > > with our behavior before this patch. By increasing this flag's value, the > > user > > can control how deeply they want the compiler to explain a constraint > > satisfaction error. > > > > For example, if the unsatisfied constraint is a disjunction, then the > > default > > behavior is to just say "no branch in the disjunction is satisfied", but > > with > > -fconcepts-diagnostics-depth=2 we will additionally replay and diagnose the > > error in each branch of the disjunction. And if the unsatisfied constraint > > is a > > requires expression, then we will replay the error in the requires > > expression, > > etc. This proceeds recursively until there is nothing more to replay or we > > reached the exceeded the maximum depth specified by the flag. > > > > Implementation wise, this patch essentially just uncomments the existing > > commented-out code that performs the error-replaying, adding logic to keep > > track > > of the current replay depth along the way. Besides that, there is a new > > routine > > collect_operands_of_disjunction which flattens a disjunction and collects > > all of > > its operands into a vector. > > > > Here are some examples of diagnostics with the two patches in this series. > > > > For the simple test case in which we call ranges::begin() on something > > that's > > not a range: > > > > #include <ranges> > > > > struct S { } s; > > auto x = std::ranges::begin(s); > > > > we get the following diagnostics with -fconcepts-diagnostics-depth={1,2,3} > > respectively: > > > > https://pppalka.github.io/ranges-begin-depth-1.html > > https://pppalka.github.io/ranges-begin-depth-2.html > > https://pppalka.github.io/ranges-begin-depth-3.html > > > > And for the new test g++.dg/concepts/diagnostic5.C, we get: > > > > https://pppalka.github.io/diagnostic5-depth-1.html > > https://pppalka.github.io/diagnostic5-depth-2.html > > https://pppalka.github.io/diagnostic5-depth-3.html > > https://pppalka.github.io/diagnostic5-depth-4.html > > > > The extra diagnostics enabled by this flag are at times longer than they > > need to > > be (e.g. "the operand is_array_v<...> is unsatisfied because \n the > > expression > > is_array_v<...> [with ...] evaluated to false") and not immediately easy to > > follow (especially when there are nested disjunctions), but the transparency > > provided by these optional diagnostics seems to be pretty helpful in > > practice. > > > > Does this seem like a sensible approach? Thoughts and ideas for improvement > > welcome. Wording and naming suggestions would be much appreciated. > > This does seem like a good approach, thanks. > > > gcc/c-family/ChangeLog: > > > > * c.opt: Add -fconcepts-diagnostics-depth. > > > > gcc/cp/ChangeLog: > > > > * constraint.cc (finish_constraint_binary_op): Set the location of > > EXPR > > as well as its range, because build_x_binary_op doesn't always do so. > > (current_constraint_diagnosis_depth): New. > > (concepts_diagnostics_max_depth_exceeded_p): New. > > (collect_operands_of_disjunction): New. > > (satisfy_disjunction): When diagnosing a satisfaction failure, maybe > > replay each branch of the disjunction, subject to the current > > diagnosis > > depth. > > (diagnose_valid_expression): When diagnosing a satisfaction failure, > > maybe replay the substitution error, subject to the current diagnosis > > recursion. > > (diagnose_valid_type): Likewise. > > (diagnose_nested_requiremnet): Likewise. > > (diagnosing_failed_constraint::diagnosing_failed_constraint): > > Increment > > current_constraint_diagnosis_depth when diagnosing. > > (diagnosing_failed_constraint::~diagnosing_failed_constraint): > > Decrement > > current_constraint_diagnosis_depth when diagnosing. > > (diagnose_constraints): Don't diagnose if > > concepts_diagnostics_max_depth > > is 0. Emit a one-off note to increase -fconcepts-diagnostics-depth if > > the limit was exceeded. > > > > gcc/testsuite/ChangeLog: > > > > * g++.dg/concepts/diagnostic2.C: Expect "no operand" instead of > > "neither operand". > > * g++.dg/concepts/diagnostic5.C: New test. > > --- > > gcc/c-family/c.opt | 4 + > > gcc/cp/constraint.cc | 146 +++++++++++++++++--- > > gcc/testsuite/g++.dg/concepts/diagnostic2.C | 2 +- > > gcc/testsuite/g++.dg/concepts/diagnostic5.C | 46 ++++++ > > 4 files changed, 177 insertions(+), 21 deletions(-) > > create mode 100644 gcc/testsuite/g++.dg/concepts/diagnostic5.C > > > > diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt > > index 1cd585fa71d..97ef488931d 100644 > > --- a/gcc/c-family/c.opt > > +++ b/gcc/c-family/c.opt > > @@ -1453,6 +1453,10 @@ fconcepts-ts > > C++ ObjC++ Var(flag_concepts_ts) Init(0) > > Enable certain features present in the Concepts TS. > > +fconcepts-diagnostics-depth= > > +C++ ObjC++ Joined RejectNegative UInteger > > Var(concepts_diagnostics_max_depth) Init(1) > > +Specify maximum error replay depth during recursive diagnosis of a > > constraint satisfaction failure. > > + > > fcond-mismatch > > C ObjC C++ ObjC++ > > Allow the arguments of the '?' operator to have different types. > > diff --git a/gcc/cp/constraint.cc b/gcc/cp/constraint.cc > > index 4bb4a3f7252..c5e3d64daa6 100644 > > --- a/gcc/cp/constraint.cc > > +++ b/gcc/cp/constraint.cc > > @@ -162,6 +162,7 @@ finish_constraint_binary_op (location_t loc, > > /* When either operand is dependent, the overload set may be non-empty. > > */ > > if (expr == error_mark_node) > > return error_mark_node; > > + expr.set_location (loc); > > expr.set_range (lhs.get_start (), rhs.get_finish ()); > > return expr; > > } > > @@ -2386,6 +2387,50 @@ satisfy_conjunction (tree t, tree args, subst_info > > info) > > return satisfy_constraint_r (TREE_OPERAND (t, 1), args, info); > > } > > +/* The current depth at which we're replaying an error during recursive > > + diagnosis of a constraint satisfaction failure. */ > > + > > +static int current_constraint_diagnosis_depth; > > + > > +/* Whether CURRENT_CONSTRAINT_DIAGNOSIS_DEPTH has ever exceeded > > + CONCEPTS_DIAGNOSTICS_MAX_DEPTH during recursive diagnosis of a > > constraint > > + satisfaction error. */ > > + > > +static bool concepts_diagnostics_max_depth_exceeded_p; > > + > > +/* Recursive subroutine of collect_operands_of_disjunction. T is a > > normalized > > + subexpression of a constraint (composed of CONJ_CONSTRs and > > DISJ_CONSTRs) > > + and E is the corresponding unnormalized subexpression (composed of > > + TRUTH_ANDIF_EXPRs and TRUTH_ORIF_EXPRs). */ > > + > > +static void > > +collect_operands_of_disjunction_r (tree t, tree e, > > + auto_vec<std::pair<tree, tree> > *operands) > > +{ > > + if (TREE_CODE (e) == TRUTH_ORIF_EXPR) > > + { > > + collect_operands_of_disjunction_r (TREE_OPERAND (t, 0), > > + TREE_OPERAND (e, 0), operands); > > + collect_operands_of_disjunction_r (TREE_OPERAND (t, 1), > > + TREE_OPERAND (e, 1), operands); > > + } > > + else > > + { > > + std::pair<tree, tree> p = {t, e}; > > + operands->safe_push (p); > > + } > > +} > > + > > +/* Recursively collect the normalized and unnormalized operands of the > > + disjunction T and append them to OPERANDS in order. */ > > + > > +static void > > +collect_operands_of_disjunction (tree t, > > + auto_vec<std::pair<tree, tree> > *operands) > > +{ > > + collect_operands_of_disjunction_r (t, CONSTR_EXPR (t), operands); > > +} > > + > > /* Compute the satisfaction of a disjunction. */ > > static tree > > @@ -2403,11 +2448,27 @@ satisfy_disjunction (tree t, tree args, subst_info > > info) > > tree rhs = satisfy_constraint_r (TREE_OPERAND (t, 1), args, quiet); > > if (rhs != boolean_true_node && info.noisy ()) > > { > > - location_t loc = cp_expr_location (CONSTR_EXPR (t)); > > - inform (loc, "neither operand of the disjunction is satisfied"); > > - /* TODO: Replay the LHS and RHS to find failures in both branches. > > */ > > - // satisfy_constraint_r (TREE_OPERAND (t, 0), args, info); > > - // satisfy_constraint_r (TREE_OPERAND (t, 1), args, info); > > + cp_expr disj_expr = CONSTR_EXPR (t); > > + inform (disj_expr.get_location (), > > + "no operand of the disjunction is satisfied"); > > + if (current_constraint_diagnosis_depth < > > concepts_diagnostics_max_depth) > > Could we localize all the tracking of current_constraint_diagnosis_depth vs. > max_depth and max_depth_exceeded_p in diagnosing_failed_constraint, where > you're already doing the increment/decrement? Hmm, like this? This version introduces a new static member function diagnosing_failed_constraint::replay_errors_p that checks current_constraint_diagnosis_depth, and also sets max_depth_exceeded_p when appropriate. -- >8 -- gcc/c-family/ChangeLog: * c.opt: Add -fconcepts-diagnostics-depth. gcc/cp/ChangeLog: * constraint.cc (finish_constraint_binary_op): Set the location of EXPR as well as its range, because build_x_binary_op doesn't always do so. (current_constraint_diagnosis_depth): New. (concepts_diagnostics_max_depth_exceeded_p): New. (collect_operands_of_disjunction): New. (satisfy_disjunction): When diagnosing a satisfaction failure, maybe replay each branch of the disjunction, subject to the current diagnosis depth. (diagnose_valid_expression): When diagnosing a satisfaction failure, maybe replay the substitution error, subject to the current diagnosis recursion. (diagnose_valid_type): Likewise. (diagnose_nested_requiremnet): Likewise. (diagnosing_failed_constraint::diagnosing_failed_constraint): Increment current_constraint_diagnosis_depth when diagnosing. (diagnosing_failed_constraint::~diagnosing_failed_constraint): Decrement current_constraint_diagnosis_depth when diagnosing. (diagnosing_failed_constraint::replay_errors_p): New static member function. (diagnose_constraints): Don't diagnose if concepts_diagnostics_max_depth is 0. Emit a one-off note to increase -fconcepts-diagnostics-depth if the limit was exceeded. * cp-tree.h (diagnosing_failed_constraint::replay_errors_p): Declare. gcc/testsuite/ChangeLog: * g++.dg/concepts/diagnostic2.C: Expect "no operand" instead of "neither operand". * g++.dg/concepts/diagnostic5.C: New test. --- gcc/c-family/c.opt | 4 + gcc/cp/constraint.cc | 150 +++++++++++++++++--- gcc/cp/cp-tree.h | 1 + gcc/testsuite/g++.dg/concepts/diagnostic2.C | 2 +- gcc/testsuite/g++.dg/concepts/diagnostic5.C | 46 ++++++ 5 files changed, 182 insertions(+), 21 deletions(-) create mode 100644 gcc/testsuite/g++.dg/concepts/diagnostic5.C diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt index 1cd585fa71d..97ef488931d 100644 --- a/gcc/c-family/c.opt +++ b/gcc/c-family/c.opt @@ -1453,6 +1453,10 @@ fconcepts-ts C++ ObjC++ Var(flag_concepts_ts) Init(0) Enable certain features present in the Concepts TS. +fconcepts-diagnostics-depth= +C++ ObjC++ Joined RejectNegative UInteger Var(concepts_diagnostics_max_depth) Init(1) +Specify maximum error replay depth during recursive diagnosis of a constraint satisfaction failure. + fcond-mismatch C ObjC C++ ObjC++ Allow the arguments of the '?' operator to have different types. diff --git a/gcc/cp/constraint.cc b/gcc/cp/constraint.cc index 697ed6726b8..6aa432aaaf7 100644 --- a/gcc/cp/constraint.cc +++ b/gcc/cp/constraint.cc @@ -162,6 +162,7 @@ finish_constraint_binary_op (location_t loc, /* When either operand is dependent, the overload set may be non-empty. */ if (expr == error_mark_node) return error_mark_node; + expr.set_location (loc); expr.set_range (lhs.get_start (), rhs.get_finish ()); return expr; } @@ -2390,6 +2391,50 @@ satisfy_conjunction (tree t, tree args, subst_info info) return satisfy_constraint_r (TREE_OPERAND (t, 1), args, info); } +/* The current depth at which we're replaying an error during recursive + diagnosis of a constraint satisfaction failure. */ + +static int current_constraint_diagnosis_depth; + +/* Whether CURRENT_CONSTRAINT_DIAGNOSIS_DEPTH has ever exceeded + CONCEPTS_DIAGNOSTICS_MAX_DEPTH during recursive diagnosis of a constraint + satisfaction error. */ + +static bool concepts_diagnostics_max_depth_exceeded_p; + +/* Recursive subroutine of collect_operands_of_disjunction. T is a normalized + subexpression of a constraint (composed of CONJ_CONSTRs and DISJ_CONSTRs) + and E is the corresponding unnormalized subexpression (composed of + TRUTH_ANDIF_EXPRs and TRUTH_ORIF_EXPRs). */ + +static void +collect_operands_of_disjunction_r (tree t, tree e, + auto_vec<std::pair<tree, tree> > *operands) +{ + if (TREE_CODE (e) == TRUTH_ORIF_EXPR) + { + collect_operands_of_disjunction_r (TREE_OPERAND (t, 0), + TREE_OPERAND (e, 0), operands); + collect_operands_of_disjunction_r (TREE_OPERAND (t, 1), + TREE_OPERAND (e, 1), operands); + } + else + { + std::pair<tree, tree> p = {t, e}; + operands->safe_push (p); + } +} + +/* Recursively collect the normalized and unnormalized operands of the + disjunction T and append them to OPERANDS in order. */ + +static void +collect_operands_of_disjunction (tree t, + auto_vec<std::pair<tree, tree> > *operands) +{ + collect_operands_of_disjunction_r (t, CONSTR_EXPR (t), operands); +} + /* Compute the satisfaction of a disjunction. */ static tree @@ -2407,11 +2452,25 @@ satisfy_disjunction (tree t, tree args, subst_info info) tree rhs = satisfy_constraint_r (TREE_OPERAND (t, 1), args, quiet); if (rhs != boolean_true_node && info.noisy ()) { - location_t loc = cp_expr_location (CONSTR_EXPR (t)); - inform (loc, "neither operand of the disjunction is satisfied"); - /* TODO: Replay the LHS and RHS to find failures in both branches. */ - // satisfy_constraint_r (TREE_OPERAND (t, 0), args, info); - // satisfy_constraint_r (TREE_OPERAND (t, 1), args, info); + cp_expr disj_expr = CONSTR_EXPR (t); + inform (disj_expr.get_location (), + "no operand of the disjunction is satisfied"); + if (diagnosing_failed_constraint::replay_errors_p ()) + { + /* Replay the error in each branch of the disjunction. */ + auto_vec<std::pair<tree, tree> > operands; + collect_operands_of_disjunction (t, &operands); + for (unsigned i = 0; i < operands.length (); i++) + { + tree norm_op = operands[i].first; + tree op = operands[i].second; + location_t loc = make_location (cp_expr_location (op), + disj_expr.get_start (), + disj_expr.get_finish ()); + inform (loc, "the operand %qE is unsatisfied because", op); + satisfy_constraint_r (norm_op, args, info); + } + } } return rhs; } @@ -3151,10 +3210,14 @@ diagnose_valid_expression (tree expr, tree args, tree in_decl) return result; location_t loc = cp_expr_loc_or_input_loc (expr); - inform (loc, "the required expression %qE is invalid", expr); - - /* TODO: Replay the substitution to diagnose the error? */ - // tsubst_expr (expr, args, tf_error, in_decl, false); + if (diagnosing_failed_constraint::replay_errors_p ()) + { + /* Replay the substitution error. */ + inform (loc, "the required expression %qE is invalid, because", expr); + tsubst_expr (expr, args, tf_error, in_decl, false); + } + else + inform (loc, "the required expression %qE is invalid", expr); return error_mark_node; } @@ -3167,10 +3230,14 @@ diagnose_valid_type (tree type, tree args, tree in_decl) return result; location_t loc = cp_expr_loc_or_input_loc (type); - inform (loc, "the required type %qT is invalid", type); - - /* TODO: Replay the substitution to diagnose the error? */ - // tsubst (type, args, tf_error, in_decl); + if (diagnosing_failed_constraint::replay_errors_p ()) + { + /* Replay the substitution error. */ + inform (loc, "the required type %qT is invalid, because", type); + tsubst (type, args, tf_error, in_decl); + } + else + inform (loc, "the required type %qT is invalid", type); return error_mark_node; } @@ -3249,11 +3316,16 @@ diagnose_nested_requirement (tree req, tree args) tree expr = TREE_OPERAND (req, 0); location_t loc = cp_expr_location (expr); - inform (loc, "nested requirement %qE is not satisfied", expr); + if (diagnosing_failed_constraint::replay_errors_p ()) + { + /* Replay the substitution error. */ + inform (loc, "nested requirement %qE is not satisfied, because", expr); + subst_info noisy (tf_warning_or_error, NULL_TREE); + satisfy_constraint_expression (expr, args, noisy); + } + else + inform (loc, "nested requirement %qE is not satisfied", expr); - /* TODO: Replay the substitution to diagnose the error? */ - // subst_info noisy (tf_warning_or_error, NULL_TREE); - // satisfy_constraint (norm, args, info); } static void @@ -3350,14 +3422,38 @@ diagnosing_failed_constraint (tree t, tree args, bool diag) : diagnosing_error (diag) { if (diagnosing_error) - current_failed_constraint = tree_cons (args, t, current_failed_constraint); + { + current_failed_constraint + = tree_cons (args, t, current_failed_constraint); + ++current_constraint_diagnosis_depth; + } } diagnosing_failed_constraint:: ~diagnosing_failed_constraint () { - if (diagnosing_error && current_failed_constraint) - current_failed_constraint = TREE_CHAIN (current_failed_constraint); + if (diagnosing_error) + { + --current_constraint_diagnosis_depth; + if (current_failed_constraint) + current_failed_constraint = TREE_CHAIN (current_failed_constraint); + } + +} + +/* Whether we can replay an error that underlies a constraint failure at the + current diagnosis depth. */ + +bool +diagnosing_failed_constraint::replay_errors_p () +{ + if (current_constraint_diagnosis_depth >= concepts_diagnostics_max_depth) + { + concepts_diagnostics_max_depth_exceeded_p = true; + return false; + } + else + return true; } /* Emit diagnostics detailing the failure ARGS to satisfy the constraints @@ -3368,11 +3464,25 @@ diagnose_constraints (location_t loc, tree t, tree args) { inform (loc, "constraints not satisfied"); + if (concepts_diagnostics_max_depth == 0) + return; + /* Replay satisfaction, but diagnose errors. */ if (!args) constraint_satisfaction_value (t, tf_warning_or_error); else constraint_satisfaction_value (t, args, tf_warning_or_error); + + static bool suggested_p; + if (concepts_diagnostics_max_depth_exceeded_p + && current_constraint_diagnosis_depth == 0 + && !suggested_p) + { + inform (UNKNOWN_LOCATION, + "set -fconcepts-diagnostics-depth= to at least %d for more detail", + concepts_diagnostics_max_depth + 1); + suggested_p = true; + } } #include "gt-cp-constraint.h" diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h index 757cdd8168a..8235e5947a2 100644 --- a/gcc/cp/cp-tree.h +++ b/gcc/cp/cp-tree.h @@ -7829,6 +7829,7 @@ struct diagnosing_failed_constraint { diagnosing_failed_constraint (tree, tree, bool); ~diagnosing_failed_constraint (); + static bool replay_errors_p (); bool diagnosing_error; }; diff --git a/gcc/testsuite/g++.dg/concepts/diagnostic2.C b/gcc/testsuite/g++.dg/concepts/diagnostic2.C index ce51b71fa8b..47accb8366e 100644 --- a/gcc/testsuite/g++.dg/concepts/diagnostic2.C +++ b/gcc/testsuite/g++.dg/concepts/diagnostic2.C @@ -5,7 +5,7 @@ template<typename T> inline constexpr bool foo_v = false; template<typename T> - concept foo = foo_v<T> || foo_v<T&>; // { dg-message "neither operand" } + concept foo = foo_v<T> || foo_v<T&>; // { dg-message "no operand" } /* { dg-begin-multiline-output "" } concept foo = foo_v<T> || foo_v<T&>; ~~~~~~~~~^~~~~~~~~~~~ diff --git a/gcc/testsuite/g++.dg/concepts/diagnostic5.C b/gcc/testsuite/g++.dg/concepts/diagnostic5.C new file mode 100644 index 00000000000..3c3b42f566c --- /dev/null +++ b/gcc/testsuite/g++.dg/concepts/diagnostic5.C @@ -0,0 +1,46 @@ +// { dg-do compile { target c++2a } } +// { dg-additional-options "-fconcepts-diagnostics-depth=2" } + +template<typename T> + concept c1 = requires { typename T::blah; }; +// { dg-message "satisfaction of .c1<char>." "" { target *-*-* } .-1 } +// { dg-message "satisfaction of .c1<char\\*>." "" { target *-*-* } .-2 } +// { dg-message ".typename T::blah. is invalid" "" { target *-*-* } .-3 } + +template<typename T> + concept c2 = requires (T x) { *x; }; +// { dg-message "satisfaction of .c2<char>." "" { target *-*-* } .-1 } +// { dg-message "in requirements with .char x." "" { target *-*-* } .-2 } +// { dg-message "required expression .* is invalid" "" { target *-*-* } .-3 } + +template<typename T> + concept c3 = __is_same(T, const T) || __is_same(T, int); +// { dg-message "satisfaction of .c3<char>." "" { target *-*-* } .-1 } +// { dg-message "no operand of the disjunction is satisfied" "" { target *-*-* } .-2 } + +template<typename T> + concept c4 = requires (T x) { requires c2<const T> || c2<volatile T>; }; +// { dg-message "satisfaction of .c4<char>." "" { target *-*-* } .-1 } +// { dg-message "nested requirement" "" { target *-*-* } .-2 } + +template<typename T> + concept c5 = requires (T x) { { &x } -> c1; }; +// { dg-message "satisfaction of .c5<char>." "" { target *-*-* } .-1 } +// { dg-message "in requirements with .char x." "" { target *-*-* } .-2 } +// { dg-message "does not satisfy return-type-requirement" "" { target *-*-* } .-3 } +// { dg-error "deduced expression type does not satisfy" "" { target *-*-* } .-4 } + +template<typename T> + requires (c1<T> || c2<T>) || (c3<T> || c4<T>) || c5<T> // { dg-message "49: no operand" } + // { dg-message ".c1<T>. is unsatisfied because" "" { target *-*-* } .-1 } + // { dg-message ".c2<T>. is unsatisfied because" "" { target *-*-* } .-2 } + // { dg-message ".c3<T>. is unsatisfied because" "" { target *-*-* } .-3 } + // { dg-message ".c4<T>. is unsatisfied because" "" { target *-*-* } .-4 } + // { dg-message ".c5<T>. is unsatisfied because" "" { target *-*-* } .-5 } + void foo() { } + +void +bar() +{ + foo<char>(); // { dg-error "use of" } +}
On 3/11/20 4:18 PM, Patrick Palka via Gcc-patches wrote: ... > Hmm, like this? This version introduces a new static member function > diagnosing_failed_constraint::replay_errors_p that checks > current_constraint_diagnosis_depth, and also sets max_depth_exceeded_p > when appropriate. > ... Just one small quoting nit: > @@ -3368,11 +3464,25 @@ diagnose_constraints (location_t loc, tree t, tree args) > { > inform (loc, "constraints not satisfied"); > > + if (concepts_diagnostics_max_depth == 0) > + return; > + > /* Replay satisfaction, but diagnose errors. */ > if (!args) > constraint_satisfaction_value (t, tf_warning_or_error); > else > constraint_satisfaction_value (t, args, tf_warning_or_error); > + > + static bool suggested_p; > + if (concepts_diagnostics_max_depth_exceeded_p > + && current_constraint_diagnosis_depth == 0 > + && !suggested_p) > + { > + inform (UNKNOWN_LOCATION, > + "set -fconcepts-diagnostics-depth= to at least %d for more detail", > + concepts_diagnostics_max_depth + 1); Can you please quote the option name in the diagnostic (e.g., by using "set %qs to...", "-fconcepts-diagnostics-depth=" or equivalent) to avoid -Wformat-diag? It won't cause errors (yet) but will eventually need to be cleaned up. Thanks Martin
Hi Martin, On Sun, 15 Mar 2020, Martin Sebor wrote: > On 3/11/20 4:18 PM, Patrick Palka via Gcc-patches wrote: > ... > > Hmm, like this? This version introduces a new static member function > > diagnosing_failed_constraint::replay_errors_p that checks > > current_constraint_diagnosis_depth, and also sets max_depth_exceeded_p > > when appropriate. > > > ... > > Just one small quoting nit: > > > > @@ -3368,11 +3464,25 @@ diagnose_constraints (location_t loc, tree t, tree > > args) > > { > > inform (loc, "constraints not satisfied"); > > + if (concepts_diagnostics_max_depth == 0) > > + return; > > + > > /* Replay satisfaction, but diagnose errors. */ > > if (!args) > > constraint_satisfaction_value (t, tf_warning_or_error); > > else > > constraint_satisfaction_value (t, args, tf_warning_or_error); > > + > > + static bool suggested_p; > > + if (concepts_diagnostics_max_depth_exceeded_p > > + && current_constraint_diagnosis_depth == 0 > > + && !suggested_p) > > + { > > + inform (UNKNOWN_LOCATION, > > + "set -fconcepts-diagnostics-depth= to at least %d for more > > detail", > > + concepts_diagnostics_max_depth + 1); > > > Can you please quote the option name in the diagnostic (e.g., by using > "set %qs to...", "-fconcepts-diagnostics-depth=" or equivalent) to avoid > -Wformat-diag? It won't cause errors (yet) but will eventually need to > be cleaned up. Yes sure, consider it done. I've amended the patch locally with this change. > > Thanks > Martin > >
On Mon, 16 Mar 2020, Patrick Palka wrote: > Hi Martin, > > On Sun, 15 Mar 2020, Martin Sebor wrote: > > > On 3/11/20 4:18 PM, Patrick Palka via Gcc-patches wrote: > > ... > > > Hmm, like this? This version introduces a new static member function > > > diagnosing_failed_constraint::replay_errors_p that checks > > > current_constraint_diagnosis_depth, and also sets max_depth_exceeded_p > > > when appropriate. > > > > > ... > > > > Just one small quoting nit: > > > > > > > @@ -3368,11 +3464,25 @@ diagnose_constraints (location_t loc, tree t, tree > > > args) > > > { > > > inform (loc, "constraints not satisfied"); > > > + if (concepts_diagnostics_max_depth == 0) > > > + return; > > > + > > > /* Replay satisfaction, but diagnose errors. */ > > > if (!args) > > > constraint_satisfaction_value (t, tf_warning_or_error); > > > else > > > constraint_satisfaction_value (t, args, tf_warning_or_error); > > > + > > > + static bool suggested_p; > > > + if (concepts_diagnostics_max_depth_exceeded_p > > > + && current_constraint_diagnosis_depth == 0 > > > + && !suggested_p) > > > + { > > > + inform (UNKNOWN_LOCATION, > > > + "set -fconcepts-diagnostics-depth= to at least %d for more > > > detail", > > > + concepts_diagnostics_max_depth + 1); > > > > > > Can you please quote the option name in the diagnostic (e.g., by using > > "set %qs to...", "-fconcepts-diagnostics-depth=" or equivalent) to avoid > > -Wformat-diag? It won't cause errors (yet) but will eventually need to > > be cleaned up. > > Yes sure, consider it done. I've amended the patch locally with this > change. Here's a rebased version of this patch with the above diagnostic change, no other changes made (aside from a minor adjustment to diagnostic5.C): -- >8 -- gcc/c-family/ChangeLog: * c.opt: Add -fconcepts-diagnostics-depth. gcc/cp/ChangeLog: * constraint.cc (finish_constraint_binary_op): Set the location of EXPR as well as its range, because build_x_binary_op doesn't always do so. (current_constraint_diagnosis_depth): New. (concepts_diagnostics_max_depth_exceeded_p): New. (collect_operands_of_disjunction): New. (satisfy_disjunction): When diagnosing a satisfaction failure, maybe replay each branch of the disjunction, subject to the current diagnosis depth. (diagnose_valid_expression): When diagnosing a satisfaction failure, maybe replay the substitution error, subject to the current diagnosis recursion. (diagnose_valid_type): Likewise. (diagnose_nested_requiremnet): Likewise. (diagnosing_failed_constraint::diagnosing_failed_constraint): Increment current_constraint_diagnosis_depth when diagnosing. (diagnosing_failed_constraint::~diagnosing_failed_constraint): Decrement current_constraint_diagnosis_depth when diagnosing. (diagnosing_failed_constraint::replay_errors_p): New static member function. (diagnose_constraints): Don't diagnose if concepts_diagnostics_max_depth is 0. Emit a one-off note to increase -fconcepts-diagnostics-depth if the limit was exceeded. * cp-tree.h (diagnosing_failed_constraint::replay_errors_p): Declare. gcc/testsuite/ChangeLog: * g++.dg/concepts/diagnostic2.C: Expect "no operand" instead of "neither operand". * g++.dg/concepts/diagnostic5.C: New test. --- gcc/c-family/c.opt | 4 + gcc/cp/constraint.cc | 150 +++++++++++++++++--- gcc/cp/cp-tree.h | 1 + gcc/testsuite/g++.dg/concepts/diagnostic2.C | 2 +- gcc/testsuite/g++.dg/concepts/diagnostic5.C | 46 ++++++ 5 files changed, 182 insertions(+), 21 deletions(-) create mode 100644 gcc/testsuite/g++.dg/concepts/diagnostic5.C diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt index a1e0f4cdc3a..c49da99d395 100644 --- a/gcc/c-family/c.opt +++ b/gcc/c-family/c.opt @@ -1453,6 +1453,10 @@ fconcepts-ts C++ ObjC++ Var(flag_concepts_ts) Init(0) Enable certain features present in the Concepts TS. +fconcepts-diagnostics-depth= +C++ ObjC++ Joined RejectNegative UInteger Var(concepts_diagnostics_max_depth) Init(1) +Specify maximum error replay depth during recursive diagnosis of a constraint satisfaction failure. + fcond-mismatch C ObjC C++ ObjC++ Allow the arguments of the '?' operator to have different types. diff --git a/gcc/cp/constraint.cc b/gcc/cp/constraint.cc index a86bcdf603a..76c520318c3 100644 --- a/gcc/cp/constraint.cc +++ b/gcc/cp/constraint.cc @@ -162,6 +162,7 @@ finish_constraint_binary_op (location_t loc, /* When either operand is dependent, the overload set may be non-empty. */ if (expr == error_mark_node) return error_mark_node; + expr.set_location (loc); expr.set_range (lhs.get_start (), rhs.get_finish ()); return expr; } @@ -2395,6 +2396,49 @@ satisfy_conjunction (tree t, tree args, subst_info info) return satisfy_constraint_r (TREE_OPERAND (t, 1), args, info); } +/* The current depth at which we're replaying an error during recursive + diagnosis of a constraint satisfaction failure. */ + +static int current_constraint_diagnosis_depth; + +/* Whether CURRENT_CONSTRAINT_DIAGNOSIS_DEPTH has ever exceeded + CONCEPTS_DIAGNOSTICS_MAX_DEPTH during recursive diagnosis of a constraint + satisfaction error. */ + +static bool concepts_diagnostics_max_depth_exceeded_p; + +/* Recursive subroutine of collect_operands_of_disjunction. T is a normalized + subexpression of a constraint (composed of CONJ_CONSTRs and DISJ_CONSTRs) + and E is the corresponding unnormalized subexpression (composed of + TRUTH_ANDIF_EXPRs and TRUTH_ORIF_EXPRs). */ + +static void +collect_operands_of_disjunction_r (tree t, tree e, + auto_vec<tree_pair> *operands) +{ + if (TREE_CODE (e) == TRUTH_ORIF_EXPR) + { + collect_operands_of_disjunction_r (TREE_OPERAND (t, 0), + TREE_OPERAND (e, 0), operands); + collect_operands_of_disjunction_r (TREE_OPERAND (t, 1), + TREE_OPERAND (e, 1), operands); + } + else + { + tree_pair p = std::make_pair (t, e); + operands->safe_push (p); + } +} + +/* Recursively collect the normalized and unnormalized operands of the + disjunction T and append them to OPERANDS in order. */ + +static void +collect_operands_of_disjunction (tree t, auto_vec<tree_pair> *operands) +{ + collect_operands_of_disjunction_r (t, CONSTR_EXPR (t), operands); +} + /* Compute the satisfaction of a disjunction. */ static tree @@ -2412,11 +2456,25 @@ satisfy_disjunction (tree t, tree args, subst_info info) tree rhs = satisfy_constraint_r (TREE_OPERAND (t, 1), args, quiet); if (rhs != boolean_true_node && info.noisy ()) { - location_t loc = cp_expr_location (CONSTR_EXPR (t)); - inform (loc, "neither operand of the disjunction is satisfied"); - /* TODO: Replay the LHS and RHS to find failures in both branches. */ - // satisfy_constraint_r (TREE_OPERAND (t, 0), args, info); - // satisfy_constraint_r (TREE_OPERAND (t, 1), args, info); + cp_expr disj_expr = CONSTR_EXPR (t); + inform (disj_expr.get_location (), + "no operand of the disjunction is satisfied"); + if (diagnosing_failed_constraint::replay_errors_p ()) + { + /* Replay the error in each branch of the disjunction. */ + auto_vec<tree_pair> operands; + collect_operands_of_disjunction (t, &operands); + for (unsigned i = 0; i < operands.length (); i++) + { + tree norm_op = operands[i].first; + tree op = operands[i].second; + location_t loc = make_location (cp_expr_location (op), + disj_expr.get_start (), + disj_expr.get_finish ()); + inform (loc, "the operand %qE is unsatisfied because", op); + satisfy_constraint_r (norm_op, args, info); + } + } } return rhs; } @@ -3182,10 +3240,14 @@ diagnose_valid_expression (tree expr, tree args, tree in_decl) return result; location_t loc = cp_expr_loc_or_input_loc (expr); - inform (loc, "the required expression %qE is invalid", expr); - - /* TODO: Replay the substitution to diagnose the error? */ - // tsubst_expr (expr, args, tf_error, in_decl, false); + if (diagnosing_failed_constraint::replay_errors_p ()) + { + /* Replay the substitution error. */ + inform (loc, "the required expression %qE is invalid, because", expr); + tsubst_expr (expr, args, tf_error, in_decl, false); + } + else + inform (loc, "the required expression %qE is invalid", expr); return error_mark_node; } @@ -3198,10 +3260,14 @@ diagnose_valid_type (tree type, tree args, tree in_decl) return result; location_t loc = cp_expr_loc_or_input_loc (type); - inform (loc, "the required type %qT is invalid", type); - - /* TODO: Replay the substitution to diagnose the error? */ - // tsubst (type, args, tf_error, in_decl); + if (diagnosing_failed_constraint::replay_errors_p ()) + { + /* Replay the substitution error. */ + inform (loc, "the required type %qT is invalid, because", type); + tsubst (type, args, tf_error, in_decl); + } + else + inform (loc, "the required type %qT is invalid", type); return error_mark_node; } @@ -3280,11 +3346,16 @@ diagnose_nested_requirement (tree req, tree args) tree expr = TREE_OPERAND (req, 0); location_t loc = cp_expr_location (expr); - inform (loc, "nested requirement %qE is not satisfied", expr); + if (diagnosing_failed_constraint::replay_errors_p ()) + { + /* Replay the substitution error. */ + inform (loc, "nested requirement %qE is not satisfied, because", expr); + subst_info noisy (tf_warning_or_error, NULL_TREE); + satisfy_constraint_expression (expr, args, noisy); + } + else + inform (loc, "nested requirement %qE is not satisfied", expr); - /* TODO: Replay the substitution to diagnose the error? */ - // subst_info noisy (tf_warning_or_error, NULL_TREE); - // satisfy_constraint (norm, args, info); } static void @@ -3385,14 +3456,38 @@ diagnosing_failed_constraint (tree t, tree args, bool diag) : diagnosing_error (diag) { if (diagnosing_error) - current_failed_constraint = tree_cons (args, t, current_failed_constraint); + { + current_failed_constraint + = tree_cons (args, t, current_failed_constraint); + ++current_constraint_diagnosis_depth; + } } diagnosing_failed_constraint:: ~diagnosing_failed_constraint () { - if (diagnosing_error && current_failed_constraint) - current_failed_constraint = TREE_CHAIN (current_failed_constraint); + if (diagnosing_error) + { + --current_constraint_diagnosis_depth; + if (current_failed_constraint) + current_failed_constraint = TREE_CHAIN (current_failed_constraint); + } + +} + +/* Whether we are allowed to replay an error that underlies a constraint failure + at the current diagnosis depth. */ + +bool +diagnosing_failed_constraint::replay_errors_p () +{ + if (current_constraint_diagnosis_depth >= concepts_diagnostics_max_depth) + { + concepts_diagnostics_max_depth_exceeded_p = true; + return false; + } + else + return true; } /* Emit diagnostics detailing the failure ARGS to satisfy the constraints @@ -3403,11 +3498,26 @@ diagnose_constraints (location_t loc, tree t, tree args) { inform (loc, "constraints not satisfied"); + if (concepts_diagnostics_max_depth == 0) + return; + /* Replay satisfaction, but diagnose errors. */ if (!args) constraint_satisfaction_value (t, tf_warning_or_error); else constraint_satisfaction_value (t, args, tf_warning_or_error); + + static bool suggested_p; + if (concepts_diagnostics_max_depth_exceeded_p + && current_constraint_diagnosis_depth == 0 + && !suggested_p) + { + inform (UNKNOWN_LOCATION, + "set %qs to at least %d for more detail", + "-fconcepts-diagnostics-depth=", + concepts_diagnostics_max_depth + 1); + suggested_p = true; + } } #include "gt-cp-constraint.h" diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h index 4e1d0f1d42e..af9c4516996 100644 --- a/gcc/cp/cp-tree.h +++ b/gcc/cp/cp-tree.h @@ -7833,6 +7833,7 @@ struct diagnosing_failed_constraint { diagnosing_failed_constraint (tree, tree, bool); ~diagnosing_failed_constraint (); + static bool replay_errors_p (); bool diagnosing_error; }; diff --git a/gcc/testsuite/g++.dg/concepts/diagnostic2.C b/gcc/testsuite/g++.dg/concepts/diagnostic2.C index ce51b71fa8b..47accb8366e 100644 --- a/gcc/testsuite/g++.dg/concepts/diagnostic2.C +++ b/gcc/testsuite/g++.dg/concepts/diagnostic2.C @@ -5,7 +5,7 @@ template<typename T> inline constexpr bool foo_v = false; template<typename T> - concept foo = foo_v<T> || foo_v<T&>; // { dg-message "neither operand" } + concept foo = foo_v<T> || foo_v<T&>; // { dg-message "no operand" } /* { dg-begin-multiline-output "" } concept foo = foo_v<T> || foo_v<T&>; ~~~~~~~~~^~~~~~~~~~~~ diff --git a/gcc/testsuite/g++.dg/concepts/diagnostic5.C b/gcc/testsuite/g++.dg/concepts/diagnostic5.C new file mode 100644 index 00000000000..2641dc18423 --- /dev/null +++ b/gcc/testsuite/g++.dg/concepts/diagnostic5.C @@ -0,0 +1,46 @@ +// { dg-do compile { target c++2a } } +// { dg-additional-options "-fconcepts-diagnostics-depth=2" } + +template<typename T> + concept c1 = requires { typename T::blah; }; +// { dg-message "satisfaction of .c1<T>. .with T = char." "" { target *-*-* } .-1 } +// { dg-message "satisfaction of .c1<char\\*>." "" { target *-*-* } .-2 } +// { dg-message ".typename T::blah. is invalid" "" { target *-*-* } .-3 } + +template<typename T> + concept c2 = requires (T x) { *x; }; +// { dg-message "satisfaction of .c2<T>. .with T = char." "" { target *-*-* } .-1 } +// { dg-message "in requirements with .char x." "" { target *-*-* } .-2 } +// { dg-message "required expression .* is invalid" "" { target *-*-* } .-3 } + +template<typename T> + concept c3 = __is_same(T, const T) || __is_same(T, int); +// { dg-message "satisfaction of .c3<T>. .with T = char." "" { target *-*-* } .-1 } +// { dg-message "no operand of the disjunction is satisfied" "" { target *-*-* } .-2 } + +template<typename T> + concept c4 = requires (T x) { requires c2<const T> || c2<volatile T>; }; +// { dg-message "satisfaction of .c4<T>. .with T = char." "" { target *-*-* } .-1 } +// { dg-message "nested requirement" "" { target *-*-* } .-2 } + +template<typename T> + concept c5 = requires (T x) { { &x } -> c1; }; +// { dg-message "satisfaction of .c5<T>. .with T = char." "" { target *-*-* } .-1 } +// { dg-message "in requirements with .char x." "" { target *-*-* } .-2 } +// { dg-message "does not satisfy return-type-requirement" "" { target *-*-* } .-3 } +// { dg-error "deduced expression type does not satisfy" "" { target *-*-* } .-4 } + +template<typename T> + requires (c1<T> || c2<T>) || (c3<T> || c4<T>) || c5<T> // { dg-message "49: no operand" } + // { dg-message ".c1<T>. is unsatisfied because" "" { target *-*-* } .-1 } + // { dg-message ".c2<T>. is unsatisfied because" "" { target *-*-* } .-2 } + // { dg-message ".c3<T>. is unsatisfied because" "" { target *-*-* } .-3 } + // { dg-message ".c4<T>. is unsatisfied because" "" { target *-*-* } .-4 } + // { dg-message ".c5<T>. is unsatisfied because" "" { target *-*-* } .-5 } + void foo() { } + +void +bar() +{ + foo<char>(); // { dg-error "use of" } +}
On 3/27/20 12:17 AM, Patrick Palka wrote: > On Mon, 16 Mar 2020, Patrick Palka wrote: > >> Hi Martin, >> >> On Sun, 15 Mar 2020, Martin Sebor wrote: >> >>> On 3/11/20 4:18 PM, Patrick Palka via Gcc-patches wrote: >>> ... >>>> Hmm, like this? This version introduces a new static member function >>>> diagnosing_failed_constraint::replay_errors_p that checks >>>> current_constraint_diagnosis_depth, and also sets max_depth_exceeded_p >>>> when appropriate. >>>> >>> ... >>> >>> Just one small quoting nit: >>> >>> >>>> @@ -3368,11 +3464,25 @@ diagnose_constraints (location_t loc, tree t, tree >>>> args) >>>> { >>>> inform (loc, "constraints not satisfied"); >>>> + if (concepts_diagnostics_max_depth == 0) >>>> + return; >>>> + >>>> /* Replay satisfaction, but diagnose errors. */ >>>> if (!args) >>>> constraint_satisfaction_value (t, tf_warning_or_error); >>>> else >>>> constraint_satisfaction_value (t, args, tf_warning_or_error); >>>> + >>>> + static bool suggested_p; >>>> + if (concepts_diagnostics_max_depth_exceeded_p >>>> + && current_constraint_diagnosis_depth == 0 >>>> + && !suggested_p) >>>> + { >>>> + inform (UNKNOWN_LOCATION, >>>> + "set -fconcepts-diagnostics-depth= to at least %d for more >>>> detail", >>>> + concepts_diagnostics_max_depth + 1); >>> >>> >>> Can you please quote the option name in the diagnostic (e.g., by using >>> "set %qs to...", "-fconcepts-diagnostics-depth=" or equivalent) to avoid >>> -Wformat-diag? It won't cause errors (yet) but will eventually need to >>> be cleaned up. >> >> Yes sure, consider it done. I've amended the patch locally with this >> change. > > Here's a rebased version of this patch with the above diagnostic change, > no other changes made (aside from a minor adjustment to diagnostic5.C): OK. > -- >8 -- > > gcc/c-family/ChangeLog: > > * c.opt: Add -fconcepts-diagnostics-depth. > > gcc/cp/ChangeLog: > > * constraint.cc (finish_constraint_binary_op): Set the location of EXPR > as well as its range, because build_x_binary_op doesn't always do so. > (current_constraint_diagnosis_depth): New. > (concepts_diagnostics_max_depth_exceeded_p): New. > (collect_operands_of_disjunction): New. > (satisfy_disjunction): When diagnosing a satisfaction failure, maybe > replay each branch of the disjunction, subject to the current diagnosis > depth. > (diagnose_valid_expression): When diagnosing a satisfaction failure, > maybe replay the substitution error, subject to the current diagnosis > recursion. > (diagnose_valid_type): Likewise. > (diagnose_nested_requiremnet): Likewise. > (diagnosing_failed_constraint::diagnosing_failed_constraint): Increment > current_constraint_diagnosis_depth when diagnosing. > (diagnosing_failed_constraint::~diagnosing_failed_constraint): Decrement > current_constraint_diagnosis_depth when diagnosing. > (diagnosing_failed_constraint::replay_errors_p): New static member > function. > (diagnose_constraints): Don't diagnose if concepts_diagnostics_max_depth > is 0. Emit a one-off note to increase -fconcepts-diagnostics-depth if > the limit was exceeded. > * cp-tree.h (diagnosing_failed_constraint::replay_errors_p): Declare. > > gcc/testsuite/ChangeLog: > > * g++.dg/concepts/diagnostic2.C: Expect "no operand" instead of > "neither operand". > * g++.dg/concepts/diagnostic5.C: New test. > --- > gcc/c-family/c.opt | 4 + > gcc/cp/constraint.cc | 150 +++++++++++++++++--- > gcc/cp/cp-tree.h | 1 + > gcc/testsuite/g++.dg/concepts/diagnostic2.C | 2 +- > gcc/testsuite/g++.dg/concepts/diagnostic5.C | 46 ++++++ > 5 files changed, 182 insertions(+), 21 deletions(-) > create mode 100644 gcc/testsuite/g++.dg/concepts/diagnostic5.C > > diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt > index a1e0f4cdc3a..c49da99d395 100644 > --- a/gcc/c-family/c.opt > +++ b/gcc/c-family/c.opt > @@ -1453,6 +1453,10 @@ fconcepts-ts > C++ ObjC++ Var(flag_concepts_ts) Init(0) > Enable certain features present in the Concepts TS. > > +fconcepts-diagnostics-depth= > +C++ ObjC++ Joined RejectNegative UInteger Var(concepts_diagnostics_max_depth) Init(1) > +Specify maximum error replay depth during recursive diagnosis of a constraint satisfaction failure. > + > fcond-mismatch > C ObjC C++ ObjC++ > Allow the arguments of the '?' operator to have different types. > diff --git a/gcc/cp/constraint.cc b/gcc/cp/constraint.cc > index a86bcdf603a..76c520318c3 100644 > --- a/gcc/cp/constraint.cc > +++ b/gcc/cp/constraint.cc > @@ -162,6 +162,7 @@ finish_constraint_binary_op (location_t loc, > /* When either operand is dependent, the overload set may be non-empty. */ > if (expr == error_mark_node) > return error_mark_node; > + expr.set_location (loc); > expr.set_range (lhs.get_start (), rhs.get_finish ()); > return expr; > } > @@ -2395,6 +2396,49 @@ satisfy_conjunction (tree t, tree args, subst_info info) > return satisfy_constraint_r (TREE_OPERAND (t, 1), args, info); > } > > +/* The current depth at which we're replaying an error during recursive > + diagnosis of a constraint satisfaction failure. */ > + > +static int current_constraint_diagnosis_depth; > + > +/* Whether CURRENT_CONSTRAINT_DIAGNOSIS_DEPTH has ever exceeded > + CONCEPTS_DIAGNOSTICS_MAX_DEPTH during recursive diagnosis of a constraint > + satisfaction error. */ > + > +static bool concepts_diagnostics_max_depth_exceeded_p; > + > +/* Recursive subroutine of collect_operands_of_disjunction. T is a normalized > + subexpression of a constraint (composed of CONJ_CONSTRs and DISJ_CONSTRs) > + and E is the corresponding unnormalized subexpression (composed of > + TRUTH_ANDIF_EXPRs and TRUTH_ORIF_EXPRs). */ > + > +static void > +collect_operands_of_disjunction_r (tree t, tree e, > + auto_vec<tree_pair> *operands) > +{ > + if (TREE_CODE (e) == TRUTH_ORIF_EXPR) > + { > + collect_operands_of_disjunction_r (TREE_OPERAND (t, 0), > + TREE_OPERAND (e, 0), operands); > + collect_operands_of_disjunction_r (TREE_OPERAND (t, 1), > + TREE_OPERAND (e, 1), operands); > + } > + else > + { > + tree_pair p = std::make_pair (t, e); > + operands->safe_push (p); > + } > +} > + > +/* Recursively collect the normalized and unnormalized operands of the > + disjunction T and append them to OPERANDS in order. */ > + > +static void > +collect_operands_of_disjunction (tree t, auto_vec<tree_pair> *operands) > +{ > + collect_operands_of_disjunction_r (t, CONSTR_EXPR (t), operands); > +} > + > /* Compute the satisfaction of a disjunction. */ > > static tree > @@ -2412,11 +2456,25 @@ satisfy_disjunction (tree t, tree args, subst_info info) > tree rhs = satisfy_constraint_r (TREE_OPERAND (t, 1), args, quiet); > if (rhs != boolean_true_node && info.noisy ()) > { > - location_t loc = cp_expr_location (CONSTR_EXPR (t)); > - inform (loc, "neither operand of the disjunction is satisfied"); > - /* TODO: Replay the LHS and RHS to find failures in both branches. */ > - // satisfy_constraint_r (TREE_OPERAND (t, 0), args, info); > - // satisfy_constraint_r (TREE_OPERAND (t, 1), args, info); > + cp_expr disj_expr = CONSTR_EXPR (t); > + inform (disj_expr.get_location (), > + "no operand of the disjunction is satisfied"); > + if (diagnosing_failed_constraint::replay_errors_p ()) > + { > + /* Replay the error in each branch of the disjunction. */ > + auto_vec<tree_pair> operands; > + collect_operands_of_disjunction (t, &operands); > + for (unsigned i = 0; i < operands.length (); i++) > + { > + tree norm_op = operands[i].first; > + tree op = operands[i].second; > + location_t loc = make_location (cp_expr_location (op), > + disj_expr.get_start (), > + disj_expr.get_finish ()); > + inform (loc, "the operand %qE is unsatisfied because", op); > + satisfy_constraint_r (norm_op, args, info); > + } > + } > } > return rhs; > } > @@ -3182,10 +3240,14 @@ diagnose_valid_expression (tree expr, tree args, tree in_decl) > return result; > > location_t loc = cp_expr_loc_or_input_loc (expr); > - inform (loc, "the required expression %qE is invalid", expr); > - > - /* TODO: Replay the substitution to diagnose the error? */ > - // tsubst_expr (expr, args, tf_error, in_decl, false); > + if (diagnosing_failed_constraint::replay_errors_p ()) > + { > + /* Replay the substitution error. */ > + inform (loc, "the required expression %qE is invalid, because", expr); > + tsubst_expr (expr, args, tf_error, in_decl, false); > + } > + else > + inform (loc, "the required expression %qE is invalid", expr); > > return error_mark_node; > } > @@ -3198,10 +3260,14 @@ diagnose_valid_type (tree type, tree args, tree in_decl) > return result; > > location_t loc = cp_expr_loc_or_input_loc (type); > - inform (loc, "the required type %qT is invalid", type); > - > - /* TODO: Replay the substitution to diagnose the error? */ > - // tsubst (type, args, tf_error, in_decl); > + if (diagnosing_failed_constraint::replay_errors_p ()) > + { > + /* Replay the substitution error. */ > + inform (loc, "the required type %qT is invalid, because", type); > + tsubst (type, args, tf_error, in_decl); > + } > + else > + inform (loc, "the required type %qT is invalid", type); > > return error_mark_node; > } > @@ -3280,11 +3346,16 @@ diagnose_nested_requirement (tree req, tree args) > > tree expr = TREE_OPERAND (req, 0); > location_t loc = cp_expr_location (expr); > - inform (loc, "nested requirement %qE is not satisfied", expr); > + if (diagnosing_failed_constraint::replay_errors_p ()) > + { > + /* Replay the substitution error. */ > + inform (loc, "nested requirement %qE is not satisfied, because", expr); > + subst_info noisy (tf_warning_or_error, NULL_TREE); > + satisfy_constraint_expression (expr, args, noisy); > + } > + else > + inform (loc, "nested requirement %qE is not satisfied", expr); > > - /* TODO: Replay the substitution to diagnose the error? */ > - // subst_info noisy (tf_warning_or_error, NULL_TREE); > - // satisfy_constraint (norm, args, info); > } > > static void > @@ -3385,14 +3456,38 @@ diagnosing_failed_constraint (tree t, tree args, bool diag) > : diagnosing_error (diag) > { > if (diagnosing_error) > - current_failed_constraint = tree_cons (args, t, current_failed_constraint); > + { > + current_failed_constraint > + = tree_cons (args, t, current_failed_constraint); > + ++current_constraint_diagnosis_depth; > + } > } > > diagnosing_failed_constraint:: > ~diagnosing_failed_constraint () > { > - if (diagnosing_error && current_failed_constraint) > - current_failed_constraint = TREE_CHAIN (current_failed_constraint); > + if (diagnosing_error) > + { > + --current_constraint_diagnosis_depth; > + if (current_failed_constraint) > + current_failed_constraint = TREE_CHAIN (current_failed_constraint); > + } > + > +} > + > +/* Whether we are allowed to replay an error that underlies a constraint failure > + at the current diagnosis depth. */ > + > +bool > +diagnosing_failed_constraint::replay_errors_p () > +{ > + if (current_constraint_diagnosis_depth >= concepts_diagnostics_max_depth) > + { > + concepts_diagnostics_max_depth_exceeded_p = true; > + return false; > + } > + else > + return true; > } > > /* Emit diagnostics detailing the failure ARGS to satisfy the constraints > @@ -3403,11 +3498,26 @@ diagnose_constraints (location_t loc, tree t, tree args) > { > inform (loc, "constraints not satisfied"); > > + if (concepts_diagnostics_max_depth == 0) > + return; > + > /* Replay satisfaction, but diagnose errors. */ > if (!args) > constraint_satisfaction_value (t, tf_warning_or_error); > else > constraint_satisfaction_value (t, args, tf_warning_or_error); > + > + static bool suggested_p; > + if (concepts_diagnostics_max_depth_exceeded_p > + && current_constraint_diagnosis_depth == 0 > + && !suggested_p) > + { > + inform (UNKNOWN_LOCATION, > + "set %qs to at least %d for more detail", > + "-fconcepts-diagnostics-depth=", > + concepts_diagnostics_max_depth + 1); > + suggested_p = true; > + } > } > > #include "gt-cp-constraint.h" > diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h > index 4e1d0f1d42e..af9c4516996 100644 > --- a/gcc/cp/cp-tree.h > +++ b/gcc/cp/cp-tree.h > @@ -7833,6 +7833,7 @@ struct diagnosing_failed_constraint > { > diagnosing_failed_constraint (tree, tree, bool); > ~diagnosing_failed_constraint (); > + static bool replay_errors_p (); > > bool diagnosing_error; > }; > diff --git a/gcc/testsuite/g++.dg/concepts/diagnostic2.C b/gcc/testsuite/g++.dg/concepts/diagnostic2.C > index ce51b71fa8b..47accb8366e 100644 > --- a/gcc/testsuite/g++.dg/concepts/diagnostic2.C > +++ b/gcc/testsuite/g++.dg/concepts/diagnostic2.C > @@ -5,7 +5,7 @@ template<typename T> > inline constexpr bool foo_v = false; > > template<typename T> > - concept foo = foo_v<T> || foo_v<T&>; // { dg-message "neither operand" } > + concept foo = foo_v<T> || foo_v<T&>; // { dg-message "no operand" } > /* { dg-begin-multiline-output "" } > concept foo = foo_v<T> || foo_v<T&>; > ~~~~~~~~~^~~~~~~~~~~~ > diff --git a/gcc/testsuite/g++.dg/concepts/diagnostic5.C b/gcc/testsuite/g++.dg/concepts/diagnostic5.C > new file mode 100644 > index 00000000000..2641dc18423 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/concepts/diagnostic5.C > @@ -0,0 +1,46 @@ > +// { dg-do compile { target c++2a } } > +// { dg-additional-options "-fconcepts-diagnostics-depth=2" } > + > +template<typename T> > + concept c1 = requires { typename T::blah; }; > +// { dg-message "satisfaction of .c1<T>. .with T = char." "" { target *-*-* } .-1 } > +// { dg-message "satisfaction of .c1<char\\*>." "" { target *-*-* } .-2 } > +// { dg-message ".typename T::blah. is invalid" "" { target *-*-* } .-3 } > + > +template<typename T> > + concept c2 = requires (T x) { *x; }; > +// { dg-message "satisfaction of .c2<T>. .with T = char." "" { target *-*-* } .-1 } > +// { dg-message "in requirements with .char x." "" { target *-*-* } .-2 } > +// { dg-message "required expression .* is invalid" "" { target *-*-* } .-3 } > + > +template<typename T> > + concept c3 = __is_same(T, const T) || __is_same(T, int); > +// { dg-message "satisfaction of .c3<T>. .with T = char." "" { target *-*-* } .-1 } > +// { dg-message "no operand of the disjunction is satisfied" "" { target *-*-* } .-2 } > + > +template<typename T> > + concept c4 = requires (T x) { requires c2<const T> || c2<volatile T>; }; > +// { dg-message "satisfaction of .c4<T>. .with T = char." "" { target *-*-* } .-1 } > +// { dg-message "nested requirement" "" { target *-*-* } .-2 } > + > +template<typename T> > + concept c5 = requires (T x) { { &x } -> c1; }; > +// { dg-message "satisfaction of .c5<T>. .with T = char." "" { target *-*-* } .-1 } > +// { dg-message "in requirements with .char x." "" { target *-*-* } .-2 } > +// { dg-message "does not satisfy return-type-requirement" "" { target *-*-* } .-3 } > +// { dg-error "deduced expression type does not satisfy" "" { target *-*-* } .-4 } > + > +template<typename T> > + requires (c1<T> || c2<T>) || (c3<T> || c4<T>) || c5<T> // { dg-message "49: no operand" } > + // { dg-message ".c1<T>. is unsatisfied because" "" { target *-*-* } .-1 } > + // { dg-message ".c2<T>. is unsatisfied because" "" { target *-*-* } .-2 } > + // { dg-message ".c3<T>. is unsatisfied because" "" { target *-*-* } .-3 } > + // { dg-message ".c4<T>. is unsatisfied because" "" { target *-*-* } .-4 } > + // { dg-message ".c5<T>. is unsatisfied because" "" { target *-*-* } .-5 } > + void foo() { } > + > +void > +bar() > +{ > + foo<char>(); // { dg-error "use of" } > +} >
diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt index 1cd585fa71d..97ef488931d 100644 --- a/gcc/c-family/c.opt +++ b/gcc/c-family/c.opt @@ -1453,6 +1453,10 @@ fconcepts-ts C++ ObjC++ Var(flag_concepts_ts) Init(0) Enable certain features present in the Concepts TS. +fconcepts-diagnostics-depth= +C++ ObjC++ Joined RejectNegative UInteger Var(concepts_diagnostics_max_depth) Init(1) +Specify maximum error replay depth during recursive diagnosis of a constraint satisfaction failure. + fcond-mismatch C ObjC C++ ObjC++ Allow the arguments of the '?' operator to have different types. diff --git a/gcc/cp/constraint.cc b/gcc/cp/constraint.cc index 4bb4a3f7252..c5e3d64daa6 100644 --- a/gcc/cp/constraint.cc +++ b/gcc/cp/constraint.cc @@ -162,6 +162,7 @@ finish_constraint_binary_op (location_t loc, /* When either operand is dependent, the overload set may be non-empty. */ if (expr == error_mark_node) return error_mark_node; + expr.set_location (loc); expr.set_range (lhs.get_start (), rhs.get_finish ()); return expr; } @@ -2386,6 +2387,50 @@ satisfy_conjunction (tree t, tree args, subst_info info) return satisfy_constraint_r (TREE_OPERAND (t, 1), args, info); } +/* The current depth at which we're replaying an error during recursive + diagnosis of a constraint satisfaction failure. */ + +static int current_constraint_diagnosis_depth; + +/* Whether CURRENT_CONSTRAINT_DIAGNOSIS_DEPTH has ever exceeded + CONCEPTS_DIAGNOSTICS_MAX_DEPTH during recursive diagnosis of a constraint + satisfaction error. */ + +static bool concepts_diagnostics_max_depth_exceeded_p; + +/* Recursive subroutine of collect_operands_of_disjunction. T is a normalized + subexpression of a constraint (composed of CONJ_CONSTRs and DISJ_CONSTRs) + and E is the corresponding unnormalized subexpression (composed of + TRUTH_ANDIF_EXPRs and TRUTH_ORIF_EXPRs). */ + +static void +collect_operands_of_disjunction_r (tree t, tree e, + auto_vec<std::pair<tree, tree> > *operands) +{ + if (TREE_CODE (e) == TRUTH_ORIF_EXPR) + { + collect_operands_of_disjunction_r (TREE_OPERAND (t, 0), + TREE_OPERAND (e, 0), operands); + collect_operands_of_disjunction_r (TREE_OPERAND (t, 1), + TREE_OPERAND (e, 1), operands); + } + else + { + std::pair<tree, tree> p = {t, e}; + operands->safe_push (p); + } +} + +/* Recursively collect the normalized and unnormalized operands of the + disjunction T and append them to OPERANDS in order. */ + +static void +collect_operands_of_disjunction (tree t, + auto_vec<std::pair<tree, tree> > *operands) +{ + collect_operands_of_disjunction_r (t, CONSTR_EXPR (t), operands); +} + /* Compute the satisfaction of a disjunction. */ static tree @@ -2403,11 +2448,27 @@ satisfy_disjunction (tree t, tree args, subst_info info) tree rhs = satisfy_constraint_r (TREE_OPERAND (t, 1), args, quiet); if (rhs != boolean_true_node && info.noisy ()) { - location_t loc = cp_expr_location (CONSTR_EXPR (t)); - inform (loc, "neither operand of the disjunction is satisfied"); - /* TODO: Replay the LHS and RHS to find failures in both branches. */ - // satisfy_constraint_r (TREE_OPERAND (t, 0), args, info); - // satisfy_constraint_r (TREE_OPERAND (t, 1), args, info); + cp_expr disj_expr = CONSTR_EXPR (t); + inform (disj_expr.get_location (), + "no operand of the disjunction is satisfied"); + if (current_constraint_diagnosis_depth < concepts_diagnostics_max_depth) + { + /* Replay the error in each branch of the disjunction. */ + auto_vec<std::pair<tree, tree> > operands; + collect_operands_of_disjunction (t, &operands); + for (unsigned i = 0; i < operands.length (); i++) + { + tree norm_op = operands[i].first; + tree op = operands[i].second; + location_t loc = make_location (cp_expr_location (op), + disj_expr.get_start (), + disj_expr.get_finish ()); + inform (loc, "the operand %qE is unsatisfied because", op); + satisfy_constraint_r (norm_op, args, info); + } + } + else + concepts_diagnostics_max_depth_exceeded_p = true; } return rhs; } @@ -3147,10 +3208,17 @@ diagnose_valid_expression (tree expr, tree args, tree in_decl) return result; location_t loc = cp_expr_loc_or_input_loc (expr); - inform (loc, "the required expression %qE is invalid", expr); - - /* TODO: Replay the substitution to diagnose the error? */ - // tsubst_expr (expr, args, tf_error, in_decl, false); + if (current_constraint_diagnosis_depth < concepts_diagnostics_max_depth) + { + /* Replay the substitution error. */ + inform (loc, "the required expression %qE is invalid, because", expr); + tsubst_expr (expr, args, tf_error, in_decl, false); + } + else + { + inform (loc, "the required expression %qE is invalid", expr); + concepts_diagnostics_max_depth_exceeded_p = true; + } return error_mark_node; } @@ -3163,10 +3231,17 @@ diagnose_valid_type (tree type, tree args, tree in_decl) return result; location_t loc = cp_expr_loc_or_input_loc (type); - inform (loc, "the required type %qT is invalid", type); - - /* TODO: Replay the substitution to diagnose the error? */ - // tsubst (type, args, tf_error, in_decl); + if (current_constraint_diagnosis_depth < concepts_diagnostics_max_depth) + { + /* Replay the substitution error. */ + inform (loc, "the required type %qT is invalid, because", type); + tsubst (type, args, tf_error, in_decl); + } + else + { + inform (loc, "the required type %qT is invalid", type); + concepts_diagnostics_max_depth_exceeded_p = true; + } return error_mark_node; } @@ -3245,11 +3320,19 @@ diagnose_nested_requirement (tree req, tree args) tree expr = TREE_OPERAND (req, 0); location_t loc = cp_expr_location (expr); - inform (loc, "nested requirement %qE is not satisfied", expr); + if (current_constraint_diagnosis_depth < concepts_diagnostics_max_depth) + { + /* Replay the substitution error. */ + inform (loc, "nested requirement %qE is not satisfied, because", expr); + subst_info noisy (tf_warning_or_error, NULL_TREE); + satisfy_constraint_expression (expr, args, noisy); + } + else + { + inform (loc, "nested requirement %qE is not satisfied", expr); + concepts_diagnostics_max_depth_exceeded_p = true; + } - /* TODO: Replay the substitution to diagnose the error? */ - // subst_info noisy (tf_warning_or_error, NULL_TREE); - // satisfy_constraint (norm, args, info); } static void @@ -3346,14 +3429,23 @@ diagnosing_failed_constraint (tree t, tree args, bool diag) : diagnosing_error (diag) { if (diagnosing_error) - current_failed_constraint = tree_cons (args, t, current_failed_constraint); + { + current_failed_constraint + = tree_cons (args, t, current_failed_constraint); + ++current_constraint_diagnosis_depth; + } } diagnosing_failed_constraint:: ~diagnosing_failed_constraint () { - if (diagnosing_error && current_failed_constraint) - current_failed_constraint = TREE_CHAIN (current_failed_constraint); + if (diagnosing_error) + { + --current_constraint_diagnosis_depth; + if (current_failed_constraint) + current_failed_constraint = TREE_CHAIN (current_failed_constraint); + } + } /* Emit diagnostics detailing the failure ARGS to satisfy the constraints @@ -3364,11 +3456,25 @@ diagnose_constraints (location_t loc, tree t, tree args) { inform (loc, "constraints not satisfied"); + if (concepts_diagnostics_max_depth == 0) + return; + /* Replay satisfaction, but diagnose errors. */ if (!args) constraint_satisfaction_value (t, tf_warning_or_error); else constraint_satisfaction_value (t, args, tf_warning_or_error); + + static bool suggested_p; + if (concepts_diagnostics_max_depth_exceeded_p + && current_constraint_diagnosis_depth == 0 + && !suggested_p) + { + inform (UNKNOWN_LOCATION, + "set -fconcepts-diagnostics-depth= to at least %d for more detail", + concepts_diagnostics_max_depth + 1); + suggested_p = true; + } } #include "gt-cp-constraint.h" diff --git a/gcc/testsuite/g++.dg/concepts/diagnostic2.C b/gcc/testsuite/g++.dg/concepts/diagnostic2.C index ce51b71fa8b..47accb8366e 100644 --- a/gcc/testsuite/g++.dg/concepts/diagnostic2.C +++ b/gcc/testsuite/g++.dg/concepts/diagnostic2.C @@ -5,7 +5,7 @@ template<typename T> inline constexpr bool foo_v = false; template<typename T> - concept foo = foo_v<T> || foo_v<T&>; // { dg-message "neither operand" } + concept foo = foo_v<T> || foo_v<T&>; // { dg-message "no operand" } /* { dg-begin-multiline-output "" } concept foo = foo_v<T> || foo_v<T&>; ~~~~~~~~~^~~~~~~~~~~~ diff --git a/gcc/testsuite/g++.dg/concepts/diagnostic5.C b/gcc/testsuite/g++.dg/concepts/diagnostic5.C new file mode 100644 index 00000000000..3c3b42f566c --- /dev/null +++ b/gcc/testsuite/g++.dg/concepts/diagnostic5.C @@ -0,0 +1,46 @@ +// { dg-do compile { target c++2a } } +// { dg-additional-options "-fconcepts-diagnostics-depth=2" } + +template<typename T> + concept c1 = requires { typename T::blah; }; +// { dg-message "satisfaction of .c1<char>." "" { target *-*-* } .-1 } +// { dg-message "satisfaction of .c1<char\\*>." "" { target *-*-* } .-2 } +// { dg-message ".typename T::blah. is invalid" "" { target *-*-* } .-3 } + +template<typename T> + concept c2 = requires (T x) { *x; }; +// { dg-message "satisfaction of .c2<char>." "" { target *-*-* } .-1 } +// { dg-message "in requirements with .char x." "" { target *-*-* } .-2 } +// { dg-message "required expression .* is invalid" "" { target *-*-* } .-3 } + +template<typename T> + concept c3 = __is_same(T, const T) || __is_same(T, int); +// { dg-message "satisfaction of .c3<char>." "" { target *-*-* } .-1 } +// { dg-message "no operand of the disjunction is satisfied" "" { target *-*-* } .-2 } + +template<typename T> + concept c4 = requires (T x) { requires c2<const T> || c2<volatile T>; }; +// { dg-message "satisfaction of .c4<char>." "" { target *-*-* } .-1 } +// { dg-message "nested requirement" "" { target *-*-* } .-2 } + +template<typename T> + concept c5 = requires (T x) { { &x } -> c1; }; +// { dg-message "satisfaction of .c5<char>." "" { target *-*-* } .-1 } +// { dg-message "in requirements with .char x." "" { target *-*-* } .-2 } +// { dg-message "does not satisfy return-type-requirement" "" { target *-*-* } .-3 } +// { dg-error "deduced expression type does not satisfy" "" { target *-*-* } .-4 } + +template<typename T> + requires (c1<T> || c2<T>) || (c3<T> || c4<T>) || c5<T> // { dg-message "49: no operand" } + // { dg-message ".c1<T>. is unsatisfied because" "" { target *-*-* } .-1 } + // { dg-message ".c2<T>. is unsatisfied because" "" { target *-*-* } .-2 } + // { dg-message ".c3<T>. is unsatisfied because" "" { target *-*-* } .-3 } + // { dg-message ".c4<T>. is unsatisfied because" "" { target *-*-* } .-4 } + // { dg-message ".c5<T>. is unsatisfied because" "" { target *-*-* } .-5 } + void foo() { } + +void +bar() +{ + foo<char>(); // { dg-error "use of" } +}
This patch adds a new flag -fconcepts-diagnostics-depth to the C++ frontend which controls how deeply we replay errors when diagnosing a constraint satisfaction failure. The default is -fconcepts-diagnostics-depth=1 which diagnoses only the topmost constraint satisfaction failure and is consistent with our behavior before this patch. By increasing this flag's value, the user can control how deeply they want the compiler to explain a constraint satisfaction error. For example, if the unsatisfied constraint is a disjunction, then the default behavior is to just say "no branch in the disjunction is satisfied", but with -fconcepts-diagnostics-depth=2 we will additionally replay and diagnose the error in each branch of the disjunction. And if the unsatisfied constraint is a requires expression, then we will replay the error in the requires expression, etc. This proceeds recursively until there is nothing more to replay or we reached the exceeded the maximum depth specified by the flag. Implementation wise, this patch essentially just uncomments the existing commented-out code that performs the error-replaying, adding logic to keep track of the current replay depth along the way. Besides that, there is a new routine collect_operands_of_disjunction which flattens a disjunction and collects all of its operands into a vector. Here are some examples of diagnostics with the two patches in this series. For the simple test case in which we call ranges::begin() on something that's not a range: #include <ranges> struct S { } s; auto x = std::ranges::begin(s); we get the following diagnostics with -fconcepts-diagnostics-depth={1,2,3} respectively: https://pppalka.github.io/ranges-begin-depth-1.html https://pppalka.github.io/ranges-begin-depth-2.html https://pppalka.github.io/ranges-begin-depth-3.html And for the new test g++.dg/concepts/diagnostic5.C, we get: https://pppalka.github.io/diagnostic5-depth-1.html https://pppalka.github.io/diagnostic5-depth-2.html https://pppalka.github.io/diagnostic5-depth-3.html https://pppalka.github.io/diagnostic5-depth-4.html The extra diagnostics enabled by this flag are at times longer than they need to be (e.g. "the operand is_array_v<...> is unsatisfied because \n the expression is_array_v<...> [with ...] evaluated to false") and not immediately easy to follow (especially when there are nested disjunctions), but the transparency provided by these optional diagnostics seems to be pretty helpful in practice. Does this seem like a sensible approach? Thoughts and ideas for improvement welcome. Wording and naming suggestions would be much appreciated. gcc/c-family/ChangeLog: * c.opt: Add -fconcepts-diagnostics-depth. gcc/cp/ChangeLog: * constraint.cc (finish_constraint_binary_op): Set the location of EXPR as well as its range, because build_x_binary_op doesn't always do so. (current_constraint_diagnosis_depth): New. (concepts_diagnostics_max_depth_exceeded_p): New. (collect_operands_of_disjunction): New. (satisfy_disjunction): When diagnosing a satisfaction failure, maybe replay each branch of the disjunction, subject to the current diagnosis depth. (diagnose_valid_expression): When diagnosing a satisfaction failure, maybe replay the substitution error, subject to the current diagnosis recursion. (diagnose_valid_type): Likewise. (diagnose_nested_requiremnet): Likewise. (diagnosing_failed_constraint::diagnosing_failed_constraint): Increment current_constraint_diagnosis_depth when diagnosing. (diagnosing_failed_constraint::~diagnosing_failed_constraint): Decrement current_constraint_diagnosis_depth when diagnosing. (diagnose_constraints): Don't diagnose if concepts_diagnostics_max_depth is 0. Emit a one-off note to increase -fconcepts-diagnostics-depth if the limit was exceeded. gcc/testsuite/ChangeLog: * g++.dg/concepts/diagnostic2.C: Expect "no operand" instead of "neither operand". * g++.dg/concepts/diagnostic5.C: New test. --- gcc/c-family/c.opt | 4 + gcc/cp/constraint.cc | 146 +++++++++++++++++--- gcc/testsuite/g++.dg/concepts/diagnostic2.C | 2 +- gcc/testsuite/g++.dg/concepts/diagnostic5.C | 46 ++++++ 4 files changed, 177 insertions(+), 21 deletions(-) create mode 100644 gcc/testsuite/g++.dg/concepts/diagnostic5.C