On Wed, 9 Feb 2022, Jason Merrill wrote:
> On 2/9/22 10:45, Patrick Palka wrote:
> > In filter_memfn_lookup, we weren't correctly recognizing and matching up
> > member functions introduced via a non-dependent using-decl. This caused
> > us to crash in the below testcases in which we correctly pruned the
> > overload set for the non-dependent call ahead of time, but then at
> > instantiation time filter_memfn_lookup failed to match the selected
> > function (introduced in each case by a non-dependent using-decl) to the
> > corresponding function from the new lookup set. Such member functions
> > need special handling in filter_memfn_lookup because they look exactly
> > the same in the old and new lookup sets, whereas ordinary member
> > functions that're defined in the (dependent) current class become more
> > specialized in the new lookup set.
> >
> > This patch reworks the matching logic in filter_memfn_lookup so that it
> > handles non-dependent using-decls correctly, and is hopefully simpler to
> > follow.
> >
> > Bootstrapped and regtested on x86_64-pc-linux, does this look OK for
> > trunk?
> >
> > PR c++/104432
> >
> > gcc/cp/ChangeLog:
> >
> > * call.cc (build_new_method_call): When a non-dependent call
> > resolves to a specialization of a member template, always build
> > the pruned overload set using the member template, not the
> > specialization.
> > * pt.cc (filter_memfn_lookup): New parameter newtype. Simplify
> > and correct how members from the new lookup set are matched to
> > those from the old one.
> > (tsubst_baselink): Pass binfo_type as newtype to
> > filter_memfn_lookup.
> >
> > gcc/testsuite/ChangeLog:
> >
> > * g++.dg/template/non-dependent19.C: New test.
> > * g++.dg/template/non-dependent19a.C: New test.
> > * g++.dg/template/non-dependent20.C: New test.
> > ---
> > gcc/cp/call.cc | 9 ++--
> > gcc/cp/pt.cc | 49 +++++++++----------
> > .../g++.dg/template/non-dependent19.C | 14 ++++++
> > .../g++.dg/template/non-dependent19a.C | 16 ++++++
> > .../g++.dg/template/non-dependent20.C | 16 ++++++
> > 5 files changed, 73 insertions(+), 31 deletions(-)
> > create mode 100644 gcc/testsuite/g++.dg/template/non-dependent19.C
> > create mode 100644 gcc/testsuite/g++.dg/template/non-dependent19a.C
> > create mode 100644 gcc/testsuite/g++.dg/template/non-dependent20.C
> >
> > diff --git a/gcc/cp/call.cc b/gcc/cp/call.cc
> > index b2e89c5d783..d6eed5ed835 100644
> > --- a/gcc/cp/call.cc
> > +++ b/gcc/cp/call.cc
> > @@ -11189,12 +11189,11 @@ build_new_method_call (tree instance, tree fns,
> > vec<tree, va_gc> **args,
> > if (really_overloaded_fn (fns))
> > {
> > if (DECL_TEMPLATE_INFO (fn)
> > - && DECL_MEMBER_TEMPLATE_P (DECL_TI_TEMPLATE (fn))
> > - && dependent_type_p (DECL_CONTEXT (fn)))
> > + && DECL_MEMBER_TEMPLATE_P (DECL_TI_TEMPLATE (fn)))
> > {
> > - /* FIXME: We're not prepared to fully instantiate "inside-out"
> > - partial instantiations such as A<T>::f<int>(). So instead
> > - use the selected template, not the specialization. */
> > + /* Use the selected template, not the specialization, so that
> > + this looks like an actual lookup result for sake of
> > + filter_memfn_lookup. */
> > if (OVL_SINGLE_P (fns))
> > /* If the original overload set consists of a single function
> > diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
> > index 862f337886c..3a5d06bf297 100644
> > --- a/gcc/cp/pt.cc
> > +++ b/gcc/cp/pt.cc
> > @@ -16311,12 +16311,12 @@ tsubst (tree t, tree args, tsubst_flags_t
> > complain, tree in_decl)
> > }
> > /* OLDFNS is a lookup set of member functions from some class template,
> > and
> > - NEWFNS is a lookup set of member functions from a specialization of that
> > - class template. Return the subset of NEWFNS which are specializations
> > of
> > - a function from OLDFNS. */
> > + NEWFNS is a lookup set of member functions from NEWTYPE, a
> > specialization
> > + of that class template. Return the subset of NEWFNS which are
> > + specializations of a function from OLDFNS. */
> > static tree
> > -filter_memfn_lookup (tree oldfns, tree newfns)
> > +filter_memfn_lookup (tree oldfns, tree newfns, tree newtype)
> > {
> > /* Record all member functions from the old lookup set OLDFNS into
> > VISIBLE_SET. */
> > @@ -16326,38 +16326,34 @@ filter_memfn_lookup (tree oldfns, tree newfns)
> > if (TREE_CODE (fn) == USING_DECL)
> > {
> > /* FIXME: Punt on (dependent) USING_DECL for now; mapping
> > - a dependent USING_DECL to its instantiation seems
> > - tricky. */
> > + a dependent USING_DECL to the member functions it introduces
> > + seems tricky. */
>
> FWIW I still think this shouldn't be very tricky.
I tried implementing this by substituting into the USING_DECL_SCOPE and
then during the filtering step keeping the member functions from the new
lookup set whose DECL_CONTEXT is the same as the substituted scope, i.e.
keeping the member functions from the new lookup set that're defined in
the base class named by the using-decl.
This seems to work but not in all cases. The problem is that a member
function introduced by a using-decl could be defined in a base of the
base class named by the using-decl. For example:
template<class T>
struct A : T
{
using T::f;
decltype(f()) g(); // filter_memfn_lookup with T=C needs to map
// USING_DECL from the old lookup set to B::f
// from the new lookup set, but the substituted
// USING_DECL_SCOPE is C.
};
struct B {
static void f();
};
struct C : B { };
template struct A<C>;
So I'm not sure what's the right approach for handling dependent
USING_DECL in light of this example..
>
> The patch is OK.
Thanks a lot, committed as r12-7134.
>
> > gcc_checking_assert (DECL_DEPENDENT_P (fn));
> > return newfns;
> > }
> > - else if (TREE_CODE (fn) == TEMPLATE_DECL)
> > - /* A member function template. */
> > - visible_set.add (fn);
> > - else if (TREE_CODE (fn) == FUNCTION_DECL)
> > - {
> > - if (DECL_TEMPLATE_INFO (fn))
> > - /* A non-template member function. */
> > - visible_set.add (DECL_TI_TEMPLATE (fn));
> > - else
> > - /* A non-template member function from a non-template base,
> > - injected via a using-decl. */
> > - visible_set.add (fn);
> > - }
> > else
> > - gcc_unreachable ();
> > + visible_set.add (fn);
> > }
> > /* Returns true iff (a less specialized version of) FN appeared in
> > the old lookup set OLDFNS. */
> > - auto visible_p = [&visible_set] (tree fn) {
> > - if (TREE_CODE (fn) == FUNCTION_DECL
> > - && !DECL_TEMPLATE_INFO (fn))
> > + auto visible_p = [newtype, &visible_set] (tree fn) {
> > + if (DECL_CONTEXT (fn) != newtype)
> > + /* FN is a member function from a base class, introduced via a
> > + non-dependent using-decl; look in the old lookup set for
> > + FN exactly. */
> > return visible_set.contains (fn);
> > - else if (DECL_TEMPLATE_INFO (fn))
> > + else if (TREE_CODE (fn) == TEMPLATE_DECL)
> > + /* FN is a member function template from the current class;
> > + look in the old lookup set for the TEMPLATE_DECL from which
> > + it was specialized. */
> > return visible_set.contains (DECL_TI_TEMPLATE (fn));
> > else
> > - gcc_unreachable ();
> > + /* FN is a non-template member function from the current class;
> > + look in the old lookup set for the FUNCTION_DECL from which
> > + it was specialized. */
> > + return visible_set.contains (DECL_TEMPLATE_RESULT
> > + (DECL_TI_TEMPLATE (fn)));
> > };
> > bool lookup_changed_p = false;
> > @@ -16449,7 +16445,8 @@ tsubst_baselink (tree baselink, tree object_type,
> > performed in an incomplete-class context, within which
> > later-declared members ought to remain invisible. */
> > BASELINK_FUNCTIONS (baselink)
> > - = filter_memfn_lookup (fns, BASELINK_FUNCTIONS (baselink));
> > + = filter_memfn_lookup (fns, BASELINK_FUNCTIONS (baselink),
> > + binfo_type);
> > BASELINK_FUNCTIONS_MAYBE_INCOMPLETE_P (baselink) = true;
> > }
> > diff --git a/gcc/testsuite/g++.dg/template/non-dependent19.C
> > b/gcc/testsuite/g++.dg/template/non-dependent19.C
> > new file mode 100644
> > index 00000000000..d690e80f2eb
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/template/non-dependent19.C
> > @@ -0,0 +1,14 @@
> > +// PR c++/104432
> > +// { dg-do compile { target c++11 } }
> > +
> > +struct A {
> > + template<class=int> void f();
> > + void f(int);
> > +};
> > +
> > +template<class> struct B : A {
> > + using A::f;
> > + void g() { f(); }
> > +};
> > +
> > +template struct B<int>;
> > diff --git a/gcc/testsuite/g++.dg/template/non-dependent19a.C
> > b/gcc/testsuite/g++.dg/template/non-dependent19a.C
> > new file mode 100644
> > index 00000000000..2f523c81988
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/template/non-dependent19a.C
> > @@ -0,0 +1,16 @@
> > +// PR c++/104432
> > +// { dg-do compile { target c++11 } }
> > +// A variant of non-dependent19.C where A is a template.
> > +
> > +template<class>
> > +struct A {
> > + template<class=int> void f();
> > + void f(int);
> > +};
> > +
> > +template<class> struct B : A<int> {
> > + using A<int>::f;
> > + void g() { f(); }
> > +};
> > +
> > +template struct B<int>;
> > diff --git a/gcc/testsuite/g++.dg/template/non-dependent20.C
> > b/gcc/testsuite/g++.dg/template/non-dependent20.C
> > new file mode 100644
> > index 00000000000..ebf7d7f40fb
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/template/non-dependent20.C
> > @@ -0,0 +1,16 @@
> > +// PR c++/104432
> > +// { dg-do compile { target c++11 } }
> > +
> > +template<class T>
> > +struct A {
> > + template<class=int> static void f(T);
> > + static void f();
> > +};
> > +
> > +template<class> struct B : A<int>, A<int&> {
> > + using A<int>::f;
> > + using A<int&>::f;
> > + void g() { f(0); }
> > +};
> > +
> > +template struct B<int>;
>
>
