https://gcc.gnu.org/g:b8e8829cfb73d7aa009d387ab09bdbab221930d7

commit r15-9217-gb8e8829cfb73d7aa009d387ab09bdbab221930d7
Author: Nathaniel Shead <nathanielosh...@gmail.com>
Date:   Fri Apr 4 11:56:53 2025 +1100

    c++/modules: Fix divergence in streaming/non-streaming tree walks [PR119608]
    
    Modules streaming walks decls multiple times, first as a non-streaming
    walk to find dependencies, and then later to actually emit the decls.
    The first walk needs to be done to note locations that will be emitted.
    
    In the PR we are getting a checking ICE because we are streaming a decl
    that we didn't initially walk when collecting dependencies, so the
    location isn't in the noted locations map.  This is because in decl_node
    we have a branch where a PARM_DECL that hasn't previously been
    referenced gets walked by value only if 'streaming_p ()' is true.
    
    The true root cause here is that the decltype(v) in the testcase refers
    to a different PARM_DECL from the one in the declaration that we're
    streaming; it's the PARM_DECL from the initial forward-declaration, that
    we're not streaming.  A proper fix would be to ensure that it gets
    remapped to the decl in the definition we're actually emitting, but for
    now this workaround fixes the bug (and any other bugs that might
    manifest similarly).
    
            PR c++/119608
    
    gcc/cp/ChangeLog:
    
            * module.cc (trees_out::decl_node): Maybe require by-value
            walking not just when streaming.
    
    gcc/testsuite/ChangeLog:
    
            * g++.dg/modules/pr119608_a.C: New test.
            * g++.dg/modules/pr119608_b.C: New test.
    
    Signed-off-by: Nathaniel Shead <nathanielosh...@gmail.com>
    Reviewed-by: Jason Merrill <ja...@redhat.com>

Diff:
---
 gcc/cp/module.cc                          | 39 +++++++++++++++++--------------
 gcc/testsuite/g++.dg/modules/pr119608_a.C | 16 +++++++++++++
 gcc/testsuite/g++.dg/modules/pr119608_b.C |  8 +++++++
 3 files changed, 45 insertions(+), 18 deletions(-)

diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
index ff8cd86c427d..37fab5b5a43e 100644
--- a/gcc/cp/module.cc
+++ b/gcc/cp/module.cc
@@ -9004,25 +9004,28 @@ trees_out::decl_node (tree decl, walk_kind ref)
        if (streaming_p ())
          i (tt_parm);
        tree_node (DECL_CONTEXT (decl));
-       if (streaming_p ())
-         {
-           /* That must have put this in the map.  */
-           walk_kind ref = ref_node (decl);
-           if (ref != WK_none)
-             // FIXME:OPTIMIZATION We can wander into bits of the
-             // template this was instantiated from.  For instance
-             // deferred noexcept and default parms.  Currently we'll
-             // end up cloning those bits of tree.  It would be nice
-             // to reference those specific nodes.  I think putting
-             // those things in the map when we reference their
-             // template by name.  See the note in add_indirects.
-             return true;
 
-           dump (dumper::TREE)
-             && dump ("Wrote %s reference %N",
-                      TREE_CODE (decl) == PARM_DECL ? "parameter" : "result",
-                      decl);
-         }
+       /* That must have put this in the map.  */
+       walk_kind ref = ref_node (decl);
+       if (ref != WK_none)
+         // FIXME:OPTIMIZATION We can wander into bits of the
+         // template this was instantiated from, for instance
+         // deferred noexcept and default parms, or references
+         // to parms from earlier forward-decls (PR c++/119608).
+         //
+         // Currently we'll end up cloning those bits of tree. 
+         // It would be nice to reference those specific nodes.
+         // I think putting those things in the map when we
+         // reference their template by name.
+         //
+         // See the note in add_indirects.
+         return true;
+
+       if (streaming_p ())
+         dump (dumper::TREE)
+           && dump ("Wrote %s reference %N",
+                    TREE_CODE (decl) == PARM_DECL ? "parameter" : "result",
+                    decl);
       }
       return false;
 
diff --git a/gcc/testsuite/g++.dg/modules/pr119608_a.C 
b/gcc/testsuite/g++.dg/modules/pr119608_a.C
new file mode 100644
index 000000000000..4e7b359ac581
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/pr119608_a.C
@@ -0,0 +1,16 @@
+// PR c++/119608
+// { dg-additional-options "-fmodules" }
+// { dg-module-cmi repro }
+
+export module repro;
+
+export template<class Visitor>
+auto
+visit(
+  Visitor v
+) -> decltype(
+  v);
+
+export template<class Visitor> auto visit(Visitor v) -> decltype(v) {
+  return {};
+}
diff --git a/gcc/testsuite/g++.dg/modules/pr119608_b.C 
b/gcc/testsuite/g++.dg/modules/pr119608_b.C
new file mode 100644
index 000000000000..023d20adf4c5
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/pr119608_b.C
@@ -0,0 +1,8 @@
+// PR c++/119608
+// { dg-additional-options "-fmodules" }
+
+import repro;
+
+int main() {
+  visit(123);
+}

Reply via email to