On Wed, 9 Feb 2022, Jason Merrill wrote:
> On 2/9/22 11:36, Patrick Palka wrote:
> > 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..
>
> To be precise, you can look up the name in the substituted T.
>
> Slightly less precise, you could just include all candidates from any base;
> even if they came from a different direct base, they must have been introduced
> by another using-decl, so we'd only include too much if using-decls appear on
> both sides of the lookup.
Aha, makes sense. The less precise approach is still more precise than
the current approach of giving up entirely, and appears easy enough to
implement. How does the following look? Unfortunately I wasn't able to
come up with a testcase for this due to using-decl weirdness in
incomplete-class contexts, which I reported as PR104476.
-- >8 --
Subject: [PATCH] c++: memfn lookup consistency and dependent using-decls
Rather than not doing any filtering when filter_memfn_lookup encounters
a dependent USING_DECL, handle this case less conservatively by holding
on to the members in the new lookup set that come from a base, i.e. that
could plausibly have been introduced by that using-decl. This is still
imprecise, but it's closer to the correct answer than the previous behavior
was.
gcc/cp/ChangeLog:
* pt.cc (filter_memfn_lookup): Handle dependent USING_DECL
less imprecisely.
---
gcc/cp/pt.cc | 24 +++++++++++++++---------
1 file changed, 15 insertions(+), 9 deletions(-)
diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
index 86b6ddc634f..8fa85ba6d9c 100644
--- a/gcc/cp/pt.cc
+++ b/gcc/cp/pt.cc
@@ -16327,15 +16327,18 @@ filter_memfn_lookup (tree oldfns, tree newfns, tree
newtype)
/* Record all member functions from the old lookup set OLDFNS into
VISIBLE_SET. */
hash_set<tree> visible_set;
+ bool seen_dep_using = false;
for (tree fn : lkp_range (oldfns))
{
if (TREE_CODE (fn) == USING_DECL)
{
- /* FIXME: Punt on (dependent) USING_DECL for now; mapping
- a dependent USING_DECL to the member functions it introduces
- seems tricky. */
+ /* We imprecisely handle dependent using-decl by keeping all
+ members in the new lookup set that are defined from any base.
+ FIXME: Track which members are introduced by a dependent
+ using-decl more precisely, perhaps by performing another
+ lookup from the substituted USING_DECL_SCOPE. */
gcc_checking_assert (DECL_DEPENDENT_P (fn));
- return newfns;
+ seen_dep_using = true;
}
else
visible_set.add (fn);
@@ -16343,12 +16346,13 @@ filter_memfn_lookup (tree oldfns, tree newfns, tree
newtype)
/* Returns true iff (a less specialized version of) FN appeared in
the old lookup set OLDFNS. */
- auto visible_p = [newtype, &visible_set] (tree fn) {
+ auto visible_p = [newtype, seen_dep_using, &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);
+ using-decl; if it might have been introduced by a dependent
+ using-decl then just conservatively keep it, otherwise look
+ in the old lookup set for FN exactly. */
+ return seen_dep_using || visible_set.contains (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
@@ -16382,7 +16386,9 @@ filter_memfn_lookup (tree oldfns, tree newfns, tree
newtype)
filtered_fns = lookup_add (fn, filtered_fns);
filtered_size++;
}
- gcc_checking_assert (filtered_size == visible_set.elements ());
+ gcc_checking_assert (seen_dep_using
+ ? filtered_size >= visible_set.elements ()
+ : filtered_size == visible_set.elements ());
return filtered_fns;
}
--
2.35.1.46.g38062e73e0
