On 2/20/20 7:27 PM, Patrick Palka wrote:
This patch improves our concept diagnostics in two ways. First, it sets a more precise location for the constraint expressions built in finish_constraint_binary_op. As a result, when a disjunction is unsatisfied we now print e.g..../include/bits/range_access.h:467:2: note: neither operand of the disjunction is satisfied 466 | requires is_bounded_array_v<remove_reference_t<_Tp>> || __member_end<_Tp> | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 467 | || __adl_end<_Tp> | ^~~~~~~~~~~~~~~~~ instead of .../include/bits/range_access.h:467:2: note: neither operand of the disjunction is satisfied 467 | || __adl_end<_Tp> | ^~
This hunk is OK.
Second, this patch changes diagnose_atomic_constraint to pretty-print unsatisfied atomic constraint expressions with their template arguments substituted in (if doing so does not result in a trivial expression). So for example we now emit cpp2a/concepts-pr67719.C:9:8: note: the expression ‘((C<int>() && C<long int>()) && C<void>())’ evaluated to ‘false’ instead of cpp2a/concepts-pr67719.C:9:8: note: the expression ‘(... &&(C<Tx>)())’ evaluated to ‘false’
Generally with templates we try to print the dependent form and then the arguments; pp_cxx_atomic_constraint already wants to do that, but we don't get there from here. Perhaps pass 'map' instead of 'args' into diagnose_atomic_constraint, and then build a new ATOMIC_CONSTR for printing with %E?
Tested on x86_64-pc-linux-gnu, and verified that all the diagnostics emitted in our concept tests are no worse with this patch. Does this look OK to commit? gcc/cp/ChangeLog: * constraint.cc (finish_constraint_binary_op): Set expr's location range to the range of its operands. (diagnose_atomic_constraint): Prefer to pretty-print the atomic constraint with template arguments substituted in. gcc/testsuite/ChangeLog: * g++.dg/concepts/diagnostic2.C: New test. * g++.dg/concepts/diagnostic3.C: New test. --- gcc/cp/constraint.cc | 15 +++++++---- gcc/testsuite/g++.dg/concepts/diagnostic2.C | 30 +++++++++++++++++++++ gcc/testsuite/g++.dg/concepts/diagnostic3.C | 19 +++++++++++++ 3 files changed, 59 insertions(+), 5 deletions(-) create mode 100644 gcc/testsuite/g++.dg/concepts/diagnostic2.C create mode 100644 gcc/testsuite/g++.dg/concepts/diagnostic3.C diff --git a/gcc/cp/constraint.cc b/gcc/cp/constraint.cc index 58044cd0f9d..4f6bc11e7e8 100644 --- a/gcc/cp/constraint.cc +++ b/gcc/cp/constraint.cc @@ -155,14 +155,14 @@ finish_constraint_binary_op (location_t loc, if (!check_constraint_operands (loc, lhs, rhs)) return error_mark_node; tree overload; - tree expr = build_x_binary_op (loc, code, - lhs, TREE_CODE (lhs), - rhs, TREE_CODE (rhs), - &overload, tf_none); + cp_expr expr = build_x_binary_op (loc, code, + lhs, TREE_CODE (lhs), + rhs, TREE_CODE (rhs), + &overload, tf_none); /* When either operand is dependent, the overload set may be non-empty. */ if (expr == error_mark_node) return error_mark_node; - SET_EXPR_LOCATION (expr, loc); + expr.set_range (lhs.get_start (), rhs.get_finish ()); return expr; }@@ -3330,6 +3330,11 @@ diagnose_atomic_constraint (tree t, tree args, subst_info info)inform (loc, "%qE is never satisfied", expr); break; default: + tree orig_expr = expr; + expr = tsubst_expr (expr, args, tf_none, NULL_TREE, false); + if (CONSTANT_CLASS_P (expr)) + /* We are better off printing the original expression. */ + expr = orig_expr; inform (loc, "the expression %qE evaluated to %<false%>", expr); } } diff --git a/gcc/testsuite/g++.dg/concepts/diagnostic2.C b/gcc/testsuite/g++.dg/concepts/diagnostic2.C new file mode 100644 index 00000000000..ce51b71fa8b --- /dev/null +++ b/gcc/testsuite/g++.dg/concepts/diagnostic2.C @@ -0,0 +1,30 @@ +// { dg-do compile { target c++2a } } +// { dg-options "-fdiagnostics-show-caret" } + +template<typename T> + inline constexpr bool foo_v = false; + +template<typename T> + concept foo = foo_v<T> || foo_v<T&>; // { dg-message "neither operand" } +/* { dg-begin-multiline-output "" } + concept foo = foo_v<T> || foo_v<T&>; + ~~~~~~~~~^~~~~~~~~~~~ + { dg-end-multiline-output "" } */ + +template<typename T> + requires foo<T> + void bar(); +/* { dg-begin-multiline-output "" } + void bar(); + { dg-end-multiline-output "" } */ +/* { dg-prune-output "~" } */ + +void +baz() +{ + bar<int>(); // { dg-error "unsatisfied constraints" } +/* { dg-begin-multiline-output "" } + bar<int>(); + ^ + { dg-end-multiline-output "" } */ +} diff --git a/gcc/testsuite/g++.dg/concepts/diagnostic3.C b/gcc/testsuite/g++.dg/concepts/diagnostic3.C new file mode 100644 index 00000000000..e0649dac51a --- /dev/null +++ b/gcc/testsuite/g++.dg/concepts/diagnostic3.C @@ -0,0 +1,19 @@ +// { dg-do compile { target c++2a } } + +template<typename T> + inline constexpr bool foo_v = false; + +template<typename T> + concept foo = (bool)(foo_v<T> | foo_v<T&>); // { dg-message "foo_v<int>.*foo_v<int&>" } + +template<typename T> +requires foo<T> +void +bar() +{ } + +void +baz() +{ + bar<int>(); // { dg-error "unsatisfied constraints" } +}
