diff mbox series

c++: Some improvements to concept diagnostics

Message ID 20200221002703.2389863-1-ppalka@redhat.com
State New
Headers show
Series c++: Some improvements to concept diagnostics | expand

Commit Message

Patrick Palka Feb. 21, 2020, 12:27 a.m. UTC 
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>
      |  ^~

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’

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

Comments

Jason Merrill Feb. 24, 2020, 3:48 p.m. UTC | #1 
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" }
> +}
>
Patrick Palka Feb. 24, 2020, 5:30 p.m. UTC | #2 
On Mon, 24 Feb 2020, Jason Merrill wrote:

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

That works really well.  I had to add handling of TYPE_ARGUMENT_PACK to
cxx_pretty_printer::type_id because otherwise an "unsupported
type_argument_pack" placeholder would get printed in these diagnostics
instead.  For consistency with the way we print TYPE_ARGUMENT_PACKs and
NONTYPE_ARGUMENT_PACKs elsewhere I also made
cxx_pretty_printer::expression print curly braces around a
NONTYPE_ARGUMENT_PACK.  Tested on x86_64-pc-linux-gnu, does this look
OK?

-- >8 --

gcc/cp/ChangeLog:

	* constraint.cc (finish_constraint_binary_op): Set expr's location range
	to the range of its operands.
	(satisfy_atom): Pass MAP instead of ARGS to diagnose_atomic_constraint.
	(diagnose_trait_expr): Take the instantiated parameter mapping MAP
	instead of the corresponding template arguments ARGS and adjust body
	accordingly.
	(diagnose_requires_expr): Likewise.
	(diagnose_atomic_constraint): Likewise.  When printing an atomic
	constraint expression, print the instantiated parameter mapping
	alongside it.
	* cxx-pretty-print.cc (cxx_pretty_printer::expression)
	[NONTYPE_ARGUMENT_PACK]: Print braces around a NONTYPE_ARGUMENT_PACK.
	(cxx_pretty_printer::type_id): Handle TYPE_ARGUMENT_PACK.

gcc/testsuite/ChangeLog:

	* g++.dg/concepts/diagnostic2.C: New test.
	* g++.dg/concepts/diagnostic3.C: New test.
---
 gcc/cp/constraint.cc                        | 33 ++++++++++++---------
 gcc/cp/cxx-pretty-print.c                   | 17 +++++++++++
 gcc/testsuite/g++.dg/concepts/diagnostic2.C | 30 +++++++++++++++++++
 gcc/testsuite/g++.dg/concepts/diagnostic3.C | 29 ++++++++++++++++++
 4 files changed, 95 insertions(+), 14 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..4e6ed440211 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;
 }
 
@@ -2547,7 +2547,7 @@ satisfy_atom (tree t, tree args, subst_info info)
   /* Compute the value of the constraint.  */
   result = satisfaction_value (cxx_constant_value (result));
   if (result == boolean_false_node && info.noisy ())
-    diagnose_atomic_constraint (t, args, info);
+    diagnose_atomic_constraint (t, map, info);
 
   return cache.save (result);
 }
@@ -3056,9 +3056,10 @@ get_constraint_error_location (tree t)
 /* Emit a diagnostic for a failed trait.  */
 
 void
-diagnose_trait_expr (tree expr, tree args)
+diagnose_trait_expr (tree expr, tree map)
 {
   location_t loc = cp_expr_location (expr);
+  tree args = get_mapped_args (map);
 
   /* Build a "fake" version of the instantiated trait, so we can
      get the instantiated types from result.  */
@@ -3271,11 +3272,12 @@ diagnose_requirement (tree req, tree args, tree in_decl)
 }
 
 static void
-diagnose_requires_expr (tree expr, tree args, tree in_decl)
+diagnose_requires_expr (tree expr, tree map, tree in_decl)
 {
   local_specialization_stack stack (lss_copy);
   tree parms = TREE_OPERAND (expr, 0);
   tree body = TREE_OPERAND (expr, 1);
+  tree args = get_mapped_args (map);
 
   cp_unevaluated u;
   subst_info info (tf_warning_or_error, NULL_TREE);
@@ -3292,11 +3294,11 @@ diagnose_requires_expr (tree expr, tree args, tree in_decl)
     }
 }
 
-/* Diagnose a substitution failure in the atomic constraint T. Note that
-   ARGS have been previously instantiated through the parameter map.  */
+/* Diagnose a substitution failure in the atomic constraint T when applied
+   to the instantiated parameter mapping MAP.  */
 
 static void
-diagnose_atomic_constraint (tree t, tree args, subst_info info)
+diagnose_atomic_constraint (tree t, tree map, subst_info info)
 {
   /* If the constraint is already ill-formed, we've previously diagnosed
      the reason. We should still say why the constraints aren't satisfied.  */
@@ -3320,17 +3322,20 @@ diagnose_atomic_constraint (tree t, tree args, subst_info info)
   switch (TREE_CODE (expr))
     {
     case TRAIT_EXPR:
-      diagnose_trait_expr (expr, args);
+      diagnose_trait_expr (expr, map);
       break;
     case REQUIRES_EXPR:
-      diagnose_requires_expr (expr, args, info.in_decl);
+      diagnose_requires_expr (expr, map, info.in_decl);
       break;
     case INTEGER_CST:
       /* This must be either 0 or false.  */
       inform (loc, "%qE is never satisfied", expr);
       break;
     default:
-      inform (loc, "the expression %qE evaluated to %<false%>", expr);
+      tree a = copy_node (t);
+      ATOMIC_CONSTR_MAP (a) = map;
+      inform (loc, "the expression %qE evaluated to %<false%>", a);
+      ggc_free (a);
     }
 }
 
diff --git a/gcc/cp/cxx-pretty-print.c b/gcc/cp/cxx-pretty-print.c
index 9e0c5fc8891..119432ebb32 100644
--- a/gcc/cp/cxx-pretty-print.c
+++ b/gcc/cp/cxx-pretty-print.c
@@ -1214,12 +1214,14 @@ cxx_pretty_printer::expression (tree t)
       {
 	tree args = ARGUMENT_PACK_ARGS (t);
 	int i, len = TREE_VEC_LENGTH (args);
+	pp_cxx_left_brace (this);
 	for (i = 0; i < len; ++i)
 	  {
 	    if (i > 0)
 	      pp_cxx_separate_with (this, ',');
 	    expression (TREE_VEC_ELT (args, i));
 	  }
+	pp_cxx_right_brace (this);
       }
       break;
 
@@ -1839,6 +1841,21 @@ cxx_pretty_printer::type_id (tree t)
       pp_cxx_ws_string (this, "...");
       break;
 
+    case TYPE_ARGUMENT_PACK:
+      {
+	tree args = ARGUMENT_PACK_ARGS (t);
+	int len = TREE_VEC_LENGTH (args);
+	pp_cxx_left_brace (this);
+	for (int i = 0; i < len; ++i)
+	  {
+	    if (i > 0)
+	      pp_cxx_separate_with (this, ',');
+	    type_id (TREE_VEC_ELT (args, i));
+	  }
+	pp_cxx_right_brace (this);
+	break;
+      }
+
     default:
       c_pretty_printer::type_id (t);
       break;
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..b4c75409b94
--- /dev/null
+++ b/gcc/testsuite/g++.dg/concepts/diagnostic3.C
@@ -0,0 +1,29 @@
+// { 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&>);
+
+template<typename... Ts>
+requires (foo<Ts> && ...)
+void
+bar() // { dg-message "with Ts = .int, char... evaluated to .false." }
+{ }
+
+template<int>
+struct S { };
+
+template<int... Is>
+requires (foo<S<Is>> && ...)
+void
+baz() // { dg-message "with Is = .2, 3, 4... evaluated to .false." }
+{ }
+
+void
+baz()
+{
+  bar<int, char>(); // { dg-error "unsatisfied constraints" }
+  baz<2,3,4>(); // { dg-error "unsatisfied constraints" }
+}
Jason Merrill Feb. 26, 2020, 5:01 a.m. UTC | #3 
On 2/24/20 12:30 PM, Patrick Palka wrote:
> On Mon, 24 Feb 2020, Jason Merrill wrote:
> 
>> 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?
> 
> That works really well.  I had to add handling of TYPE_ARGUMENT_PACK to
> cxx_pretty_printer::type_id because otherwise an "unsupported
> type_argument_pack" placeholder would get printed in these diagnostics
> instead.  For consistency with the way we print TYPE_ARGUMENT_PACKs and
> NONTYPE_ARGUMENT_PACKs elsewhere I also made
> cxx_pretty_printer::expression print curly braces around a
> NONTYPE_ARGUMENT_PACK.  Tested on x86_64-pc-linux-gnu, does this look
> OK?

OK.

> -- >8 --
> 
> gcc/cp/ChangeLog:
> 
> 	* constraint.cc (finish_constraint_binary_op): Set expr's location range
> 	to the range of its operands.
> 	(satisfy_atom): Pass MAP instead of ARGS to diagnose_atomic_constraint.
> 	(diagnose_trait_expr): Take the instantiated parameter mapping MAP
> 	instead of the corresponding template arguments ARGS and adjust body
> 	accordingly.
> 	(diagnose_requires_expr): Likewise.
> 	(diagnose_atomic_constraint): Likewise.  When printing an atomic
> 	constraint expression, print the instantiated parameter mapping
> 	alongside it.
> 	* cxx-pretty-print.cc (cxx_pretty_printer::expression)
> 	[NONTYPE_ARGUMENT_PACK]: Print braces around a NONTYPE_ARGUMENT_PACK.
> 	(cxx_pretty_printer::type_id): Handle TYPE_ARGUMENT_PACK.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* g++.dg/concepts/diagnostic2.C: New test.
> 	* g++.dg/concepts/diagnostic3.C: New test.
> ---
>   gcc/cp/constraint.cc                        | 33 ++++++++++++---------
>   gcc/cp/cxx-pretty-print.c                   | 17 +++++++++++
>   gcc/testsuite/g++.dg/concepts/diagnostic2.C | 30 +++++++++++++++++++
>   gcc/testsuite/g++.dg/concepts/diagnostic3.C | 29 ++++++++++++++++++
>   4 files changed, 95 insertions(+), 14 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..4e6ed440211 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;
>   }
>   
> @@ -2547,7 +2547,7 @@ satisfy_atom (tree t, tree args, subst_info info)
>     /* Compute the value of the constraint.  */
>     result = satisfaction_value (cxx_constant_value (result));
>     if (result == boolean_false_node && info.noisy ())
> -    diagnose_atomic_constraint (t, args, info);
> +    diagnose_atomic_constraint (t, map, info);
>   
>     return cache.save (result);
>   }
> @@ -3056,9 +3056,10 @@ get_constraint_error_location (tree t)
>   /* Emit a diagnostic for a failed trait.  */
>   
>   void
> -diagnose_trait_expr (tree expr, tree args)
> +diagnose_trait_expr (tree expr, tree map)
>   {
>     location_t loc = cp_expr_location (expr);
> +  tree args = get_mapped_args (map);
>   
>     /* Build a "fake" version of the instantiated trait, so we can
>        get the instantiated types from result.  */
> @@ -3271,11 +3272,12 @@ diagnose_requirement (tree req, tree args, tree in_decl)
>   }
>   
>   static void
> -diagnose_requires_expr (tree expr, tree args, tree in_decl)
> +diagnose_requires_expr (tree expr, tree map, tree in_decl)
>   {
>     local_specialization_stack stack (lss_copy);
>     tree parms = TREE_OPERAND (expr, 0);
>     tree body = TREE_OPERAND (expr, 1);
> +  tree args = get_mapped_args (map);
>   
>     cp_unevaluated u;
>     subst_info info (tf_warning_or_error, NULL_TREE);
> @@ -3292,11 +3294,11 @@ diagnose_requires_expr (tree expr, tree args, tree in_decl)
>       }
>   }
>   
> -/* Diagnose a substitution failure in the atomic constraint T. Note that
> -   ARGS have been previously instantiated through the parameter map.  */
> +/* Diagnose a substitution failure in the atomic constraint T when applied
> +   to the instantiated parameter mapping MAP.  */
>   
>   static void
> -diagnose_atomic_constraint (tree t, tree args, subst_info info)
> +diagnose_atomic_constraint (tree t, tree map, subst_info info)
>   {
>     /* If the constraint is already ill-formed, we've previously diagnosed
>        the reason. We should still say why the constraints aren't satisfied.  */
> @@ -3320,17 +3322,20 @@ diagnose_atomic_constraint (tree t, tree args, subst_info info)
>     switch (TREE_CODE (expr))
>       {
>       case TRAIT_EXPR:
> -      diagnose_trait_expr (expr, args);
> +      diagnose_trait_expr (expr, map);
>         break;
>       case REQUIRES_EXPR:
> -      diagnose_requires_expr (expr, args, info.in_decl);
> +      diagnose_requires_expr (expr, map, info.in_decl);
>         break;
>       case INTEGER_CST:
>         /* This must be either 0 or false.  */
>         inform (loc, "%qE is never satisfied", expr);
>         break;
>       default:
> -      inform (loc, "the expression %qE evaluated to %<false%>", expr);
> +      tree a = copy_node (t);
> +      ATOMIC_CONSTR_MAP (a) = map;
> +      inform (loc, "the expression %qE evaluated to %<false%>", a);
> +      ggc_free (a);
>       }
>   }
>   
> diff --git a/gcc/cp/cxx-pretty-print.c b/gcc/cp/cxx-pretty-print.c
> index 9e0c5fc8891..119432ebb32 100644
> --- a/gcc/cp/cxx-pretty-print.c
> +++ b/gcc/cp/cxx-pretty-print.c
> @@ -1214,12 +1214,14 @@ cxx_pretty_printer::expression (tree t)
>         {
>   	tree args = ARGUMENT_PACK_ARGS (t);
>   	int i, len = TREE_VEC_LENGTH (args);
> +	pp_cxx_left_brace (this);
>   	for (i = 0; i < len; ++i)
>   	  {
>   	    if (i > 0)
>   	      pp_cxx_separate_with (this, ',');
>   	    expression (TREE_VEC_ELT (args, i));
>   	  }
> +	pp_cxx_right_brace (this);
>         }
>         break;
>   
> @@ -1839,6 +1841,21 @@ cxx_pretty_printer::type_id (tree t)
>         pp_cxx_ws_string (this, "...");
>         break;
>   
> +    case TYPE_ARGUMENT_PACK:
> +      {
> +	tree args = ARGUMENT_PACK_ARGS (t);
> +	int len = TREE_VEC_LENGTH (args);
> +	pp_cxx_left_brace (this);
> +	for (int i = 0; i < len; ++i)
> +	  {
> +	    if (i > 0)
> +	      pp_cxx_separate_with (this, ',');
> +	    type_id (TREE_VEC_ELT (args, i));
> +	  }
> +	pp_cxx_right_brace (this);
> +	break;
> +      }
> +
>       default:
>         c_pretty_printer::type_id (t);
>         break;
> 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..b4c75409b94
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/concepts/diagnostic3.C
> @@ -0,0 +1,29 @@
> +// { 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&>);
> +
> +template<typename... Ts>
> +requires (foo<Ts> && ...)
> +void
> +bar() // { dg-message "with Ts = .int, char... evaluated to .false." }
> +{ }
> +
> +template<int>
> +struct S { };
> +
> +template<int... Is>
> +requires (foo<S<Is>> && ...)
> +void
> +baz() // { dg-message "with Is = .2, 3, 4... evaluated to .false." }
> +{ }
> +
> +void
> +baz()
> +{
> +  bar<int, char>(); // { dg-error "unsatisfied constraints" }
> +  baz<2,3,4>(); // { dg-error "unsatisfied constraints" }
> +}
>
diff mbox series

Patch

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" }
+}