Re: c++/modules: Emit definitions of ODR-used static members imported from modules [PR112899]

[Patrick Palka <ppalka@redhat.com>]

On Wed, 3 Jan 2024, Nathaniel Shead wrote:

> Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk?
> 
> -- >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.
> 
> 	PR c++/112899
> 
> gcc/cp/ChangeLog:
> 
> 	* cp-tree.h (note_variable_template_instantiation): Rename to...
> 	(note_static_storage_variable): ...this.
> 	* decl2.cc (note_variable_template_instantiation): Rename to...
> 	(note_static_storage_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.
> 
> Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com>
> ---
>  gcc/cp/cp-tree.h                        |  2 +-
>  gcc/cp/decl2.cc                         |  4 ++--
>  gcc/cp/module.cc                        |  4 ++++
>  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 +++++++++++
>  6 files changed, 28 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
> 
> diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
> index 1979572c365..ebd2850599a 100644
> --- a/gcc/cp/cp-tree.h
> +++ b/gcc/cp/cp-tree.h
> @@ -7113,7 +7113,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_static_storage_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 0850d3f5bce..241216b0dfe 100644
> --- a/gcc/cp/decl2.cc
> +++ b/gcc/cp/decl2.cc
> @@ -910,10 +910,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 with static storage duration.  */
>  
>  void
> -note_variable_template_instantiation (tree decl)
> +note_static_storage_variable (tree decl)
>  {
>    vec_safe_push (pending_statics, decl);
>  }
> diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
> index 0bd46414da9..14818131a70 100644
> --- a/gcc/cp/module.cc
> +++ b/gcc/cp/module.cc
> @@ -11752,6 +11752,10 @@ 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_CONTEXT (decl)
> +	      && RECORD_OR_UNION_TYPE_P (DECL_CONTEXT (decl))
> +	      && !DECL_TEMPLATE_INFO (decl))
> +	    note_static_storage_variable (decl);

It seems this should also handle templated inlines via

   && (!DECL_TEMPLATE_INFO (decl)
       || DECL_IMPLICIT_INSTANTIATION (decl))

otherwise the following fails to link:

  $ cat init-5_a.H
  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;
  }

  $ cat init-6_b.C
  import "init-5_a.H";

  int main() {
    __from_chars_alnum_to_val();
  }

  $ g++ -fmodules-ts -std=c++20 init-5_a.H init-5_b.C
  /usr/bin/ld: /tmp/ccNRaads.o: in function `__from_chars_alnum_to_val()':
  init-6_b.C:(.text._Z25__from_chars_alnum_to_valv[_Z25__from_chars_alnum_to_valv]+0x6): undefined reference to `__from_chars_alnum_to_val_table<false>::value'


By the way I ran into this when testing out std::print with modules:

  $ cat std.C
  export module std;
  export import <bits/stdc++.h>;

  $ cat hello.C
  import std;

  int main() {
    std::print("Hello {}!", "World");
  }

  $ g++ -fmodules-ts -std=c++26 -x c++-system-header bits/stdc++.h
  $ g++ -fmodules-ts -std=c++26 std.C hello.C && ./a.out # before
  /usr/bin/ld: /tmp/ccqNgOM1.o: in function `unsigned char std::__detail::__from_chars_alnum_to_val<false>(unsigned char)':
  hello.C:(.text._ZNSt8__detail25__from_chars_alnum_to_valILb0EEEhh[_ZNSt8__detail25__from_chars_alnum_to_valILb0EEEhh]+0x12): undefined reference to `std::__detail::__from_chars_alnum_to_val_table<false>::value'
  $ g++ -fmodules-ts -std=c++26 std.C hello.C && ./a.out # after
  Hello World!

It's great that this is so close to working!

>  	}
>        DECL_INITIAL (decl) = init;
>        if (!dyn_init)
> diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
> index f7063e09581..ce498750758 100644
> --- a/gcc/cp/pt.cc
> +++ b/gcc/cp/pt.cc
> @@ -27150,7 +27150,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_static_storage_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();
> +}
> -- 
> 2.43.0
> 
>