On Wed, 5 Mar 2025, Jason Merrill wrote:
On 3/5/25 10:13 AM, Patrick Palka wrote:
On Tue, 4 Mar 2025, Jason Merrill wrote:
On 3/4/25 2:49 PM, Patrick Palka wrote:
Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK
for trunk/14?
-- >8 --
In the three-parameter version of satisfy_declaration_constraints, when
't' isn't the most general template, then 't' won't correspond with
'args' after we augment the latter via add_outermost_template_args, and
so the instantiation context that we push via push_tinst_level isn't
quite correct: 'args' is a complete set of template arguments, but 't'
is not necessarily the most general template. This manifests as
misleading diagnostic context lines when issuing a hard error (or a
constraint recursion error) that occurred during satisfaction, e.g. for
the below testcase without this patch we emit:
In substitution of '... void A<int>::f<U>() [with U = int]'
and with this patch we emit:
In substitution of '... void A<T>::f<U>() [with U = char; T = int]'.
This patch fixes this by always passing the most general template to
push_tinst_level.
That soungs good, but getting it by passing it back from
get_normalized_constraints_from_decl seems confusing; I'd think we should
calculate it in parallel to changing args to correspond to that template.
Hmm, won't that mean duplicating the template adjustment logic in
get_normalized_constraints_from_decl, which seems undesirable? The
function has many callers, some of which are for satisfaction where
targs are involved, and the rest are for subsumption where no targs are
involved, so I don't see a clean way of refactoring the code to avoid
duplication of the template adjustment logic. Right now the targ
adjustment logic is unfortunately duplicated across both overloads
of satisfy_declaration_constraints and it seems undesirable to add
more duplication.
Fair enough. Incidentally, I wonder why the two-parm overload doesn't call
the three-parm overload?
Maybe one way to reduce the duplication would be to go the other way and
move the targ adjustment logic to get_normalized_constraints_from_decl
as well (so that it has two out-parameters, 'gen_d' and 'gen_args').
The proposed patch then would be an incremental step towards that.
That makes sense, passing back something suitable for
add_outermost_template_args.
I tried combining the two overloads, and/or moving the targ adjustment
logic to get_normalized_constraints_from_decl, but I couldn't arrive at
a formulation that worked and I was happy with (i.e. didn't lead to more
code duplication than the original appproach).
In the meantime I noticed that this bug is more pervasive than I
thought, and leads to wrong diagnostic context lines printed even in the
case of ordinary satisfaction failure -- however the wrong diagnostic
lines are more annoying/noticable during a hard error or constraint
recursion where there's likely no other useful diagnostic lines that
might have the correct args printed.
So I adjusted the testcase in the original patch accordingly. Could the
following go in for now?
I also attached a diff of the output of all our concepts testcases
currently, before/after this patch. Each change seems like a clear
improvement/correction to me.
-- >8 --
Subject: [PATCH] c++: wrong targs in satisfaction diagnostic context line
[PR99214]
In the three-parameter version of satisfy_declaration_constraints, when
't' isn't the most general template, then 't' won't correspond with
'args' after we augment the latter via add_outermost_template_args, and
so the instantiation context that we push via push_tinst_level isn't
quite correct: 'args' is a complete set of template arguments, but 't'
is not necessarily the most general template. This manifests as
misleading diagnostic context lines when issuing a satisfaction failure
error, e.g. the below testcase without this patch we emit:
In substitution of '... void A<int>::f<U>() [with U = int]'
and with this patch we emit:
In substitution of '... void A<T>::f<U>() [with U = char; T = int]'.
This patch fixes this by always passing the most general template to
push_tinst_level.
PR c++/99214
gcc/cp/ChangeLog:
* constraint.cc (get_normalized_constraints_from_decl): New
optional out-parameter GEN_D.
(satisfy_declaration_constraints): Use it to pass the most
general version of T to push_tinst_level.
gcc/testsuite/ChangeLog:
* g++.dg/concepts/diagnostic20.C: New test.
---
gcc/cp/constraint.cc | 15 +++++++++++----
gcc/testsuite/g++.dg/concepts/diagnostic20.C | 14 ++++++++++++++
2 files changed, 25 insertions(+), 4 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/concepts/diagnostic20.C
diff --git a/gcc/cp/constraint.cc b/gcc/cp/constraint.cc
index a9caba8e2cc7..f688a99c5fd7 100644
--- a/gcc/cp/constraint.cc
+++ b/gcc/cp/constraint.cc
@@ -648,10 +648,13 @@ get_normalized_constraints_from_info (tree ci, tree
in_decl, bool diag = false)
return t;
}
-/* Returns the normalized constraints for the declaration D. */
+/* Returns the normalized constraints for the declaration D.
+ If GEN_D is non-NULL, sets *GEN_D to the most general version
+ of D that ultimately owns its constraints. */
static tree
-get_normalized_constraints_from_decl (tree d, bool diag = false)
+get_normalized_constraints_from_decl (tree d, bool diag = false,
+ tree *gen_d = nullptr)
{
tree tmpl;
tree decl;
@@ -716,6 +719,8 @@ get_normalized_constraints_from_decl (tree d, bool diag =
false)
tmpl = most_general_template (tmpl);
d = tmpl ? tmpl : decl;
+ if (gen_d)
+ *gen_d = d;
/* If we're not diagnosing errors, use cached constraints, if any. */
if (!diag)
@@ -2730,9 +2735,11 @@ satisfy_declaration_constraints (tree t, tree args,
sat_info info)
return boolean_true_node;
tree result = boolean_true_node;
- if (tree norm = get_normalized_constraints_from_decl (t, info.noisy ()))
+ tree gen_t;
+ if (tree norm = get_normalized_constraints_from_decl (t, info.noisy (),
+ &gen_t))
{
- if (!push_tinst_level (t, args))
+ if (!push_tinst_level (gen_t, args))
return result;
tree pattern = DECL_TEMPLATE_RESULT (t);
push_to_top_level ();
diff --git a/gcc/testsuite/g++.dg/concepts/diagnostic20.C
b/gcc/testsuite/g++.dg/concepts/diagnostic20.C
new file mode 100644
index 000000000000..d88000b342c3
--- /dev/null
+++ b/gcc/testsuite/g++.dg/concepts/diagnostic20.C
@@ -0,0 +1,14 @@
+// PR c++/99214
+// { dg-do compile { target c++20 } }
+
+template <class T>
+struct A {
+ template <class U> static void f() requires requires { T::fail; };
+};
+
+int main() {
+ A<int>::f<char>(); // { dg-error "no match" }
+}
+
+// This matches the context line "In substitution of '... [with U = char; T =
int]'"
+// { dg-message "U = char; T = int" "" { target *-*-* } 0 }
--
2.49.0.221.g485f5f8636
