Ping for https://gcc.gnu.org/pipermail/gcc-patches/2024-June/654792.html
On Sun, Jun 16, 2024 at 3:41 PM Nathaniel Shead
<nathanielosh...@gmail.com> wrote:
>
> On Sun, Jun 16, 2024 at 12:29:23PM +1000, Nathaniel Shead wrote:
> > Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk?
> >
> > This probably isn't the most efficient approach, since we need to do
> > name lookup to find deduction guides for a type which will also
> > potentially do a bunch of pointless lazy loading from imported modules,
> > but I wasn't able to work out a better approach without completely
> > reworking how deduction guides are stored and represented. This way at
> > least is correct (I believe), and we can maybe revisit this in the
> > future.
> >
> > -- >8 --
> >
> > Deduction guides are represented as 'normal' functions currently, and
> > have no special handling in modules. However, this causes some issues;
> > by [temp.deduct.guide] a deduction guide is not found by normal name
> > lookup and instead all reachable deduction guides for a class template
> > should be considered, but this does not happen currently.
> >
> > To solve this, this patch ensures that all deduction guides are
> > considered exported to ensure that they are always visible to importers
> > if they are reachable. Another alternative here would be to add a new
> > kind of "all reachable" flag to name lookup, but that is complicated by
> > some difficulties in handling GM entities; this may be a better way to
> > go if more kinds of entities end up needing this handling, however.
> >
> > Another issue here is that because deduction guides are "unrelated"
> > functions, they will usually get discarded from the GMF, so this patch
> > ensures that when finding dependencies, GMF deduction guides will also
> > have bindings created. We do this in find_dependencies so that we don't
> > unnecessarily create bindings for GMF deduction guides that are never
> > reached; for consistency we do this for *all* deduction guides, not just
> > GM ones.
> >
> > Finally, when merging deduction guides from multiple modules, the name
> > lookup code may now return two-dimensional overload sets, so update
> > callers to match.
> >
> > As a small drive-by improvement this patch also updates the error pretty
> > printing code to add a space before the '->' when printing a deduction
> > guide, so we get 'S(int) -> S<int>' instead of 'S(int)-> S<int>'.
> >
> > PR c++/115231
> >
> > gcc/cp/ChangeLog:
> >
> > * error.cc (dump_function_decl): Add a space before '->' when
> > printing deduction guides.
> > * module.cc (depset::hash::add_binding_entity): Skip deduction
> > guides here.
> > (depset::hash::add_deduction_guides): New.
> > (depset::hash::find_dependencies): Add deduction guide
> > dependencies for a class template.
> > (module_state::write_cluster): Always consider deduction guides
> > as exported.
> > * pt.cc (deduction_guides_for): Use 'lkp_iterator' instead of
> > 'ovl_iterator'.
> >
> > gcc/testsuite/ChangeLog:
> >
> > * g++.dg/modules/dguide-1_a.C: New test.
> > * g++.dg/modules/dguide-1_b.C: New test.
> > * g++.dg/modules/dguide-2_a.C: New test.
> > * g++.dg/modules/dguide-2_b.C: New test.
> > * g++.dg/modules/dguide-3_a.C: New test.
> > * g++.dg/modules/dguide-3_b.C: New test.
> > * g++.dg/modules/dguide-3_c.C: New test.
> >
> > Signed-off-by: Nathaniel Shead <nathanielosh...@gmail.com>
> > ---
> > gcc/cp/error.cc | 1 +
> > gcc/cp/module.cc | 60 +++++++++++++++++++++++
> > gcc/cp/pt.cc | 2 +-
> > gcc/testsuite/g++.dg/modules/dguide-1_a.C | 44 +++++++++++++++++
> > gcc/testsuite/g++.dg/modules/dguide-1_b.C | 20 ++++++++
> > gcc/testsuite/g++.dg/modules/dguide-2_a.C | 24 +++++++++
> > gcc/testsuite/g++.dg/modules/dguide-2_b.C | 19 +++++++
> > gcc/testsuite/g++.dg/modules/dguide-3_a.C | 10 ++++
> > gcc/testsuite/g++.dg/modules/dguide-3_b.C | 10 ++++
> > gcc/testsuite/g++.dg/modules/dguide-3_c.C | 22 +++++++++
> > 10 files changed, 211 insertions(+), 1 deletion(-)
> > create mode 100644 gcc/testsuite/g++.dg/modules/dguide-1_a.C
> > create mode 100644 gcc/testsuite/g++.dg/modules/dguide-1_b.C
> > create mode 100644 gcc/testsuite/g++.dg/modules/dguide-2_a.C
> > create mode 100644 gcc/testsuite/g++.dg/modules/dguide-2_b.C
> > create mode 100644 gcc/testsuite/g++.dg/modules/dguide-3_a.C
> > create mode 100644 gcc/testsuite/g++.dg/modules/dguide-3_b.C
> > create mode 100644 gcc/testsuite/g++.dg/modules/dguide-3_c.C
> >
> > diff --git a/gcc/cp/error.cc b/gcc/cp/error.cc
> > index 171a352c85f..2fb5084320e 100644
> > --- a/gcc/cp/error.cc
> > +++ b/gcc/cp/error.cc
> > @@ -1866,6 +1866,7 @@ dump_function_decl (cxx_pretty_printer *pp, tree t,
> > int flags)
> > dump_type_suffix (pp, ret, flags);
> > else if (deduction_guide_p (t))
> > {
> > + pp->set_padding (pp_before);
> > pp_cxx_ws_string (pp, "->");
> > dump_type (pp, TREE_TYPE (TREE_TYPE (t)), flags);
> > }
> > diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
> > index 6d6044af199..a2c9e151fd5 100644
> > --- a/gcc/cp/module.cc
> > +++ b/gcc/cp/module.cc
> > @@ -2589,6 +2589,9 @@ public:
> > void add_partial_entities (vec<tree, va_gc> *);
> > void add_class_entities (vec<tree, va_gc> *);
> >
> > + private:
> > + void add_deduction_guides (tree decl);
> > +
> > public:
> > void find_dependencies (module_state *);
> > bool finalize_dependencies ();
> > @@ -13127,6 +13130,11 @@ depset::hash::add_binding_entity (tree decl,
> > WMB_Flags flags, void *data_)
> > {
> > tree inner = decl;
> >
> > + if (deduction_guide_p (inner))
> > + /* Ignore deduction guides here, they'll be handled within
> > + find_dependencies for a class template. */
> > + return false;
> > +
> > if (TREE_CODE (inner) == CONST_DECL
> > && TREE_CODE (DECL_CONTEXT (inner)) == ENUMERAL_TYPE)
> > inner = TYPE_NAME (DECL_CONTEXT (inner));
> > @@ -13590,6 +13598,50 @@ find_pending_key (tree decl, tree *decl_p =
> > nullptr)
> > return ns;
> > }
> >
> > +/* Creates bindings and dependencies for all deduction guides of
> > + the given class template DECL as needed. */
> > +
> > +void
> > +depset::hash::add_deduction_guides (tree decl)
> > +{
> > + /* Alias templates never have deduction guides. */
> > + if (DECL_ALIAS_TEMPLATE_P (decl))
> > + return;
> > +
> > + /* We don't need to do anything for class-scope deduction guides,
> > + as they will be added as members anyway. */
> > + if (!DECL_NAMESPACE_SCOPE_P (decl))
> > + return;
> > +
> > + tree ns = CP_DECL_CONTEXT (decl);
> > + tree name = dguide_name (decl);
> > +
> > + /* We always add all deduction guides with a given name at once,
> > + so if there's already a binding there's nothing to do. */
> > + if (find_binding (ns, name))
> > + return;
> > +
> > + tree guides = lookup_qualified_name (ns, name, LOOK_want::NORMAL,
> > + /*complain=*/false);
> > + if (guides == error_mark_node)
> > + return;
> > +
> > + /* We have bindings to add. */
> > + depset *binding = make_binding (ns, name);
> > + add_namespace_context (binding, ns);
> > +
> > + depset **slot = binding_slot (ns, name, /*insert=*/true);
> > + *slot = binding;
> > +
> > + for (lkp_iterator it (guides); it; ++it)
> > + {
> > + gcc_checking_assert (!TREE_VISITED (*it));
> > + depset *dep = make_dependency (*it, EK_FOR_BINDING);
> > + binding->deps.safe_push (dep);
> > + dep->deps.safe_push (binding);
> > + }
> > +}
> > +
> > /* Iteratively find dependencies. During the walk we may find more
> > entries on the same binding that need walking. */
> >
> > @@ -13649,6 +13701,10 @@ depset::hash::find_dependencies (module_state
> > *module)
> > }
> > walker.end ();
> >
> > + if (!walker.is_key_order ()
> > + && DECL_CLASS_TEMPLATE_P (decl))
> > + add_deduction_guides (decl);
> > +
> > if (!walker.is_key_order ()
> > && TREE_CODE (decl) == TEMPLATE_DECL
> > && !DECL_UNINSTANTIATED_TEMPLATE_FRIEND_P (decl))
> > @@ -15145,6 +15201,10 @@ module_state::write_cluster (elf_out *to, depset
> > *scc[], unsigned size,
> > flags |= cbf_hidden;
> > else if (DECL_MODULE_EXPORT_P (STRIP_TEMPLATE (bound)))
> > flags |= cbf_export;
> > + else if (deduction_guide_p (bound))
> > + /* Deduction guides are always exported so that they are
> > + reachable whenever their class template is. */
> > + flags |= cbf_export;
> > }
> >
> > gcc_checking_assert (DECL_P (bound));
> > diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
> > index 607753ae6b7..8e007a571a5 100644
> > --- a/gcc/cp/pt.cc
> > +++ b/gcc/cp/pt.cc
> > @@ -30784,7 +30784,7 @@ deduction_guides_for (tree tmpl, bool
> > &any_dguides_p, tsubst_flags_t complain)
> > else
> > {
> > cands = ctor_deduction_guides_for (tmpl, complain);
> > - for (ovl_iterator it (guides); it; ++it)
> > + for (lkp_iterator it (guides); it; ++it)
> > cands = lookup_add (*it, cands);
> > }
> >
> > diff --git a/gcc/testsuite/g++.dg/modules/dguide-1_a.C
> > b/gcc/testsuite/g++.dg/modules/dguide-1_a.C
> > new file mode 100644
> > index 00000000000..834e033eae3
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/modules/dguide-1_a.C
> > @@ -0,0 +1,44 @@
> > +// PR c++/115231
> > +// { dg-additional-options "-fmodules-ts -Wno-global-module" }
> > +// { dg-module-cmi M }
> > +
> > +module;
> > +
> > +template <typename T>
> > +struct A {
> > + template <typename U> A(U);
> > +};
> > +
> > +template <typename T> A(T) -> A<T>;
> > +
> > +export module M;
> > +
> > +// Exporting a GMF entity should make the deduction guides reachable.
> > +export using ::A;
> > +
> > +
> > +export template <typename T>
> > +struct B {
> > + template <typename U> B(U);
> > +};
> > +
> > +// Not exported, but should still be reachable by [temp.deduct.guide] p1.
> > +B(int) -> B<double>;
> > +
> > +
> > +// Class-scope deduction guides should be reachable as well, even if
> > +// the class body was not exported.
> > +export template <typename T> struct C;
> > +
> > +template <typename T>
> > +struct C {
> > + template <typename U>
> > + struct I {
> > + template <typename V> I(V);
> > + };
> > +
> > + I(int) -> I<int>;
> > +
> > + template <typename P>
> > + I(const P*) -> I<P>;
> > +};
> > diff --git a/gcc/testsuite/g++.dg/modules/dguide-1_b.C
> > b/gcc/testsuite/g++.dg/modules/dguide-1_b.C
> > new file mode 100644
> > index 00000000000..97266986d8f
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/modules/dguide-1_b.C
> > @@ -0,0 +1,20 @@
> > +// PR c++/115231
> > +// { dg-additional-options "-fmodules-ts" }
> > +
> > +import M;
> > +
> > +int main() {
> > + // Check that deduction guides are reachable,
> > + // and that they declared the right type.
> > + A a(1);
> > + A<int> a2 = a;
> > +
> > + B b(2);
> > + B<double> b2 = b;
> > +
> > + C<int>::I x(10);
> > + C<int>::I<int> x2 = x;
> > +
> > + C<int>::I y("xyz");
> > + C<int>::I<char> y2 = y;
> > +}
> > diff --git a/gcc/testsuite/g++.dg/modules/dguide-2_a.C
> > b/gcc/testsuite/g++.dg/modules/dguide-2_a.C
> > new file mode 100644
> > index 00000000000..fcd6c579813
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/modules/dguide-2_a.C
> > @@ -0,0 +1,24 @@
> > +// PR c++/115231
> > +// { dg-additional-options "-fmodules-ts -Wno-global-module" }
> > +// { dg-module-cmi M }
> > +
> > +module;
> > +
> > +template <typename T>
> > +struct A {
> > + template <typename U> A(U);
> > +};
> > +template <typename T> A(T) -> A<T>;
> > +
> > +export module M;
> > +
> > +template <typename T>
> > +struct B {
> > + template <typename U> B(U);
> > +};
> > +B(int) -> B<int>;
> > +
> > +// Accessing deduction guides should be possible,
> > +// even if we can't name the type directly.
> > +export A<void> f();
> > +export B<void> g();
> > diff --git a/gcc/testsuite/g++.dg/modules/dguide-2_b.C
> > b/gcc/testsuite/g++.dg/modules/dguide-2_b.C
> > new file mode 100644
> > index 00000000000..ca31306aea3
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/modules/dguide-2_b.C
> > @@ -0,0 +1,19 @@
> > +// PR c++/115231
> > +// { dg-additional-options "-fmodules-ts" }
> > +
> > +import M;
> > +
> > +template <typename>
> > +struct U;
> > +
> > +template <template <typename> typename TT, typename Inner>
> > +struct U<TT<Inner>> {
> > + void go() {
> > + TT t(10);
> > + }
> > +};
> > +
> > +int main() {
> > + U<decltype(f())>{}.go();
> > + U<decltype(g())>{}.go();
> > +}
> > diff --git a/gcc/testsuite/g++.dg/modules/dguide-3_a.C
> > b/gcc/testsuite/g++.dg/modules/dguide-3_a.C
> > new file mode 100644
> > index 00000000000..33350ce9804
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/modules/dguide-3_a.C
> > @@ -0,0 +1,10 @@
> > +// { dg-additional-options "-fmodules-ts" }
> > +// { dg-module-cmi A }
> > +
> > +export module A;
> > +
> > +extern "C++" {
> > + template <typename T> struct S;
> > + S(int) -> S<int>;
> > + S(double) -> S<double>;
> > +}
> > diff --git a/gcc/testsuite/g++.dg/modules/dguide-3_b.C
> > b/gcc/testsuite/g++.dg/modules/dguide-3_b.C
> > new file mode 100644
> > index 00000000000..d23696c74bd
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/modules/dguide-3_b.C
> > @@ -0,0 +1,10 @@
> > +// { dg-additional-options "-fmodules-ts" }
> > +// { dg-module-cmi B }
> > +
> > +export module B;
> > +
> > +extern "C++" {
> > + template <typename T> struct S;
> > + S(int) -> S<int>;
> > + S(char) -> S<char>;
> > +}
> > diff --git a/gcc/testsuite/g++.dg/modules/dguide-3_c.C
> > b/gcc/testsuite/g++.dg/modules/dguide-3_c.C
> > new file mode 100644
> > index 00000000000..39ae8f4aa58
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/modules/dguide-3_c.C
> > @@ -0,0 +1,22 @@
> > +// { dg-additional-options "-fmodules-ts" }
> > +// Test merging deduction guides.
> > +
> > +import A;
> > +import B;
> > +
> > +template <typename T> struct S {
> > + template <typename U> S(U);
> > +};
> > +
> > +int main() {
> > + S x(123); // { dg-bogus "incomplete" "" { xfail *-*-* } }
> > + S<int> x2 = x;
> > +
> > + S y('c'); // { dg-bogus "incomplete" "" { xfail *-*-* } }
> > + S<char> y2 = y;
> > +
> > + S z(0.5); // { dg-bogus "incomplete" "" { xfail *-*-* } }
> > + S<double> z2 = z;
> > +}
> > +
> > +S(char) -> S<double>; // { dg-error "ambiguating" }
> > --
> > 2.43.2
> >
>
> I just realised I forgot to explain these xfails in the above commit:
> this is because the return types declare a new "specialisation" that is
> treated as a separate type on stream-in, rather than merging with the
> existing template on instantiation.
>
> A simplified example of this error is:
>
> // a.cpp
> export module A;
> extern "C++" template <typename T> struct S;
> export S<int> foo();
>
> // main.cpp
> import A;
> template <typename T> struct S {};
> int main() { foo(); }
>
> I suspect this is related to PR c++/113814. I'm pretty sure this should
> be valid code ([temp.inst] p2 doesn't seem to require instantiation in A
> because a function return type doesn't require a completely-defined
> object type and completeness won't affect the semantics of the program)
> but I haven't looked much into how to fix this yet.
>
> Nathaniel
