On Wed, Jun 25, 2025 at 11:52:14AM -0400, Jason Merrill wrote:
> On 6/25/25 9:02 AM, Nathaniel Shead wrote:
> > 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;
> > + }
>
> Perhaps we should update template_for_substitution to handle partial specs
> and then use it here?
>
Something like this?
Bootstrapped and tested on x86_64-pc-linux-gnu (so far just modules.exp
and dg.exp), OK for trunk if full regtest passes?
-- >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 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.
* pt.cc (template_for_substitution): Handle partial specs.
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 | 10 ++++++----
gcc/cp/module.cc | 7 ++++++-
gcc/cp/pt.cc | 4 ++++
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 +++++
6 files changed, 38 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..83c8e283b56 100644
--- a/gcc/cp/decl.cc
+++ b/gcc/cp/decl.cc
@@ -8923,10 +8923,12 @@ 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 tmpl = template_for_substitution (decl);
+ if (DECL_TEMPLATE_RESULT (tmpl) == decl)
+ TREE_TYPE (tmpl) = 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/cp/pt.cc b/gcc/cp/pt.cc
index deb0106b158..319c733270a 100644
--- a/gcc/cp/pt.cc
+++ b/gcc/cp/pt.cc
@@ -27536,6 +27536,10 @@ tree
template_for_substitution (tree decl)
{
tree tmpl = DECL_TI_TEMPLATE (decl);
+ if (VAR_P (decl) || TYPE_P (decl))
+ if (tree partial = most_specialized_partial_spec (decl, tf_none))
+ if (partial != error_mark_node)
+ tmpl = TI_TEMPLATE (partial);
/* Set TMPL to the template whose DECL_TEMPLATE_RESULT is the pattern
for the instantiation. This is not always the most general
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
