* [PATCH] c++: Include the constraint parameter mapping in diagnostic constraint contexts
@ 2020-03-18 19:21 Patrick Palka
2020-03-18 19:26 ` Patrick Palka
0 siblings, 1 reply; 6+ messages in thread
From: Patrick Palka @ 2020-03-18 19:21 UTC (permalink / raw)
To: gcc-patches
When diagnosing a constraint error, we currently try to print the constraint
inside a diagnostic constraint context with its template arguments substituted
in. If substitution fails, then we instead just print the dependent
form, as in the the test case below:
gcc/testsuite/g++.dg/concepts/diagnostic6.C:14:15: error: static assertion failed
14 | static_assert(E<int>); // { dg-error "static assertion failed|not a class" }
| ^~~~~~
gcc/testsuite/g++.dg/concepts/diagnostic6.C:14:15: note: constraints not satisfied
gcc/testsuite/g++.dg/concepts/diagnostic6.C:4:11: required for the satisfaction of ‘C<T>’
gcc/testsuite/g++.dg/concepts/diagnostic6.C:8:11: required for the satisfaction of ‘D<typename T::type>’
gcc/testsuite/g++.dg/concepts/diagnostic6.C:14:15: error: ‘int’ is not a class, struct, or union type
But printing just the dependent form sometimes makes it difficult to decipher
the diagnostic. In the above example, for instance, there's no indication of
how the template argument 'int' relates to either of the 'T's.
This patch improves the situation by changing these diagnostics to always print
the dependent form of the constraint, and alongside it the (preferably
substituted) constraint parameter mapping. So with the same test case below we
now get:
gcc/testsuite/g++.dg/concepts/diagnostic6.C:14:15: error: static assertion failed
14 | static_assert(E<int>); // { dg-error "static assertion failed|not a class" }
| ^~~~~~
gcc/testsuite/g++.dg/concepts/diagnostic6.C:14:15: note: constraints not satisfied
gcc/testsuite/g++.dg/concepts/diagnostic6.C:4:11: required for the satisfaction of ‘C<T>’ [with T = typename T::type]
gcc/testsuite/g++.dg/concepts/diagnostic6.C:8:11: required for the satisfaction of ‘D<typename T::type>’ [with T = int]
gcc/testsuite/g++.dg/concepts/diagnostic6.C:14:15: error: ‘int’ is not a class, struct, or union type
This change arguably makes it easier to figure out what's going on whenever a
constraint fails due to substitution resulting in an invalid type rather than
failing due to the constraint evaluating to false.
Tested on x86_64-pc-linux-gnu, does this look reasonable? I'm not sure if
printing an unsubstituted parameter mapping (like in the line 4 message above)
is always useful, but perhaps it's better than nothing?
gcc/cp/ChangeLog:
* cxx-pretty-print.c (pp_cxx_parameter_mapping): Make extern.
* cxx-pretty-print.h (pp_cxx_parameter_mapping): Declare.
* error.c (rebuild_concept_check): Delete.
(print_concept_check_info): Print the constraint uninstantiated and the
parameter mapping alongside it.
gcc/testsuite/ChangeLog:
* g++.dg/concepts/diagnostic6.C: New test.
---
gcc/cp/cxx-pretty-print.c | 2 +-
gcc/cp/cxx-pretty-print.h | 1 +
gcc/cp/error.c | 39 ++++++++-------------
gcc/testsuite/g++.dg/concepts/diagnostic6.C | 14 ++++++++
4 files changed, 30 insertions(+), 26 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/concepts/diagnostic6.C
diff --git a/gcc/cp/cxx-pretty-print.c b/gcc/cp/cxx-pretty-print.c
index 100154e400f..1e89c3f9033 100644
--- a/gcc/cp/cxx-pretty-print.c
+++ b/gcc/cp/cxx-pretty-print.c
@@ -2878,7 +2878,7 @@ pp_cxx_check_constraint (cxx_pretty_printer *pp, tree t)
/* Output the "[with ...]" clause for a parameter mapping of an atomic
constraint. */
-static void
+void
pp_cxx_parameter_mapping (cxx_pretty_printer *pp, tree map)
{
for (tree p = map; p; p = TREE_CHAIN (p))
diff --git a/gcc/cp/cxx-pretty-print.h b/gcc/cp/cxx-pretty-print.h
index 7c7347f57ba..494f3fdde59 100644
--- a/gcc/cp/cxx-pretty-print.h
+++ b/gcc/cp/cxx-pretty-print.h
@@ -112,5 +112,6 @@ void pp_cxx_conjunction (cxx_pretty_printer *, tree);
void pp_cxx_disjunction (cxx_pretty_printer *, tree);
void pp_cxx_constraint (cxx_pretty_printer *, tree);
void pp_cxx_constrained_type_spec (cxx_pretty_printer *, tree);
+void pp_cxx_parameter_mapping (cxx_pretty_printer *, tree);
#endif /* GCC_CXX_PRETTY_PRINT_H */
diff --git a/gcc/cp/error.c b/gcc/cp/error.c
index cc51b6ffe13..4bf835e84a1 100644
--- a/gcc/cp/error.c
+++ b/gcc/cp/error.c
@@ -3680,27 +3680,6 @@ print_location (diagnostic_context *context, location_t loc)
"locus", xloc.file, xloc.line);
}
-/* Instantiate the concept check for the purpose of diagnosing an error. */
-
-static tree
-rebuild_concept_check (tree expr, tree map, tree args)
-{
- /* Instantiate the parameter mapping for the template-id. */
- map = tsubst_parameter_mapping (map, args, tf_none, NULL_TREE);
- if (map == error_mark_node)
- return error_mark_node;
- args = get_mapped_args (map);
-
- /* Rebuild the template id using substituted arguments. Substituting
- directly through the expression will trigger recursive satisfaction,
- so don't do that. */
- tree id = unpack_concept_check (expr);
- args = tsubst_template_args (TREE_OPERAND (id, 1), args, tf_none, NULL_TREE);
- if (args == error_mark_node)
- return error_mark_node;
- return build_nt (TEMPLATE_ID_EXPR, TREE_OPERAND (id, 0), args);
-}
-
static void
print_constrained_decl_info (diagnostic_context *context, tree decl)
{
@@ -3717,12 +3696,22 @@ print_concept_check_info (diagnostic_context *context, tree expr, tree map, tree
tree tmpl = TREE_OPERAND (id, 0);
if (OVL_P (tmpl))
tmpl = OVL_FIRST (tmpl);
- tree check = rebuild_concept_check (expr, map, args);
- if (check == error_mark_node)
- check = expr;
print_location (context, DECL_SOURCE_LOCATION (tmpl));
- pp_verbatim (context->printer, "required for the satisfaction of %qE\n", check);
+
+ cxx_pretty_printer *pp = (cxx_pretty_printer *)context->printer;
+ pp_verbatim (pp, "required for the satisfaction of %qE", expr);
+ map = tsubst_parameter_mapping (map, args, tf_none, NULL_TREE);
+ if (map && map != error_mark_node)
+ {
+ pp_cxx_whitespace (pp);
+ pp_cxx_left_bracket (pp);
+ pp->translate_string ("with");
+ pp_cxx_whitespace (pp);
+ pp_cxx_parameter_mapping (pp, map);
+ pp_cxx_right_bracket (pp);
+ }
+ pp_newline (pp);
}
/* Diagnose the entry point into the satisfaction error. Returns the next
diff --git a/gcc/testsuite/g++.dg/concepts/diagnostic6.C b/gcc/testsuite/g++.dg/concepts/diagnostic6.C
new file mode 100644
index 00000000000..06b17caccbe
--- /dev/null
+++ b/gcc/testsuite/g++.dg/concepts/diagnostic6.C
@@ -0,0 +1,14 @@
+// { dg-do compile { target c++2a } }
+
+template<typename T>
+ concept C = requires (T t) { t + 0; };
+// { dg-message "satisfaction of .C<T>. .with T = typename T::type." "" { target *-*-* } .-1 }
+
+template<typename T>
+ concept D = C<T>;
+// { dg-message "satisfaction of .D<typename T::type>. .with T = int." "" { target *-*-* } .-1 }
+
+template<typename T>
+ concept E = D<typename T::type>;
+
+static_assert(E<int>); // { dg-error "static assertion failed|not a class" }
--
2.26.0.rc1.11.g30e9940356
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] c++: Include the constraint parameter mapping in diagnostic constraint contexts
2020-03-18 19:21 [PATCH] c++: Include the constraint parameter mapping in diagnostic constraint contexts Patrick Palka
@ 2020-03-18 19:26 ` Patrick Palka
2020-03-19 14:31 ` Jason Merrill
0 siblings, 1 reply; 6+ messages in thread
From: Patrick Palka @ 2020-03-18 19:26 UTC (permalink / raw)
To: Patrick Palka; +Cc: gcc-patches, jason
On Wed, 18 Mar 2020, Patrick Palka wrote:
> When diagnosing a constraint error, we currently try to print the constraint
> inside a diagnostic constraint context with its template arguments substituted
> in. If substitution fails, then we instead just print the dependent
> form, as in the the test case below:
>
> gcc/testsuite/g++.dg/concepts/diagnostic6.C:14:15: error: static assertion failed
> 14 | static_assert(E<int>); // { dg-error "static assertion failed|not a class" }
> | ^~~~~~
> gcc/testsuite/g++.dg/concepts/diagnostic6.C:14:15: note: constraints not satisfied
> gcc/testsuite/g++.dg/concepts/diagnostic6.C:4:11: required for the satisfaction of ‘C<T>’
> gcc/testsuite/g++.dg/concepts/diagnostic6.C:8:11: required for the satisfaction of ‘D<typename T::type>’
> gcc/testsuite/g++.dg/concepts/diagnostic6.C:14:15: error: ‘int’ is not a class, struct, or union type
>
> But printing just the dependent form sometimes makes it difficult to decipher
> the diagnostic. In the above example, for instance, there's no indication of
> how the template argument 'int' relates to either of the 'T's.
>
> This patch improves the situation by changing these diagnostics to always print
> the dependent form of the constraint, and alongside it the (preferably
> substituted) constraint parameter mapping. So with the same test case below we
> now get:
>
> gcc/testsuite/g++.dg/concepts/diagnostic6.C:14:15: error: static assertion failed
> 14 | static_assert(E<int>); // { dg-error "static assertion failed|not a class" }
> | ^~~~~~
> gcc/testsuite/g++.dg/concepts/diagnostic6.C:14:15: note: constraints not satisfied
> gcc/testsuite/g++.dg/concepts/diagnostic6.C:4:11: required for the satisfaction of ‘C<T>’ [with T = typename T::type]
> gcc/testsuite/g++.dg/concepts/diagnostic6.C:8:11: required for the satisfaction of ‘D<typename T::type>’ [with T = int]
> gcc/testsuite/g++.dg/concepts/diagnostic6.C:14:15: error: ‘int’ is not a class, struct, or union type
>
> This change arguably makes it easier to figure out what's going on whenever a
> constraint fails due to substitution resulting in an invalid type rather than
> failing due to the constraint evaluating to false.
>
> Tested on x86_64-pc-linux-gnu, does this look reasonable? I'm not sure if
> printing an unsubstituted parameter mapping (like in the line 4 message above)
> is always useful, but perhaps it's better than nothing?
>
Ah sorry, this is an old version of the patch. Please consider this one
instead:
-- >8 --
gcc/cp/ChangeLog:
* cxx-pretty-print.c (pp_cxx_parameter_mapping): Make extern.
* cxx-pretty-print.h (pp_cxx_parameter_mapping): Declare.
* error.c (rebuild_concept_check): Delete.
(print_concept_check_info): Print the dependent form of the constraint and the
preferably substituted parameter mapping alongside it.
gcc/testsuite/ChangeLog:
* g++.dg/concepts/diagnostic6.C: New test.
---
gcc/cp/cxx-pretty-print.c | 2 +-
gcc/cp/cxx-pretty-print.h | 1 +
gcc/cp/error.c | 41 ++++++++-------------
gcc/testsuite/g++.dg/concepts/diagnostic6.C | 14 +++++++
4 files changed, 32 insertions(+), 26 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/concepts/diagnostic6.C
diff --git a/gcc/cp/cxx-pretty-print.c b/gcc/cp/cxx-pretty-print.c
index 100154e400f..1e89c3f9033 100644
--- a/gcc/cp/cxx-pretty-print.c
+++ b/gcc/cp/cxx-pretty-print.c
@@ -2878,7 +2878,7 @@ pp_cxx_check_constraint (cxx_pretty_printer *pp, tree t)
/* Output the "[with ...]" clause for a parameter mapping of an atomic
constraint. */
-static void
+void
pp_cxx_parameter_mapping (cxx_pretty_printer *pp, tree map)
{
for (tree p = map; p; p = TREE_CHAIN (p))
diff --git a/gcc/cp/cxx-pretty-print.h b/gcc/cp/cxx-pretty-print.h
index 7c7347f57ba..494f3fdde59 100644
--- a/gcc/cp/cxx-pretty-print.h
+++ b/gcc/cp/cxx-pretty-print.h
@@ -112,5 +112,6 @@ void pp_cxx_conjunction (cxx_pretty_printer *, tree);
void pp_cxx_disjunction (cxx_pretty_printer *, tree);
void pp_cxx_constraint (cxx_pretty_printer *, tree);
void pp_cxx_constrained_type_spec (cxx_pretty_printer *, tree);
+void pp_cxx_parameter_mapping (cxx_pretty_printer *, tree);
#endif /* GCC_CXX_PRETTY_PRINT_H */
diff --git a/gcc/cp/error.c b/gcc/cp/error.c
index cc51b6ffe13..fc08558e9b6 100644
--- a/gcc/cp/error.c
+++ b/gcc/cp/error.c
@@ -3680,27 +3680,6 @@ print_location (diagnostic_context *context, location_t loc)
"locus", xloc.file, xloc.line);
}
-/* Instantiate the concept check for the purpose of diagnosing an error. */
-
-static tree
-rebuild_concept_check (tree expr, tree map, tree args)
-{
- /* Instantiate the parameter mapping for the template-id. */
- map = tsubst_parameter_mapping (map, args, tf_none, NULL_TREE);
- if (map == error_mark_node)
- return error_mark_node;
- args = get_mapped_args (map);
-
- /* Rebuild the template id using substituted arguments. Substituting
- directly through the expression will trigger recursive satisfaction,
- so don't do that. */
- tree id = unpack_concept_check (expr);
- args = tsubst_template_args (TREE_OPERAND (id, 1), args, tf_none, NULL_TREE);
- if (args == error_mark_node)
- return error_mark_node;
- return build_nt (TEMPLATE_ID_EXPR, TREE_OPERAND (id, 0), args);
-}
-
static void
print_constrained_decl_info (diagnostic_context *context, tree decl)
{
@@ -3717,12 +3696,24 @@ print_concept_check_info (diagnostic_context *context, tree expr, tree map, tree
tree tmpl = TREE_OPERAND (id, 0);
if (OVL_P (tmpl))
tmpl = OVL_FIRST (tmpl);
- tree check = rebuild_concept_check (expr, map, args);
- if (check == error_mark_node)
- check = expr;
print_location (context, DECL_SOURCE_LOCATION (tmpl));
- pp_verbatim (context->printer, "required for the satisfaction of %qE\n", check);
+
+ cxx_pretty_printer *pp = (cxx_pretty_printer *)context->printer;
+ pp_verbatim (pp, "required for the satisfaction of %qE", expr);
+ tree subst_map = tsubst_parameter_mapping (map, args, tf_none, NULL_TREE);
+ if (subst_map != error_mark_node)
+ map = subst_map;
+ if (map)
+ {
+ pp_cxx_whitespace (pp);
+ pp_cxx_left_bracket (pp);
+ pp->translate_string ("with");
+ pp_cxx_whitespace (pp);
+ pp_cxx_parameter_mapping (pp, map);
+ pp_cxx_right_bracket (pp);
+ }
+ pp_newline (pp);
}
/* Diagnose the entry point into the satisfaction error. Returns the next
diff --git a/gcc/testsuite/g++.dg/concepts/diagnostic6.C b/gcc/testsuite/g++.dg/concepts/diagnostic6.C
new file mode 100644
index 00000000000..06b17caccbe
--- /dev/null
+++ b/gcc/testsuite/g++.dg/concepts/diagnostic6.C
@@ -0,0 +1,14 @@
+// { dg-do compile { target c++2a } }
+
+template<typename T>
+ concept C = requires (T t) { t + 0; };
+// { dg-message "satisfaction of .C<T>. .with T = typename T::type." "" { target *-*-* } .-1 }
+
+template<typename T>
+ concept D = C<T>;
+// { dg-message "satisfaction of .D<typename T::type>. .with T = int." "" { target *-*-* } .-1 }
+
+template<typename T>
+ concept E = D<typename T::type>;
+
+static_assert(E<int>); // { dg-error "static assertion failed|not a class" }
--
2.26.0.rc1.11.g30e9940356
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] c++: Include the constraint parameter mapping in diagnostic constraint contexts
2020-03-18 19:26 ` Patrick Palka
@ 2020-03-19 14:31 ` Jason Merrill
2020-03-19 16:50 ` Patrick Palka
0 siblings, 1 reply; 6+ messages in thread
From: Jason Merrill @ 2020-03-19 14:31 UTC (permalink / raw)
To: Patrick Palka; +Cc: gcc-patches
On 3/18/20 3:26 PM, Patrick Palka wrote:
> + if (map)
> + {
> + pp_cxx_whitespace (pp);
> + pp_cxx_left_bracket (pp);
> + pp->translate_string ("with");
> + pp_cxx_whitespace (pp);
> + pp_cxx_parameter_mapping (pp, map);
> + pp_cxx_right_bracket (pp);
> + }
Perhaps we should move the [with ] bits into pp_cxx_parameter_mapping
rather than duplicate them here.
Jason
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] c++: Include the constraint parameter mapping in diagnostic constraint contexts
2020-03-19 14:31 ` Jason Merrill
@ 2020-03-19 16:50 ` Patrick Palka
2020-03-19 20:24 ` Jason Merrill
0 siblings, 1 reply; 6+ messages in thread
From: Patrick Palka @ 2020-03-19 16:50 UTC (permalink / raw)
To: Jason Merrill; +Cc: Patrick Palka, gcc-patches
On Thu, 19 Mar 2020, Jason Merrill wrote:
> On 3/18/20 3:26 PM, Patrick Palka wrote:
> > + if (map)
> > + {
> > + pp_cxx_whitespace (pp);
> > + pp_cxx_left_bracket (pp);
> > + pp->translate_string ("with");
> > + pp_cxx_whitespace (pp);
> > + pp_cxx_parameter_mapping (pp, map);
> > + pp_cxx_right_bracket (pp);
> > + }
>
> Perhaps we should move the [with ] bits into pp_cxx_parameter_mapping rather
> than duplicate them here.
Like this?
-- >8 --
gcc/cp/ChangeLog:
* cxx-pretty-print.c (pp_cxx_parameter_mapping): Make extern. Move
the "[with ]" bits to here from ...
(pp_cxx_atomic_constraint): ... here.
* cxx-pretty-print.h (pp_cxx_parameter_mapping): Declare.
* error.c (rebuild_concept_check): Delete.
(print_concept_check_info): Print the dependent form of the constraint and the
preferably substituted parameter mapping alongside it.
gcc/testsuite/ChangeLog:
* g++.dg/concepts/diagnostic6.C: New test.
---
gcc/cp/cxx-pretty-print.c | 12 ++++---
gcc/cp/cxx-pretty-print.h | 1 +
gcc/cp/error.c | 36 +++++++--------------
gcc/testsuite/g++.dg/concepts/diagnostic6.C | 14 ++++++++
4 files changed, 33 insertions(+), 30 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/concepts/diagnostic6.C
diff --git a/gcc/cp/cxx-pretty-print.c b/gcc/cp/cxx-pretty-print.c
index 100154e400f..15d8b7eb609 100644
--- a/gcc/cp/cxx-pretty-print.c
+++ b/gcc/cp/cxx-pretty-print.c
@@ -2878,9 +2878,13 @@ pp_cxx_check_constraint (cxx_pretty_printer *pp, tree t)
/* Output the "[with ...]" clause for a parameter mapping of an atomic
constraint. */
-static void
+void
pp_cxx_parameter_mapping (cxx_pretty_printer *pp, tree map)
{
+ pp_cxx_left_bracket (pp);
+ pp->translate_string ("with");
+ pp_cxx_whitespace (pp);
+
for (tree p = map; p; p = TREE_CHAIN (p))
{
tree parm = TREE_VALUE (p);
@@ -2903,6 +2907,8 @@ pp_cxx_parameter_mapping (cxx_pretty_printer *pp, tree map)
if (TREE_CHAIN (p) != NULL_TREE)
pp_cxx_separate_with (pp, ';');
}
+
+ pp_cxx_right_bracket (pp);
}
void
@@ -2915,12 +2921,8 @@ pp_cxx_atomic_constraint (cxx_pretty_printer *pp, tree t)
tree map = ATOMIC_CONSTR_MAP (t);
if (map && map != error_mark_node)
{
- pp_cxx_whitespace (pp);
- pp_cxx_left_bracket (pp);
- pp->translate_string ("with");
pp_cxx_whitespace (pp);
pp_cxx_parameter_mapping (pp, map);
- pp_cxx_right_bracket (pp);
}
}
diff --git a/gcc/cp/cxx-pretty-print.h b/gcc/cp/cxx-pretty-print.h
index 7c7347f57ba..494f3fdde59 100644
--- a/gcc/cp/cxx-pretty-print.h
+++ b/gcc/cp/cxx-pretty-print.h
@@ -112,5 +112,6 @@ void pp_cxx_conjunction (cxx_pretty_printer *, tree);
void pp_cxx_disjunction (cxx_pretty_printer *, tree);
void pp_cxx_constraint (cxx_pretty_printer *, tree);
void pp_cxx_constrained_type_spec (cxx_pretty_printer *, tree);
+void pp_cxx_parameter_mapping (cxx_pretty_printer *, tree);
#endif /* GCC_CXX_PRETTY_PRINT_H */
diff --git a/gcc/cp/error.c b/gcc/cp/error.c
index cc51b6ffe13..9f19e9bbaa4 100644
--- a/gcc/cp/error.c
+++ b/gcc/cp/error.c
@@ -3680,27 +3680,6 @@ print_location (diagnostic_context *context, location_t loc)
"locus", xloc.file, xloc.line);
}
-/* Instantiate the concept check for the purpose of diagnosing an error. */
-
-static tree
-rebuild_concept_check (tree expr, tree map, tree args)
-{
- /* Instantiate the parameter mapping for the template-id. */
- map = tsubst_parameter_mapping (map, args, tf_none, NULL_TREE);
- if (map == error_mark_node)
- return error_mark_node;
- args = get_mapped_args (map);
-
- /* Rebuild the template id using substituted arguments. Substituting
- directly through the expression will trigger recursive satisfaction,
- so don't do that. */
- tree id = unpack_concept_check (expr);
- args = tsubst_template_args (TREE_OPERAND (id, 1), args, tf_none, NULL_TREE);
- if (args == error_mark_node)
- return error_mark_node;
- return build_nt (TEMPLATE_ID_EXPR, TREE_OPERAND (id, 0), args);
-}
-
static void
print_constrained_decl_info (diagnostic_context *context, tree decl)
{
@@ -3717,12 +3696,19 @@ print_concept_check_info (diagnostic_context *context, tree expr, tree map, tree
tree tmpl = TREE_OPERAND (id, 0);
if (OVL_P (tmpl))
tmpl = OVL_FIRST (tmpl);
- tree check = rebuild_concept_check (expr, map, args);
- if (check == error_mark_node)
- check = expr;
print_location (context, DECL_SOURCE_LOCATION (tmpl));
- pp_verbatim (context->printer, "required for the satisfaction of %qE\n", check);
+
+ cxx_pretty_printer *pp = (cxx_pretty_printer *)context->printer;
+ pp_verbatim (pp, "required for the satisfaction of %qE", expr);
+ if (map)
+ {
+ tree subst_map = tsubst_parameter_mapping (map, args, tf_none, NULL_TREE);
+ pp_cxx_whitespace (pp);
+ pp_cxx_parameter_mapping (pp, (subst_map != error_mark_node
+ ? subst_map : map));
+ }
+ pp_newline (pp);
}
/* Diagnose the entry point into the satisfaction error. Returns the next
diff --git a/gcc/testsuite/g++.dg/concepts/diagnostic6.C b/gcc/testsuite/g++.dg/concepts/diagnostic6.C
new file mode 100644
index 00000000000..06b17caccbe
--- /dev/null
+++ b/gcc/testsuite/g++.dg/concepts/diagnostic6.C
@@ -0,0 +1,14 @@
+// { dg-do compile { target c++2a } }
+
+template<typename T>
+ concept C = requires (T t) { t + 0; };
+// { dg-message "satisfaction of .C<T>. .with T = typename T::type." "" { target *-*-* } .-1 }
+
+template<typename T>
+ concept D = C<T>;
+// { dg-message "satisfaction of .D<typename T::type>. .with T = int." "" { target *-*-* } .-1 }
+
+template<typename T>
+ concept E = D<typename T::type>;
+
+static_assert(E<int>); // { dg-error "static assertion failed|not a class" }
--
2.26.0.rc1.11.g30e9940356
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] c++: Include the constraint parameter mapping in diagnostic constraint contexts
2020-03-19 16:50 ` Patrick Palka
@ 2020-03-19 20:24 ` Jason Merrill
2020-03-20 14:23 ` Patrick Palka
0 siblings, 1 reply; 6+ messages in thread
From: Jason Merrill @ 2020-03-19 20:24 UTC (permalink / raw)
To: Patrick Palka; +Cc: gcc-patches
On 3/19/20 12:50 PM, Patrick Palka wrote:
> On Thu, 19 Mar 2020, Jason Merrill wrote:
>
>> On 3/18/20 3:26 PM, Patrick Palka wrote:
>>> + if (map)
>>> + {
>>> + pp_cxx_whitespace (pp);
>>> + pp_cxx_left_bracket (pp);
>>> + pp->translate_string ("with");
>>> + pp_cxx_whitespace (pp);
>>> + pp_cxx_parameter_mapping (pp, map);
>>> + pp_cxx_right_bracket (pp);
>>> + }
>>
>> Perhaps we should move the [with ] bits into pp_cxx_parameter_mapping rather
>> than duplicate them here.
>
> Like this?
>
> -- >8 --
>
> gcc/cp/ChangeLog:
>
> * cxx-pretty-print.c (pp_cxx_parameter_mapping): Make extern. Move
> the "[with ]" bits to here from ...
> (pp_cxx_atomic_constraint): ... here.
> * cxx-pretty-print.h (pp_cxx_parameter_mapping): Declare.
> * error.c (rebuild_concept_check): Delete.
> (print_concept_check_info): Print the dependent form of the constraint and the
> preferably substituted parameter mapping alongside it.
>
> gcc/testsuite/ChangeLog:
>
> * g++.dg/concepts/diagnostic6.C: New test.
> ---
> gcc/cp/cxx-pretty-print.c | 12 ++++---
> gcc/cp/cxx-pretty-print.h | 1 +
> gcc/cp/error.c | 36 +++++++--------------
> gcc/testsuite/g++.dg/concepts/diagnostic6.C | 14 ++++++++
> 4 files changed, 33 insertions(+), 30 deletions(-)
> create mode 100644 gcc/testsuite/g++.dg/concepts/diagnostic6.C
>
> diff --git a/gcc/cp/cxx-pretty-print.c b/gcc/cp/cxx-pretty-print.c
> index 100154e400f..15d8b7eb609 100644
> --- a/gcc/cp/cxx-pretty-print.c
> +++ b/gcc/cp/cxx-pretty-print.c
> @@ -2878,9 +2878,13 @@ pp_cxx_check_constraint (cxx_pretty_printer *pp, tree t)
> /* Output the "[with ...]" clause for a parameter mapping of an atomic
> constraint. */
>
> -static void
> +void
> pp_cxx_parameter_mapping (cxx_pretty_printer *pp, tree map)
> {
> + pp_cxx_left_bracket (pp);
> + pp->translate_string ("with");
> + pp_cxx_whitespace (pp);
> +
> for (tree p = map; p; p = TREE_CHAIN (p))
> {
> tree parm = TREE_VALUE (p);
> @@ -2903,6 +2907,8 @@ pp_cxx_parameter_mapping (cxx_pretty_printer *pp, tree map)
> if (TREE_CHAIN (p) != NULL_TREE)
> pp_cxx_separate_with (pp, ';');
> }
> +
> + pp_cxx_right_bracket (pp);
> }
>
> void
> @@ -2915,12 +2921,8 @@ pp_cxx_atomic_constraint (cxx_pretty_printer *pp, tree t)
> tree map = ATOMIC_CONSTR_MAP (t);
> if (map && map != error_mark_node)
> {
> - pp_cxx_whitespace (pp);
> - pp_cxx_left_bracket (pp);
> - pp->translate_string ("with");
> pp_cxx_whitespace (pp);
Is there a reason not to move the initial whitespace as well, like in
dump_substitution? OK with that change.
> pp_cxx_parameter_mapping (pp, map);
> - pp_cxx_right_bracket (pp);
> }
> }
>
> diff --git a/gcc/cp/cxx-pretty-print.h b/gcc/cp/cxx-pretty-print.h
> index 7c7347f57ba..494f3fdde59 100644
> --- a/gcc/cp/cxx-pretty-print.h
> +++ b/gcc/cp/cxx-pretty-print.h
> @@ -112,5 +112,6 @@ void pp_cxx_conjunction (cxx_pretty_printer *, tree);
> void pp_cxx_disjunction (cxx_pretty_printer *, tree);
> void pp_cxx_constraint (cxx_pretty_printer *, tree);
> void pp_cxx_constrained_type_spec (cxx_pretty_printer *, tree);
> +void pp_cxx_parameter_mapping (cxx_pretty_printer *, tree);
>
> #endif /* GCC_CXX_PRETTY_PRINT_H */
> diff --git a/gcc/cp/error.c b/gcc/cp/error.c
> index cc51b6ffe13..9f19e9bbaa4 100644
> --- a/gcc/cp/error.c
> +++ b/gcc/cp/error.c
> @@ -3680,27 +3680,6 @@ print_location (diagnostic_context *context, location_t loc)
> "locus", xloc.file, xloc.line);
> }
>
> -/* Instantiate the concept check for the purpose of diagnosing an error. */
> -
> -static tree
> -rebuild_concept_check (tree expr, tree map, tree args)
> -{
> - /* Instantiate the parameter mapping for the template-id. */
> - map = tsubst_parameter_mapping (map, args, tf_none, NULL_TREE);
> - if (map == error_mark_node)
> - return error_mark_node;
> - args = get_mapped_args (map);
> -
> - /* Rebuild the template id using substituted arguments. Substituting
> - directly through the expression will trigger recursive satisfaction,
> - so don't do that. */
> - tree id = unpack_concept_check (expr);
> - args = tsubst_template_args (TREE_OPERAND (id, 1), args, tf_none, NULL_TREE);
> - if (args == error_mark_node)
> - return error_mark_node;
> - return build_nt (TEMPLATE_ID_EXPR, TREE_OPERAND (id, 0), args);
> -}
> -
> static void
> print_constrained_decl_info (diagnostic_context *context, tree decl)
> {
> @@ -3717,12 +3696,19 @@ print_concept_check_info (diagnostic_context *context, tree expr, tree map, tree
> tree tmpl = TREE_OPERAND (id, 0);
> if (OVL_P (tmpl))
> tmpl = OVL_FIRST (tmpl);
> - tree check = rebuild_concept_check (expr, map, args);
> - if (check == error_mark_node)
> - check = expr;
>
> print_location (context, DECL_SOURCE_LOCATION (tmpl));
> - pp_verbatim (context->printer, "required for the satisfaction of %qE\n", check);
> +
> + cxx_pretty_printer *pp = (cxx_pretty_printer *)context->printer;
> + pp_verbatim (pp, "required for the satisfaction of %qE", expr);
> + if (map)
> + {
> + tree subst_map = tsubst_parameter_mapping (map, args, tf_none, NULL_TREE);
> + pp_cxx_whitespace (pp);
> + pp_cxx_parameter_mapping (pp, (subst_map != error_mark_node
> + ? subst_map : map));
> + }
> + pp_newline (pp);
> }
>
> /* Diagnose the entry point into the satisfaction error. Returns the next
> diff --git a/gcc/testsuite/g++.dg/concepts/diagnostic6.C b/gcc/testsuite/g++.dg/concepts/diagnostic6.C
> new file mode 100644
> index 00000000000..06b17caccbe
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/concepts/diagnostic6.C
> @@ -0,0 +1,14 @@
> +// { dg-do compile { target c++2a } }
> +
> +template<typename T>
> + concept C = requires (T t) { t + 0; };
> +// { dg-message "satisfaction of .C<T>. .with T = typename T::type." "" { target *-*-* } .-1 }
> +
> +template<typename T>
> + concept D = C<T>;
> +// { dg-message "satisfaction of .D<typename T::type>. .with T = int." "" { target *-*-* } .-1 }
> +
> +template<typename T>
> + concept E = D<typename T::type>;
> +
> +static_assert(E<int>); // { dg-error "static assertion failed|not a class" }
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] c++: Include the constraint parameter mapping in diagnostic constraint contexts
2020-03-19 20:24 ` Jason Merrill
@ 2020-03-20 14:23 ` Patrick Palka
0 siblings, 0 replies; 6+ messages in thread
From: Patrick Palka @ 2020-03-20 14:23 UTC (permalink / raw)
To: Jason Merrill; +Cc: Patrick Palka, gcc-patches
On Thu, 19 Mar 2020, Jason Merrill wrote:
> On 3/19/20 12:50 PM, Patrick Palka wrote:
> > On Thu, 19 Mar 2020, Jason Merrill wrote:
> >
> > > On 3/18/20 3:26 PM, Patrick Palka wrote:
> > > > + if (map)
> > > > + {
> > > > + pp_cxx_whitespace (pp);
> > > > + pp_cxx_left_bracket (pp);
> > > > + pp->translate_string ("with");
> > > > + pp_cxx_whitespace (pp);
> > > > + pp_cxx_parameter_mapping (pp, map);
> > > > + pp_cxx_right_bracket (pp);
> > > > + }
> > >
> > > Perhaps we should move the [with ] bits into pp_cxx_parameter_mapping
> > > rather
> > > than duplicate them here.
> >
> > Like this?
> >
> > -- >8 --
> >
> > gcc/cp/ChangeLog:
> >
> > * cxx-pretty-print.c (pp_cxx_parameter_mapping): Make extern. Move
> > the "[with ]" bits to here from ...
> > (pp_cxx_atomic_constraint): ... here.
> > * cxx-pretty-print.h (pp_cxx_parameter_mapping): Declare.
> > * error.c (rebuild_concept_check): Delete.
> > (print_concept_check_info): Print the dependent form of the constraint
> > and the
> > preferably substituted parameter mapping alongside it.
> >
> > gcc/testsuite/ChangeLog:
> >
> > * g++.dg/concepts/diagnostic6.C: New test.
> > ---
> > gcc/cp/cxx-pretty-print.c | 12 ++++---
> > gcc/cp/cxx-pretty-print.h | 1 +
> > gcc/cp/error.c | 36 +++++++--------------
> > gcc/testsuite/g++.dg/concepts/diagnostic6.C | 14 ++++++++
> > 4 files changed, 33 insertions(+), 30 deletions(-)
> > create mode 100644 gcc/testsuite/g++.dg/concepts/diagnostic6.C
> >
> > diff --git a/gcc/cp/cxx-pretty-print.c b/gcc/cp/cxx-pretty-print.c
> > index 100154e400f..15d8b7eb609 100644
> > --- a/gcc/cp/cxx-pretty-print.c
> > +++ b/gcc/cp/cxx-pretty-print.c
> > @@ -2878,9 +2878,13 @@ pp_cxx_check_constraint (cxx_pretty_printer *pp, tree
> > t)
> > /* Output the "[with ...]" clause for a parameter mapping of an atomic
> > constraint. */
> > -static void
> > +void
> > pp_cxx_parameter_mapping (cxx_pretty_printer *pp, tree map)
> > {
> > + pp_cxx_left_bracket (pp);
> > + pp->translate_string ("with");
> > + pp_cxx_whitespace (pp);
> > +
> > for (tree p = map; p; p = TREE_CHAIN (p))
> > {
> > tree parm = TREE_VALUE (p);
> > @@ -2903,6 +2907,8 @@ pp_cxx_parameter_mapping (cxx_pretty_printer *pp, tree
> > map)
> > if (TREE_CHAIN (p) != NULL_TREE)
> > pp_cxx_separate_with (pp, ';');
> > }
> > +
> > + pp_cxx_right_bracket (pp);
> > }
> > void
> > @@ -2915,12 +2921,8 @@ pp_cxx_atomic_constraint (cxx_pretty_printer *pp,
> > tree t)
> > tree map = ATOMIC_CONSTR_MAP (t);
> > if (map && map != error_mark_node)
> > {
> > - pp_cxx_whitespace (pp);
> > - pp_cxx_left_bracket (pp);
> > - pp->translate_string ("with");
> > pp_cxx_whitespace (pp);
>
> Is there a reason not to move the initial whitespace as well, like in
> dump_substitution? OK with that change.
No good reason :) Thanks, patch committed with that change.
>
> > pp_cxx_parameter_mapping (pp, map);
> > - pp_cxx_right_bracket (pp);
> > }
> > }
> > diff --git a/gcc/cp/cxx-pretty-print.h b/gcc/cp/cxx-pretty-print.h
> > index 7c7347f57ba..494f3fdde59 100644
> > --- a/gcc/cp/cxx-pretty-print.h
> > +++ b/gcc/cp/cxx-pretty-print.h
> > @@ -112,5 +112,6 @@ void pp_cxx_conjunction (cxx_pretty_printer *, tree);
> > void pp_cxx_disjunction (cxx_pretty_printer *, tree);
> > void pp_cxx_constraint (cxx_pretty_printer *, tree);
> > void pp_cxx_constrained_type_spec (cxx_pretty_printer *, tree);
> > +void pp_cxx_parameter_mapping (cxx_pretty_printer *, tree);
> > #endif /* GCC_CXX_PRETTY_PRINT_H */
> > diff --git a/gcc/cp/error.c b/gcc/cp/error.c
> > index cc51b6ffe13..9f19e9bbaa4 100644
> > --- a/gcc/cp/error.c
> > +++ b/gcc/cp/error.c
> > @@ -3680,27 +3680,6 @@ print_location (diagnostic_context *context,
> > location_t loc)
> > "locus", xloc.file, xloc.line);
> > }
> > -/* Instantiate the concept check for the purpose of diagnosing an error.
> > */
> > -
> > -static tree
> > -rebuild_concept_check (tree expr, tree map, tree args)
> > -{
> > - /* Instantiate the parameter mapping for the template-id. */
> > - map = tsubst_parameter_mapping (map, args, tf_none, NULL_TREE);
> > - if (map == error_mark_node)
> > - return error_mark_node;
> > - args = get_mapped_args (map);
> > -
> > - /* Rebuild the template id using substituted arguments. Substituting
> > - directly through the expression will trigger recursive satisfaction,
> > - so don't do that. */
> > - tree id = unpack_concept_check (expr);
> > - args = tsubst_template_args (TREE_OPERAND (id, 1), args, tf_none,
> > NULL_TREE);
> > - if (args == error_mark_node)
> > - return error_mark_node;
> > - return build_nt (TEMPLATE_ID_EXPR, TREE_OPERAND (id, 0), args);
> > -}
> > -
> > static void
> > print_constrained_decl_info (diagnostic_context *context, tree decl)
> > {
> > @@ -3717,12 +3696,19 @@ print_concept_check_info (diagnostic_context
> > *context, tree expr, tree map, tree
> > tree tmpl = TREE_OPERAND (id, 0);
> > if (OVL_P (tmpl))
> > tmpl = OVL_FIRST (tmpl);
> > - tree check = rebuild_concept_check (expr, map, args);
> > - if (check == error_mark_node)
> > - check = expr;
> > print_location (context, DECL_SOURCE_LOCATION (tmpl));
> > - pp_verbatim (context->printer, "required for the satisfaction of %qE\n",
> > check);
> > +
> > + cxx_pretty_printer *pp = (cxx_pretty_printer *)context->printer;
> > + pp_verbatim (pp, "required for the satisfaction of %qE", expr);
> > + if (map)
> > + {
> > + tree subst_map = tsubst_parameter_mapping (map, args, tf_none,
> > NULL_TREE);
> > + pp_cxx_whitespace (pp);
> > + pp_cxx_parameter_mapping (pp, (subst_map != error_mark_node
> > + ? subst_map : map));
> > + }
> > + pp_newline (pp);
> > }
> > /* Diagnose the entry point into the satisfaction error. Returns the
> > next
> > diff --git a/gcc/testsuite/g++.dg/concepts/diagnostic6.C
> > b/gcc/testsuite/g++.dg/concepts/diagnostic6.C
> > new file mode 100644
> > index 00000000000..06b17caccbe
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/concepts/diagnostic6.C
> > @@ -0,0 +1,14 @@
> > +// { dg-do compile { target c++2a } }
> > +
> > +template<typename T>
> > + concept C = requires (T t) { t + 0; };
> > +// { dg-message "satisfaction of .C<T>. .with T = typename T::type." "" {
> > target *-*-* } .-1 }
> > +
> > +template<typename T>
> > + concept D = C<T>;
> > +// { dg-message "satisfaction of .D<typename T::type>. .with T = int." "" {
> > target *-*-* } .-1 }
> > +
> > +template<typename T>
> > + concept E = D<typename T::type>;
> > +
> > +static_assert(E<int>); // { dg-error "static assertion failed|not a class"
> > }
> >
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-03-20 14:23 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-18 19:21 [PATCH] c++: Include the constraint parameter mapping in diagnostic constraint contexts Patrick Palka
2020-03-18 19:26 ` Patrick Palka
2020-03-19 14:31 ` Jason Merrill
2020-03-19 16:50 ` Patrick Palka
2020-03-19 20:24 ` Jason Merrill
2020-03-20 14:23 ` Patrick Palka
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).