On 9/26/22 15:05, Patrick Palka wrote:
On Mon, 26 Sep 2022, Patrick Palka wrote:
On Mon, 26 Sep 2022, Nathan Sidwell wrote:
On 9/26/22 10:08, Nathan Sidwell wrote:
On 9/23/22 09:32, Patrick Palka wrote:
Judging by the two commits that introduced/modified this part of
maybe_register_incomplete_var, r196852 and r214333, ISTM the code
is really only concerned with constexpr static data members (whose
initializer may contain a pointer-to-member for a currently open class).
So maybe we ought to restrict the branch like so, which effectively
disables this part of maybe_register_incomplete_var during stream-in, and
guarantees that outermost_open_class doesn't return NULL if the branch is
taken?
I think the problem is that we're streaming these VAR_DECLs as regular
VAR_DECLS, when we should be handling them as a new kind of object fished
out from the template they're instantiating. (I'm guessing that'll just be a
new tag, a type and an initializer?)
Then on stream-in we can handle them in the same way as a non-modules
compilation handles such redeclarations. I.e. how does:
template<auto> struct C { };
struct A { };
C<A{}> c1; // #1
C<A{}> c2; // #2
work. Presumably at some point #2's A{} gets unified such that we find the
instantation that occurred at #1?
This works because the lookup in get_template_parm_object for #2's A{}
finds and reuses the VAR_DECL created for #1's A{}.
But IIUC this lookup (performed via get_global_binding) isn't
import-aware, which I suppose explains why we don't find the VAR_DECL
from another TU.
I notice the template arg for C<A{}> is a var decl mangled as _ZTAXtl1AEE,
which is a 'template paramete object for A{}'. I see that's a special
mangler 'mangle_template_parm_object', called from
get_template_parm_object. Perhaps these VAR_DECLs need an additional
in-tree flag that the streamer can check for?
I wonder if we're setting the module attachment for these variables sanely?
They should be attached to the global module. My guess is the
pushdecl_top_level_and_finish call in get_templatE_parm_object is not doing
what is needed (as well as the other issues).
This is a bit of a shot in the dark, but the following seems to work:
when pushing the VAR_DECL, we need to call set_originating_module to
attach it to the global module, and when looking it up, we need to do so
in an import-aware way. Hopefully something like this is sufficient
to properly handle these VAR_DECLs and we don't need to stream them
specially?
Err, rather than changing the behavior of get_namespace_binding (which
has many unrelated callers), I guess we could just use the already
import-aware lookup_qualified_name instead where appropriate. WDYT of
the following? (testing in progress)
I'm going to have to think further. Morally these VAR_DECLs are like
the typeinfo objects -- which we have to handle specially. Reminding
myself, I see rtti.cc does the pushdecl_top_level stuff creating them --
so they go into the slot for the current TU. But the streaming code
writes tt_tinfo_var/tt_tinfo_typedef records, and recreates the typeinfo
on stream in, using the same above pushdec_top_level path. So even
though the tinfo decls might seem attached to the current module, that
doesn;t confuse the streaming, nor create collisions on read back. Nor
do we stream out tinfo decls that are not reachable through the streamed
AST (if we treated then as normal decls, we'd stream them cos they're
inthe current TU in the symbol table. I have the same fear for these
NTTPs.)
It looks like TREE_LANG_FLAG_5 can be used to note these VAR_DECLs are
NTTPs, and then the streaming can deal with them. Let me look further.
@@ -7307,6 +7307,7 @@ get_template_parm_object (tree expr, tsubst_flags_t
complain)
hash_map_safe_put<hm_ggc> (tparm_obj_values, decl, copy);
}
+ set_originating_module (decl);
pushdecl_top_level_and_finish (decl, expr);
this is wrong. You're attaching the decl to the current module. which
will mean conflicts when reading in such VAR_DECLs for the same NTTP
from different modules. Your test case might be hiding that as you have
an interface and implementation unit from the same module (but you
should be getting some kind of multiple definition error anyway?)
return decl;
@@ -29150,9 +29151,10 @@ finish_concept_definition (cp_expr id, tree init)
static tree
listify (tree arg)
{
- tree std_init_list = get_namespace_binding (std_node, init_list_identifier);
+ tree std_init_list = lookup_qualified_name (std_node, init_list_identifier);
- if (!std_init_list || !DECL_CLASS_TEMPLATE_P (std_init_list))
+ if (std_init_list == error_mark_node
+ || !DECL_CLASS_TEMPLATE_P (std_init_list))
{
gcc_rich_location richloc (input_location);
maybe_add_include_fixit (&richloc, "<initializer_list>", false);
diff --git a/gcc/testsuite/g++.dg/modules/pr100616_a.C
b/gcc/testsuite/g++.dg/modules/pr100616_a.C
new file mode 100644
index 00000000000..788af2eb533
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/pr100616_a.C
@@ -0,0 +1,8 @@
+// PR c++/100616
+// { dg-additional-options "-std=c++20 -fmodules-ts" }
+// { dg-module-cmi pr100616 }
+export module pr100616;
+
+template<auto> struct C { };
+struct A { };
+C<A{}> c1;
diff --git a/gcc/testsuite/g++.dg/modules/pr100616_b.C
b/gcc/testsuite/g++.dg/modules/pr100616_b.C
new file mode 100644
index 00000000000..fc89cd08ac5
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/pr100616_b.C
@@ -0,0 +1,8 @@
+// PR c++/100616
+// { dg-additional-options "-std=c++20 -fmodules-ts" }
+module pr100616;
+
+C<A{}> c2;
+
+using type = decltype(c1);
+using type = decltype(c2);
diff --git a/gcc/testsuite/g++.dg/modules/pr102576_a.H
b/gcc/testsuite/g++.dg/modules/pr102576_a.H
new file mode 100644
index 00000000000..87ba9b52031
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/pr102576_a.H
@@ -0,0 +1,5 @@
+// PR c++/102576
+// { dg-additional-options -fmodule-header }
+// { dg-module-cmi {} }
+
+#include <initializer_list>
diff --git a/gcc/testsuite/g++.dg/modules/pr102576_b.C
b/gcc/testsuite/g++.dg/modules/pr102576_b.C
new file mode 100644
index 00000000000..10251ed5304
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/pr102576_b.C
@@ -0,0 +1,9 @@
+// PR c++/102576
+// { dg-additional-options -fmodules-ts }
+
+import "pr102576_a.H";
+
+int main() {
+ for (int i : {1, 2, 3})
+ ;
+}
--
Nathan Sidwell
