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.
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.
>
> > PR c++/99214
> >
> > gcc/cp/ChangeLog:
> >
> > * constraint.cc (get_normalized_constraints_from_decl): New
> > 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 | 15 +++++++++++++++
> > 2 files changed, 26 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 a9caba8e2cc..f688a99c5fd 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 00000000000..b8d586e9a21
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/concepts/diagnostic20.C
> > @@ -0,0 +1,15 @@
> > +// PR c++/99214
> > +// { dg-do compile { target c++20 } }
> > +
> > +template <class T>
> > +struct A {
> > + template <class U> static void f() requires ([] { return U::fail; }());
> > // { dg-error "fail" }
> > + template <class U> static void f();
> > +};
> > +
> > +int main() {
> > + A<int>::f<char>();
> > +}
> > +
> > +// This matches the context line "In substitution of '... [with U = char; T
> > = int]'"
> > +// { dg-message "U = char; T = int" "" { target *-*-* } 0 }
>
>
