On Tue, Jun 24, 2025 at 12:10:09PM -0400, Patrick Palka wrote: > On Tue, 24 Jun 2025, Jason Merrill wrote: > > > On 6/23/25 5:41 PM, Nathaniel Shead wrote: > > > Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk/15? > > > > > > -- >8 -- > > > > > > We were erroring because the TEMPLATE_DECL of the existing partial > > > specialisation has an undeduced return type, but the imported > > > declaration did not. > > > > > > The root cause is similar to what was fixed in r13-2744-g4fac53d6522189, > > > where modules streaming code assumes that a TEMPLATE_DECL and its > > > DECL_TEMPLATE_RESULT will always have the same TREE_TYPE. That commit > > > fixed the issue by ensuring that when the type of a variable is deduced > > > the TEMPLATE_DECL is updated as well, but this missed handling partial > > > specialisations. > > > > > > However, I don't think we actually care about that, since it seems that > > > only the type of the inner decl actually matters in practice. Instead, > > > this patch handles the issue on the modules side when deduping a > > > streamed decl, by only comparing the inner type. > > > > > > PR c++/120644 > > > > > > gcc/cp/ChangeLog: > > > > > > * decl.cc (cp_finish_decl): Remove workaround. > > > > Hmm, if we aren't going to try to keep the type of the TEMPLATE_DECL > > correct, > > maybe we should always set it to NULL_TREE to make sure we only look at the > > inner type. > > FWIW cp_finish_decl can get at the TEMPLATE_DECL of a VAR_DECL > corresponding to a partial specialization via > > TI_TEMPLATE (TI_PARTIAL_INFO (DECL_TEMPLATE_INFO (decl))) > > if we do want to end up keeping the two TREE_TYPEs in sync. >
Thanks. On further reflection, maybe the safest approach is to just ensure that the types are always consistent (including for partial specs); this is what the following patch does. Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk? -- >8 -- Subject: [PATCH] c++/modules: Ensure type of partial spec VAR_DECL is consistent with its template [PR120644] We were erroring because the TEMPLATE_DECL of the existing partial specialisation has an undeduced return type, but the imported declaration did not. The root cause is similar to what was fixed in r13-2744-g4fac53d6522189, where modules streaming code assumes that a TEMPLATE_DECL and its DECL_TEMPLATE_RESULT will always have the same TREE_TYPE. That commit fixed the issue by ensuring that when the type of a variable is deduced the TEMPLATE_DECL is updated as well, but missed handling partial specialisations. This patch ensures that the same adjustment is made there as well. PR c++/120644 gcc/cp/ChangeLog: * decl.cc (cp_finish_decl): Also propagate type to partial templates. * module.cc (trees_out::decl_value): Add assertion that the TREE_TYPE of a streamed template decl matches its inner. (trees_in::is_matching_decl): Clarify function return type deduction should only occur for non-TEMPLATE_DECL. gcc/testsuite/ChangeLog: * g++.dg/modules/auto-7.h: New test. * g++.dg/modules/auto-7_a.H: New test. * g++.dg/modules/auto-7_b.C: New test. Signed-off-by: Nathaniel Shead <nathanielosh...@gmail.com> Reviewed-by: Jason Merrill <ja...@redhat.com> Reviewed-by: Patrick Palka <ppa...@redhat.com> --- gcc/cp/decl.cc | 13 +++++++++---- gcc/cp/module.cc | 7 ++++++- gcc/testsuite/g++.dg/modules/auto-7.h | 12 ++++++++++++ gcc/testsuite/g++.dg/modules/auto-7_a.H | 5 +++++ gcc/testsuite/g++.dg/modules/auto-7_b.C | 5 +++++ 5 files changed, 37 insertions(+), 5 deletions(-) create mode 100644 gcc/testsuite/g++.dg/modules/auto-7.h create mode 100644 gcc/testsuite/g++.dg/modules/auto-7_a.H create mode 100644 gcc/testsuite/g++.dg/modules/auto-7_b.C diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc index 4fe97ffbf8f..59701197e16 100644 --- a/gcc/cp/decl.cc +++ b/gcc/cp/decl.cc @@ -8923,10 +8923,15 @@ cp_finish_decl (tree decl, tree init, bool init_const_expr_p, cp_apply_type_quals_to_decl (cp_type_quals (type), decl); /* Update the type of the corresponding TEMPLATE_DECL to match. */ - if (DECL_LANG_SPECIFIC (decl) - && DECL_TEMPLATE_INFO (decl) - && DECL_TEMPLATE_RESULT (DECL_TI_TEMPLATE (decl)) == decl) - TREE_TYPE (DECL_TI_TEMPLATE (decl)) = type; + if (DECL_LANG_SPECIFIC (decl) && DECL_TEMPLATE_INFO (decl)) + { + tree info = DECL_TEMPLATE_INFO (decl); + tree tmpl = TI_TEMPLATE (info); + if (DECL_TEMPLATE_RESULT (tmpl) == decl) + TREE_TYPE (tmpl) = type; + else if (PRIMARY_TEMPLATE_P (tmpl) && TI_PARTIAL_INFO (info)) + TREE_TYPE (TI_TEMPLATE (TI_PARTIAL_INFO (info))) = type; + } } if (ensure_literal_type_for_constexpr_object (decl) == error_mark_node) diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc index c99988da05b..53edb2ff203 100644 --- a/gcc/cp/module.cc +++ b/gcc/cp/module.cc @@ -8212,6 +8212,10 @@ trees_out::decl_value (tree decl, depset *dep) inner = DECL_TEMPLATE_RESULT (decl); inner_tag = insert (inner, WK_value); + /* On stream-in we assume that a template and its result will + have the same type. */ + gcc_checking_assert (TREE_TYPE (decl) == TREE_TYPE (inner)); + if (streaming_p ()) { int code = TREE_CODE (inner); @@ -12193,7 +12197,8 @@ trees_in::is_matching_decl (tree existing, tree decl, bool is_typedef) { dump (dumper::MERGE) && dump ("Propagating deduced return type to %N", existing); - FNDECL_USED_AUTO (e_inner) = true; + gcc_checking_assert (existing == e_inner); + FNDECL_USED_AUTO (existing) = true; DECL_SAVED_AUTO_RETURN_TYPE (existing) = TREE_TYPE (e_type); TREE_TYPE (existing) = change_return_type (TREE_TYPE (d_type), e_type); } diff --git a/gcc/testsuite/g++.dg/modules/auto-7.h b/gcc/testsuite/g++.dg/modules/auto-7.h new file mode 100644 index 00000000000..324b60cfa0a --- /dev/null +++ b/gcc/testsuite/g++.dg/modules/auto-7.h @@ -0,0 +1,12 @@ +// PR c++/120644 + +enum class E { E0, E1 }; + +template <typename T> +constexpr auto fmt_kind = E::E0; + +template <typename T> +class opt{}; + +template <typename T> +constexpr auto fmt_kind<opt<T>> = E::E1; diff --git a/gcc/testsuite/g++.dg/modules/auto-7_a.H b/gcc/testsuite/g++.dg/modules/auto-7_a.H new file mode 100644 index 00000000000..40cb0f886c0 --- /dev/null +++ b/gcc/testsuite/g++.dg/modules/auto-7_a.H @@ -0,0 +1,5 @@ +// PR c++/120644 +// { dg-additional-options "-fmodule-header" } +// { dg-module-cmi {} } + +#include "auto-7.h" diff --git a/gcc/testsuite/g++.dg/modules/auto-7_b.C b/gcc/testsuite/g++.dg/modules/auto-7_b.C new file mode 100644 index 00000000000..c6ad37fd828 --- /dev/null +++ b/gcc/testsuite/g++.dg/modules/auto-7_b.C @@ -0,0 +1,5 @@ +// PR c++/120644 +// { dg-additional-options "-fmodules -fno-module-lazy" } + +#include "auto-7.h" +import "auto-7_a.H"; -- 2.47.0