| Message ID | 20221006161916.4118820-1-ppalka@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | c++ modules: static var in inline function [PR104433] | expand |
On 10/6/22 12:19, Patrick Palka wrote: > The below testcase fails to link with the error > > undefined reference to `f()::y' > > ultimately because during stream out for the static VAR_DECL y we > override DECL_EXTERNAL to true, which later during IPA confuses > symbol_table::remove_unreachable_nodes into thinking it's safe > to not emit the symbol. > > The streaming code in question already avoids overriding DECL_EXTERNAL > here for DECL_VAR_DECLARED_INLINE_P vars, so it seems natural to do the > same for static vars from an DECL_DECLARED_INLINE_P function scope. > > After this patch (and r13-3134-g09df0d8b14dda6), the following now > links: > > import <memory>; > int main() { std::make_shared<int>(); } > > Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for > trunk? yeah, that's correct -- these are just as inline > > PR c++/104433 > > gcc/cp/ChangeLog: > > * module.cc (trees_out::core_bools): Don't override > DECL_EXTERNAL to true for static variables from an inline > function. > > gcc/testsuite/ChangeLog: > > * g++.dg/modules/static-2_a.H: New test. > * g++.dg/modules/static-2_b.C: New test. > --- > gcc/cp/module.cc | 3 +++ > gcc/testsuite/g++.dg/modules/static-2_a.H | 8 ++++++++ > gcc/testsuite/g++.dg/modules/static-2_b.C | 9 +++++++++ > 3 files changed, 20 insertions(+) > create mode 100644 gcc/testsuite/g++.dg/modules/static-2_a.H > create mode 100644 gcc/testsuite/g++.dg/modules/static-2_b.C > > diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc > index 79cbb346ffa..11f68794cd2 100644 > --- a/gcc/cp/module.cc > +++ b/gcc/cp/module.cc > @@ -5397,6 +5397,9 @@ trees_out::core_bools (tree t) > > case VAR_DECL: > if (TREE_PUBLIC (t) > + && !(TREE_STATIC (t) > + && DECL_FUNCTION_SCOPE_P (t) > + && DECL_DECLARED_INLINE_P (DECL_CONTEXT (t))) > && !DECL_VAR_DECLARED_INLINE_P (t)) > is_external = true; > break; > diff --git a/gcc/testsuite/g++.dg/modules/static-2_a.H b/gcc/testsuite/g++.dg/modules/static-2_a.H > new file mode 100644 > index 00000000000..b4546932a12 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/modules/static-2_a.H > @@ -0,0 +1,8 @@ > +// PR c++/104433 > +// { dg-additional-options -fmodule-header } > +// { dg-module-cmi {} } > + > +inline int* f() { > + static int y = 0; > + return &y; > +} > diff --git a/gcc/testsuite/g++.dg/modules/static-2_b.C b/gcc/testsuite/g++.dg/modules/static-2_b.C > new file mode 100644 > index 00000000000..bfd35b0fc15 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/modules/static-2_b.C > @@ -0,0 +1,9 @@ > +// PR c++/104433 > +// { dg-additional-options -fmodules-ts } > +// { dg-do link } > + > +import "static-2_a.H"; > + > +int main() { > + f(); > +}
diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc index 79cbb346ffa..11f68794cd2 100644 --- a/gcc/cp/module.cc +++ b/gcc/cp/module.cc @@ -5397,6 +5397,9 @@ trees_out::core_bools (tree t) case VAR_DECL: if (TREE_PUBLIC (t) + && !(TREE_STATIC (t) + && DECL_FUNCTION_SCOPE_P (t) + && DECL_DECLARED_INLINE_P (DECL_CONTEXT (t))) && !DECL_VAR_DECLARED_INLINE_P (t)) is_external = true; break; diff --git a/gcc/testsuite/g++.dg/modules/static-2_a.H b/gcc/testsuite/g++.dg/modules/static-2_a.H new file mode 100644 index 00000000000..b4546932a12 --- /dev/null +++ b/gcc/testsuite/g++.dg/modules/static-2_a.H @@ -0,0 +1,8 @@ +// PR c++/104433 +// { dg-additional-options -fmodule-header } +// { dg-module-cmi {} } + +inline int* f() { + static int y = 0; + return &y; +} diff --git a/gcc/testsuite/g++.dg/modules/static-2_b.C b/gcc/testsuite/g++.dg/modules/static-2_b.C new file mode 100644 index 00000000000..bfd35b0fc15 --- /dev/null +++ b/gcc/testsuite/g++.dg/modules/static-2_b.C @@ -0,0 +1,9 @@ +// PR c++/104433 +// { dg-additional-options -fmodules-ts } +// { dg-do link } + +import "static-2_a.H"; + +int main() { + f(); +}
The below testcase fails to link with the error undefined reference to `f()::y' ultimately because during stream out for the static VAR_DECL y we override DECL_EXTERNAL to true, which later during IPA confuses symbol_table::remove_unreachable_nodes into thinking it's safe to not emit the symbol. The streaming code in question already avoids overriding DECL_EXTERNAL here for DECL_VAR_DECLARED_INLINE_P vars, so it seems natural to do the same for static vars from an DECL_DECLARED_INLINE_P function scope. After this patch (and r13-3134-g09df0d8b14dda6), the following now links: import <memory>; int main() { std::make_shared<int>(); } Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for trunk? PR c++/104433 gcc/cp/ChangeLog: * module.cc (trees_out::core_bools): Don't override DECL_EXTERNAL to true for static variables from an inline function. gcc/testsuite/ChangeLog: * g++.dg/modules/static-2_a.H: New test. * g++.dg/modules/static-2_b.C: New test. --- gcc/cp/module.cc | 3 +++ gcc/testsuite/g++.dg/modules/static-2_a.H | 8 ++++++++ gcc/testsuite/g++.dg/modules/static-2_b.C | 9 +++++++++ 3 files changed, 20 insertions(+) create mode 100644 gcc/testsuite/g++.dg/modules/static-2_a.H create mode 100644 gcc/testsuite/g++.dg/modules/static-2_b.C