Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk/14?

-- >8 --

In the linked testcase, an ICE occurs because when reading the
(duplicate) function definition for _M_do_parse from module Y, the local
type definitions have already been streamed from module X and setup as
regular backreferences, rather than being found with find_duplicate,
causing issues with managing DECL_CHAIN.

It is tempting to just skip setting up the DECL_CHAIN for this case.
However, for the future it would be best to ensure that the block vars
for the duplicate definition are accurate, so that we could implement
ODR checking on function definitions at some point.

So to solve this, this patch creates a copy of the streamed-in local
type and chains that; it will be discarded along with the rest of the
duplicate function after we've finished processing.

A couple of suggested implementations from the discussion on the PR that
don't work:

- Replacing the `DECL_CHAIN` assertion with `(*chain && *chain != decl)`
  doesn't handle the case where type definitions are followed by regular
  local variables, since those won't have been imported as separate
  backreferences and so the chains will diverge.

- Correcting the purviewness of GMF template instantiations to force Y
  to emit copies of the local types rather than backreferences into X is
  insufficient, as it's still possible that the local types got streamed
  in a separate cluster to the function definition, and so will be again
  referred to via regular backreferences when importing.

- Likewise, preventing the emission of function definitions where an
  import has already provided that same definition also is insufficient,
  for much the same reason.

        PR c++/114630

gcc/cp/ChangeLog:

        * module.cc (trees_in::core_vals) <BLOCK>: Chain a new node if
        DECL_CHAIN already is set.

gcc/testsuite/ChangeLog:

        * g++.dg/modules/pr114630.h: New test.
        * g++.dg/modules/pr114630_a.C: New test.
        * g++.dg/modules/pr114630_b.C: New test.
        * g++.dg/modules/pr114630_c.C: New test.

Signed-off-by: Nathaniel Shead <nathanielosh...@gmail.com>
---
 gcc/cp/module.cc                          | 14 +++++++++++++-
 gcc/testsuite/g++.dg/modules/pr114630.h   | 11 +++++++++++
 gcc/testsuite/g++.dg/modules/pr114630_a.C |  7 +++++++
 gcc/testsuite/g++.dg/modules/pr114630_b.C |  8 ++++++++
 gcc/testsuite/g++.dg/modules/pr114630_c.C |  4 ++++
 5 files changed, 43 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/g++.dg/modules/pr114630.h
 create mode 100644 gcc/testsuite/g++.dg/modules/pr114630_a.C
 create mode 100644 gcc/testsuite/g++.dg/modules/pr114630_b.C
 create mode 100644 gcc/testsuite/g++.dg/modules/pr114630_c.C

diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
index 5350e6c4bad..ff2683de73e 100644
--- a/gcc/cp/module.cc
+++ b/gcc/cp/module.cc
@@ -6928,11 +6928,23 @@ trees_in::core_vals (tree t)
               body) anyway.  */
            decl = maybe_duplicate (decl);
 
-           if (!DECL_P (decl) || DECL_CHAIN (decl))
+           if (!DECL_P (decl))
              {
                set_overrun ();
                break;
              }
+
+           /* If DECL_CHAIN is already set then this was a backreference to a
+              local type or enumerator from a previous read (PR c++/114630).
+              Let's copy the node so we can keep building the chain for ODR
+              checking later.  */
+           if (DECL_CHAIN (decl))
+             {
+               gcc_checking_assert (TREE_CODE (decl) == TYPE_DECL
+                                    && find_duplicate (DECL_CONTEXT (decl)));
+               decl = copy_node (decl);
+             }
+
            *chain = decl;
            chain = &DECL_CHAIN (decl);
          }
diff --git a/gcc/testsuite/g++.dg/modules/pr114630.h 
b/gcc/testsuite/g++.dg/modules/pr114630.h
new file mode 100644
index 00000000000..8730007f59f
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/pr114630.h
@@ -0,0 +1,11 @@
+template <typename _CharT>
+void _M_do_parse() {
+  struct A {};
+  struct B {};
+  int x;
+}
+
+template <typename> struct formatter;
+template <> struct formatter<int> {
+  void parse() { _M_do_parse<int>(); }
+};
diff --git a/gcc/testsuite/g++.dg/modules/pr114630_a.C 
b/gcc/testsuite/g++.dg/modules/pr114630_a.C
new file mode 100644
index 00000000000..ecfd7ca0b28
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/pr114630_a.C
@@ -0,0 +1,7 @@
+// { dg-additional-options "-fmodules" }
+// { dg-module-cmi X }
+
+module;
+#include "pr114630.h"
+export module X;
+formatter<int> a;
diff --git a/gcc/testsuite/g++.dg/modules/pr114630_b.C 
b/gcc/testsuite/g++.dg/modules/pr114630_b.C
new file mode 100644
index 00000000000..52fe04e2ce0
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/pr114630_b.C
@@ -0,0 +1,8 @@
+// { dg-additional-options "-fmodules" }
+// { dg-module-cmi Y }
+
+module;
+#include "pr114630.h"
+export module Y;
+import X;
+formatter<int> b;
diff --git a/gcc/testsuite/g++.dg/modules/pr114630_c.C 
b/gcc/testsuite/g++.dg/modules/pr114630_c.C
new file mode 100644
index 00000000000..54a21f08057
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/pr114630_c.C
@@ -0,0 +1,4 @@
+// { dg-additional-options "-fmodules" }
+
+#include "pr114630.h"
+import Y;
-- 
2.47.0

Reply via email to