[gcc r14-9881] c++: Track declarations imported from partitions [PR99377]

[gcc r14-9881] c++: Track declarations imported from partitions [PR99377]

[Nathaniel Shead]

https://gcc.gnu.org/g:77c0b5b23f91404004a9bf710981f6d615b63f57

commit r14-9881-g77c0b5b23f91404004a9bf710981f6d615b63f57
Author: Nathaniel Shead <nathanieloshead@gmail.com>
Date:   Thu Apr 4 23:16:08 2024 +1100

    c++: Track declarations imported from partitions [PR99377]
    
    The testcase in comment 15 of the linked PR is caused because the
    following assumption in depset::hash::make_dependency doesn't hold:
    
      if (DECL_LANG_SPECIFIC (not_tmpl)
          && DECL_MODULE_IMPORT_P (not_tmpl))
        {
          /* Store the module number and index in cluster/section,
             so we don't have to look them up again.  */
          unsigned index = import_entity_index (decl);
          module_state *from = import_entity_module (index);
          /* Remap will be zero for imports from partitions, which
             we want to treat as-if declared in this TU.  */
          if (from->remap)
            {
              dep->cluster = index - from->entity_lwm;
              dep->section = from->remap;
              dep->set_flag_bit<DB_IMPORTED_BIT> ();
            }
        }
    
    This is because at least for template specialisations, we first see the
    declaration in the header unit imported from the partition, and then the
    instantiation provided by the partition itself.  This means that the
    'import_entity_index' lookup doesn't report that the specialisation was
    declared in the partition and thus should be considered as-if it was
    part of the TU, and get emitted into the CMI.
    
    We always need to emit definitions from module partitions into the
    primary module interface's CMI, as unlike with other kinds of transitive
    imports the built CMIs for module partitions are not visible to
    importers.
    
    To fix this, this patch allows, as a special case for installing an
    entity from a partition, to overwrite the entity_map entry with the
    (later) index into the partition so that this assumption holds again.
    
    We only do this for the first time we override with a partition, so that
    entities are at least still reported as originating from the first
    imported partition that declares them (rather than the last); existing
    tests check for this and this seems to be a friendlier approach to go
    for, albeit slightly more expensive.
    
            PR c++/99377
    
    gcc/cp/ChangeLog:
    
            * module.cc (trees_in::install_entity): Overwrite entity map
            index if installing from a partition.
    
    gcc/testsuite/ChangeLog:
    
            * g++.dg/modules/pr99377-3_a.H: New test.
            * g++.dg/modules/pr99377-3_b.C: New test.
            * g++.dg/modules/pr99377-3_c.C: New test.
            * g++.dg/modules/pr99377-3_d.C: New test.
    
    Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com>

Diff:
---
 gcc/cp/module.cc                           | 13 +++++++++++++
 gcc/testsuite/g++.dg/modules/pr99377-3_a.H | 17 +++++++++++++++++
 gcc/testsuite/g++.dg/modules/pr99377-3_b.C | 10 ++++++++++
 gcc/testsuite/g++.dg/modules/pr99377-3_c.C |  5 +++++
 gcc/testsuite/g++.dg/modules/pr99377-3_d.C |  8 ++++++++
 5 files changed, 53 insertions(+)

diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
index 8aab9ea0bae..4e91fa6e052 100644
--- a/gcc/cp/module.cc
+++ b/gcc/cp/module.cc
@@ -7649,6 +7649,19 @@ trees_in::install_entity (tree decl)
       gcc_checking_assert (!existed);
       slot = ident;
     }
+  else if (state->is_partition ())
+    {
+      /* The decl is already in the entity map, but we see it again now from a
+	 partition: we want to overwrite if the original decl wasn't also from
+	 a (possibly different) partition.  Otherwise, for things like template
+	 instantiations, make_dependency might not realise that this is also
+	 provided from a partition and should be considered part of this module
+	 (and thus always emitted into the primary interface's CMI).  */
+      unsigned *slot = entity_map->get (DECL_UID (decl));
+      module_state *imp = import_entity_module (*slot);
+      if (!imp->is_partition ())
+	*slot = ident;
+    }
 
   return true;
 }
diff --git a/gcc/testsuite/g++.dg/modules/pr99377-3_a.H b/gcc/testsuite/g++.dg/modules/pr99377-3_a.H
new file mode 100644
index 00000000000..580a7631ae1
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/pr99377-3_a.H
@@ -0,0 +1,17 @@
+// { dg-additional-options "-fmodule-header" }
+// { dg-module-cmi {} }
+
+template<typename>
+struct Widget
+{
+  Widget (int) { }
+
+  bool First() const { return true; }
+
+  bool Second () const { return true;}
+};
+
+inline void Frob (const Widget<int>& w) noexcept
+{
+  w.First ();
+}
diff --git a/gcc/testsuite/g++.dg/modules/pr99377-3_b.C b/gcc/testsuite/g++.dg/modules/pr99377-3_b.C
new file mode 100644
index 00000000000..5cbce7b3544
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/pr99377-3_b.C
@@ -0,0 +1,10 @@
+// { dg-additional-options "-fmodules-ts" }
+// { dg-module-cmi Foo:check }
+
+export module Foo:check;
+import "pr99377-3_a.H";
+
+export inline bool Check (const Widget<int>& w)
+{
+  return w.Second ();
+}
diff --git a/gcc/testsuite/g++.dg/modules/pr99377-3_c.C b/gcc/testsuite/g++.dg/modules/pr99377-3_c.C
new file mode 100644
index 00000000000..fa7c24203bd
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/pr99377-3_c.C
@@ -0,0 +1,5 @@
+// { dg-additional-options "-fmodules-ts" }
+// { dg-module-cmi Foo }
+
+export module Foo;
+export import :check;
diff --git a/gcc/testsuite/g++.dg/modules/pr99377-3_d.C b/gcc/testsuite/g++.dg/modules/pr99377-3_d.C
new file mode 100644
index 00000000000..cb1f28321b1
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/pr99377-3_d.C
@@ -0,0 +1,8 @@
+// { dg-module-do link }
+// { dg-additional-options "-fmodules-ts" }
+
+import Foo;
+
+int main() {
+  return Check(0) ? 0 : 1;
+}