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
