On 1/25/24 21:28, Nathaniel Shead wrote:
On Wed, Jan 24, 2024 at 03:24:42PM -0500, Jason Merrill wrote:
On 1/20/24 05:45, Nathaniel Shead wrote:
I also included
your change to only add class variable templates to 'pending_statics'
(and the normal 'static_decl's for non-class otherwise) as otherwise I
could imagine that they would cause issues with this later too.

That seems wrong; the 'static_decls' vec is just for checking that
static/inline variables got defined.

pending_statics has been used for template instantiations for a long time,
for non-module code; let's not mess with that in a modules patch.


OK, makes sense.

I know that there's been discussion about the correct ABI for inline
declarations, but personally I'd like to have this fixed for normal uses
in GCC14 at least, and we can revisit the specific cases where various
kinds of declarations are emitted in stage 1.

Makes sense.

P.S.  As I go to send this, I wonder if maybe something like
'note_static_member_variable' would be a clearer choice of name than
'note_static_storage_variable'?

Let's call it note_vague_linkage_variable, to go with _fn just above.


Sounds good.

-- >8 --

Static data members marked 'inline' should be emitted in TUs where they
are ODR-used.  We need to make sure that statics imported from modules
are correctly added to the 'pending_statics' map so that they get
emitted if needed, otherwise the attached testcase fails to link.

What about non-member variables marked inline, and non-member variable
template instantiations?

Jason


Non-member variables marked inline are already handled by 'static_decls'
via 'add_module_namespace_decl' and 'add_decl_to_level' during
stream-in, and then are later emitted from 'wrapup_namespace_globals'.

I'd assumed that non-member variable template instantiations would also
be handled by here, but that turns out not to be the case, since the
instantiations themselves are not (of course) namespace-scope decls.
I've added a case to the tests for this.

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

OK.

-- >8 --

Static data members marked 'inline' should be emitted in TUs where they
are ODR-used.  We need to make sure that inlines imported from modules
are correctly added to the 'pending_statics' map so that they get
emitted if needed, otherwise the attached testcase fails to link.

        PR c++/112899

gcc/cp/ChangeLog:

        * cp-tree.h (note_variable_template_instantiation): Rename to...
        (note_vague_linkage_variable): ...this.
        * decl2.cc (note_variable_template_instantiation): Rename to...
        (note_vague_linkage_variable): ...this.
        * pt.cc (instantiate_decl): Rename usage of above function.
        * module.cc (trees_in::read_var_def): Remember pending statics
        that we stream in.

gcc/testsuite/ChangeLog:

        * g++.dg/modules/init-4_a.C: New test.
        * g++.dg/modules/init-4_b.C: New test.
        * g++.dg/modules/init-6_a.H: New test.
        * g++.dg/modules/init-6_b.C: New test.

Signed-off-by: Nathaniel Shead <nathanielosh...@gmail.com>
Reviewed-by: Patrick Palka <ppa...@redhat.com>
Reviewed-by: Jason Merrill <ja...@redhat.com
---
  gcc/cp/cp-tree.h                        |  2 +-
  gcc/cp/decl2.cc                         |  4 ++--
  gcc/cp/module.cc                        |  5 +++++
  gcc/cp/pt.cc                            |  2 +-
  gcc/testsuite/g++.dg/modules/init-4_a.C |  9 +++++++++
  gcc/testsuite/g++.dg/modules/init-4_b.C | 11 +++++++++++
  gcc/testsuite/g++.dg/modules/init-6_a.H | 20 ++++++++++++++++++++
  gcc/testsuite/g++.dg/modules/init-6_b.C |  9 +++++++++
  8 files changed, 58 insertions(+), 4 deletions(-)
  create mode 100644 gcc/testsuite/g++.dg/modules/init-4_a.C
  create mode 100644 gcc/testsuite/g++.dg/modules/init-4_b.C
  create mode 100644 gcc/testsuite/g++.dg/modules/init-6_a.H
  create mode 100644 gcc/testsuite/g++.dg/modules/init-6_b.C

diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index 60e6dafc549..f46b448ce0d 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -7151,7 +7151,7 @@ extern tree maybe_get_tls_wrapper_call            (tree);
  extern void mark_needed                               (tree);
  extern bool decl_needed_p                     (tree);
  extern void note_vague_linkage_fn             (tree);
-extern void note_variable_template_instantiation (tree);
+extern void note_vague_linkage_variable                (tree);
  extern tree build_artificial_parm             (tree, tree, tree);
  extern bool possibly_inlined_p                        (tree);
  extern int parm_index                           (tree);
diff --git a/gcc/cp/decl2.cc b/gcc/cp/decl2.cc
index 91d4e2e5ea4..f569d4045ec 100644
--- a/gcc/cp/decl2.cc
+++ b/gcc/cp/decl2.cc
@@ -947,10 +947,10 @@ note_vague_linkage_fn (tree decl)
    vec_safe_push (deferred_fns, decl);
  }
-/* As above, but for variable template instantiations. */
+/* As above, but for variables.  */
void
-note_variable_template_instantiation (tree decl)
+note_vague_linkage_variable (tree decl)
  {
    vec_safe_push (pending_statics, decl);
  }
diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
index f26b2265bce..6176801b7a7 100644
--- a/gcc/cp/module.cc
+++ b/gcc/cp/module.cc
@@ -11789,6 +11789,11 @@ trees_in::read_var_def (tree decl, tree maybe_template)
          DECL_INITIALIZED_P (decl) = true;
          if (maybe_dup && DECL_INITIALIZED_BY_CONSTANT_EXPRESSION_P 
(maybe_dup))
            DECL_INITIALIZED_BY_CONSTANT_EXPRESSION_P (decl) = true;
+         if (DECL_IMPLICIT_INSTANTIATION (decl)
+             || (DECL_CLASS_SCOPE_P (decl)
+                 && !DECL_VTABLE_OR_VTT_P (decl)
+                 && !DECL_TEMPLATE_INFO (decl)))
+           note_vague_linkage_variable (decl);
        }
        DECL_INITIAL (decl) = init;
        if (!dyn_init)
diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
index 74013533b0f..f5bf159a879 100644
--- a/gcc/cp/pt.cc
+++ b/gcc/cp/pt.cc
@@ -27300,7 +27300,7 @@ instantiate_decl (tree d, bool defer_ok, bool 
expl_inst_class_mem_p)
      {
        set_instantiating_module (d);
        if (variable_template_p (gen_tmpl))
-       note_variable_template_instantiation (d);
+       note_vague_linkage_variable (d);
        instantiate_body (td, args, d, false);
      }
diff --git a/gcc/testsuite/g++.dg/modules/init-4_a.C b/gcc/testsuite/g++.dg/modules/init-4_a.C
new file mode 100644
index 00000000000..e0eb97b474e
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/init-4_a.C
@@ -0,0 +1,9 @@
+// PR c++/112899
+// { dg-additional-options "-fmodules-ts" }
+// { dg-module-cmi M }
+
+export module M;
+
+export struct A {
+  static constexpr int x = -1;
+};
diff --git a/gcc/testsuite/g++.dg/modules/init-4_b.C 
b/gcc/testsuite/g++.dg/modules/init-4_b.C
new file mode 100644
index 00000000000..d28017a1d14
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/init-4_b.C
@@ -0,0 +1,11 @@
+// PR c++/112899
+// { dg-module-do run }
+// { dg-additional-options "-fmodules-ts" }
+
+import M;
+
+int main() {
+  const int& x = A::x;
+  if (x != -1)
+    __builtin_abort();
+}
diff --git a/gcc/testsuite/g++.dg/modules/init-6_a.H 
b/gcc/testsuite/g++.dg/modules/init-6_a.H
new file mode 100644
index 00000000000..1c3fe4535d4
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/init-6_a.H
@@ -0,0 +1,20 @@
+// { dg-additional-options "-fmodule-header" }
+// { dg-module-cmi {} }
+
+template <bool _DecOnly>
+struct __from_chars_alnum_to_val_table {
+  static inline int value = 42;
+};
+
+inline unsigned char
+__from_chars_alnum_to_val() {
+  return __from_chars_alnum_to_val_table<false>::value;
+}
+
+template <bool Foo>
+static inline int nonclass_value = 42;
+
+inline unsigned char
+get_nonclass_val() {
+  return nonclass_value<false>;
+}
diff --git a/gcc/testsuite/g++.dg/modules/init-6_b.C 
b/gcc/testsuite/g++.dg/modules/init-6_b.C
new file mode 100644
index 00000000000..d704968ec37
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/init-6_b.C
@@ -0,0 +1,9 @@
+// { dg-module-do link }
+// { dg-additional-options "-fmodules-ts" }
+
+import "init-6_a.H";
+
+int main() {
+  __from_chars_alnum_to_val();
+  get_nonclass_val();
+}

Reply via email to