c++ modules: streaming constexpr_fundef [PR101449]
authorPatrick Palka <ppalka@redhat.com>
Mon, 17 Oct 2022 15:14:36 +0000 (11:14 -0400)
committerPatrick Palka <ppalka@redhat.com>
Mon, 17 Oct 2022 15:14:36 +0000 (11:14 -0400)
It looks like we currently avoid streaming the RESULT_DECL and PARM_DECLs
of a constexpr_fundef entry under the assumption that they're just copies
of the DECL_RESULT and DECL_ARGUMENTS of the FUNCTION_DECL.  Thus we can
just make new copies of DECL_RESULT and DECL_ARGUMENTS on stream in rather
than separately streaming them.

But the FUNCTION_DECL's DECL_RESULT and DECL_ARGUMENTS eventually get
genericized, whereas the constexpr_fundef entry consists of a copy of the
FUNCTION_DECL's pre-GENERIC trees.  And notably during genericization we
lower invisref parms (which entails changing their TREE_TYPE and setting
DECL_BY_REFERENCE), the lowered form of which the constexpr evaluator
doesn't expect to see, and so this copying approach causes us to ICE for
the below testcase.

This patch fixes this by faithfully streaming the RESULT_DECL and
PARM_DECLs of a constexpr_fundef entry, which seems to just work.

Nathan says[1]: Hm, the reason for the complexity was that I wanted to
recreate the tree graph where the fndecl came from one TU and the defn
came from another one -- we need the definition to refer to argument
decls from the already-read decl.  However, it seems that for constexpr
fns here, that is not needed, resulting in a significant simplification.

[1]: https://gcc.gnu.org/pipermail/gcc-patches/2022-October/603662.html

PR c++/101449

gcc/cp/ChangeLog:

* module.cc (trees_out::write_function_def): Stream the
parms and result of the constexpr_fundef entry.
(trees_in::read_function_def): Likewise.

gcc/testsuite/ChangeLog:

* g++.dg/modules/cexpr-3_a.C: New test.
* g++.dg/modules/cexpr-3_b.C: New test.

gcc/cp/module.cc
gcc/testsuite/g++.dg/modules/cexpr-3_a.C [new file with mode: 0644]
gcc/testsuite/g++.dg/modules/cexpr-3_b.C [new file with mode: 0644]

index 7ffeefa..999ff3f 100644 (file)
@@ -11553,34 +11553,13 @@ trees_out::write_function_def (tree decl)
   tree_node (DECL_FRIEND_CONTEXT (decl));
 
   constexpr_fundef *cexpr = retrieve_constexpr_fundef (decl);
-  int tag = 0;
-  if (cexpr)
-    {
-      if (cexpr->result == error_mark_node)
-       /* We'll stream the RESULT_DECL naturally during the
-          serialization.  We never need to fish it back again, so
-          that's ok.  */
-       tag = 0;
-      else
-       tag = insert (cexpr->result);
-    }
+
   if (streaming_p ())
+    u (cexpr != nullptr);
+  if (cexpr)
     {
-      i (tag);
-      if (tag)
-       dump (dumper::TREE)
-         && dump ("Constexpr:%d result %N", tag, cexpr->result);
-    }
-  if (tag)
-    {
-      unsigned ix = 0;
-      for (tree parm = cexpr->parms; parm; parm = DECL_CHAIN (parm), ix++)
-       {
-         tag = insert (parm);
-         if (streaming_p ())
-           dump (dumper::TREE)
-             && dump ("Constexpr:%d parm:%u %N", tag, ix, parm);
-       }
+      chained_decls (cexpr->parms);
+      tree_node (cexpr->result);
       tree_node (cexpr->body);
     }
 
@@ -11613,32 +11592,10 @@ trees_in::read_function_def (tree decl, tree maybe_template)
   tree maybe_dup = odr_duplicate (maybe_template, DECL_SAVED_TREE (decl));
   bool installing = maybe_dup && !DECL_SAVED_TREE (decl);
 
-  if (int wtag = i ())
+  if (u ())
     {
-      int tag = 1;
-      cexpr.result = error_mark_node;
-
-      cexpr.result = copy_decl (result);
-      tag = insert (cexpr.result);
-
-      if (wtag != tag)
-       set_overrun ();
-      dump (dumper::TREE)
-       && dump ("Constexpr:%d result %N", tag, cexpr.result);
-
-      cexpr.parms = NULL_TREE;
-      tree *chain = &cexpr.parms;
-      unsigned ix = 0;
-      for (tree parm = DECL_ARGUMENTS (maybe_dup ? maybe_dup : decl);
-          parm; parm = DECL_CHAIN (parm), ix++)
-       {
-         tree p = copy_decl (parm);
-         tag = insert (p);
-         dump (dumper::TREE)
-           && dump ("Constexpr:%d parm:%u %N", tag, ix, p);
-         *chain = p;
-         chain = &DECL_CHAIN (p);
-       }
+      cexpr.parms = chained_decls ();
+      cexpr.result = tree_node ();
       cexpr.body = tree_node ();
       cexpr.decl = decl;
     }
diff --git a/gcc/testsuite/g++.dg/modules/cexpr-3_a.C b/gcc/testsuite/g++.dg/modules/cexpr-3_a.C
new file mode 100644 (file)
index 0000000..be24bb4
--- /dev/null
@@ -0,0 +1,14 @@
+// PR c++/101449
+// { dg-additional-options -fmodules-ts }
+// { dg-module-cmi pr101449 }
+
+export module pr101449;
+
+struct X {
+  bool b = true;
+  constexpr X() { }
+  constexpr X(const X&) { }
+};
+
+export constexpr X f() { return {}; }
+export constexpr bool g(X x) { return x.b; }
diff --git a/gcc/testsuite/g++.dg/modules/cexpr-3_b.C b/gcc/testsuite/g++.dg/modules/cexpr-3_b.C
new file mode 100644 (file)
index 0000000..cbf3be4
--- /dev/null
@@ -0,0 +1,7 @@
+// PR c++/101449
+// { dg-additional-options -fmodules-ts }
+
+import pr101449;
+
+static_assert(f().b);
+static_assert(g(f()));