https://gcc.gnu.org/g:9fcb9d39c36ae0c1b1681c68d314580ae0c820c4
commit r15-8687-g9fcb9d39c36ae0c1b1681c68d314580ae0c820c4 Author: Nathaniel Shead <nathanielosh...@gmail.com> Date: Sat Mar 22 23:04:12 2025 +1100 c++/modules: Fix explicit instantiations and gnu_inlines [PR119154] My change in r15-8012 for PR c++/119154 exposed a bug with explicit instantation declarations. The change cleared DECL_INTERFACE_KNOWN for all vague-linkage entities, including explicit instantiations. When we then perform lazy loading at EOF (due to processing deferred function bodies), expand_or_defer_fn ends up calling import_export_decl which will error because DECL_INTERFACE_KNOWN is still unset but no definition is available in the file, violating some assertions. It turns out that for function templates marked inline we would not respect an 'extern template' imported in general, either; this patch fixes both of these issues by always treating explicit instantiations as external, and so marking DECL_INTERFACE_KNOWN eagerly. For an explicit instantiation declaration we don't want to emit the body of the function as it must be emitted in a different TU anyway. And for explicit instantiation definitions we similarly know that it will have been emitted in the interface TU we streamed it in from, so there's no need to emit it. The same error can happen with lazy-loaded gnu_inlines at EOF; in some cases they'll be marked DECL_COMDAT and pass through the vague_linkage_p check anyway. This patch reworks the handling of gnu_inlines to ensure that both DECL_INTERFACE_KNOWN is always correctly set and that importing a gnu_inline function over the top of an existing forward declaration works correctly. The other case that duplicate_decls handles (importing a regular definition over the top of a gnu_inline function) doesn't seem like something we need to handle specially in modules; we'll just use the existing gnu_inline function and rely on the guarantee that there is a single non-inline function definition provided elsewhere. PR c++/119154 gcc/cp/ChangeLog: * decl2.cc (vague_linkage_p): Revert gnu_linkage handling. * module.cc (importer_interface): New enumeration. (get_importer_interface): New function. (trees_out::core_bools): Use it to determine interface. (trees_in::is_matching_decl): Propagate gnu_inline handling onto existing forward declarations. (trees_in::read_var_def): Also note explicit instantiation definitions of variable templates to be emitted. gcc/testsuite/ChangeLog: * g++.dg/modules/pr119154_a.C: Move to... * g++.dg/modules/gnu-inline-1_a.C: ...here, and add decl. * g++.dg/modules/pr119154_b.C: Move to... * g++.dg/modules/gnu-inline-1_b.C: here, and add check. * g++.dg/modules/gnu-inline-1_c.C: New test. * g++.dg/modules/gnu-inline-1_d.C: New test. * g++.dg/modules/gnu-inline-2_a.C: New test. * g++.dg/modules/gnu-inline-2_b.C: New test. * g++.dg/modules/extern-tpl-3_a.C: New test. * g++.dg/modules/extern-tpl-3_b.C: New test. * g++.dg/modules/extern-tpl-4_a.H: New test. * g++.dg/modules/extern-tpl-4_b.C: New test. * g++.dg/modules/extern-tpl-4_c.C: New test. Signed-off-by: Nathaniel Shead <nathanielosh...@gmail.com> Reviewed-by: Jason Merrill <ja...@redhat.com> Diff: --- gcc/cp/decl2.cc | 4 +- gcc/cp/module.cc | 69 +++++++++++++++++++++-- gcc/testsuite/g++.dg/modules/extern-tpl-3_a.C | 11 ++++ gcc/testsuite/g++.dg/modules/extern-tpl-3_b.C | 12 ++++ gcc/testsuite/g++.dg/modules/extern-tpl-4_a.H | 22 ++++++++ gcc/testsuite/g++.dg/modules/extern-tpl-4_b.C | 24 ++++++++ gcc/testsuite/g++.dg/modules/extern-tpl-4_c.C | 80 +++++++++++++++++++++++++++ gcc/testsuite/g++.dg/modules/gnu-inline-1_a.C | 7 +++ gcc/testsuite/g++.dg/modules/gnu-inline-1_b.C | 14 +++++ gcc/testsuite/g++.dg/modules/gnu-inline-1_c.C | 16 ++++++ gcc/testsuite/g++.dg/modules/gnu-inline-1_d.C | 16 ++++++ gcc/testsuite/g++.dg/modules/gnu-inline-2_a.C | 11 ++++ gcc/testsuite/g++.dg/modules/gnu-inline-2_b.C | 14 +++++ gcc/testsuite/g++.dg/modules/pr119154_a.C | 6 -- gcc/testsuite/g++.dg/modules/pr119154_b.C | 10 ---- 15 files changed, 293 insertions(+), 23 deletions(-) diff --git a/gcc/cp/decl2.cc b/gcc/cp/decl2.cc index 4987987daed7..21156f1dd3b7 100644 --- a/gcc/cp/decl2.cc +++ b/gcc/cp/decl2.cc @@ -2482,9 +2482,7 @@ vague_linkage_p (tree decl) DECL_COMDAT. */ if (DECL_COMDAT (decl) || (TREE_CODE (decl) == FUNCTION_DECL - && DECL_DECLARED_INLINE_P (decl) - /* But gnu_inline functions are always external. */ - && !lookup_attribute ("gnu_inline", DECL_ATTRIBUTES (decl))) + && DECL_DECLARED_INLINE_P (decl)) || (DECL_LANG_SPECIFIC (decl) && DECL_TEMPLATE_INSTANTIATION (decl)) || (VAR_P (decl) && DECL_INLINE_VAR_P (decl))) diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc index beceafe05f6b..2cded878c64b 100644 --- a/gcc/cp/module.cc +++ b/gcc/cp/module.cc @@ -5541,6 +5541,50 @@ trees_in::start (unsigned code) return t; } +/* The kinds of interface an importer could have for a decl. */ + +enum class importer_interface { + unknown, /* The definition may or may not need to be emitted. */ + always_import, /* The definition can always be found in another TU. */ + always_emit, /* The definition must be emitted in the importer's TU. */ +}; + +/* Returns what kind of interface an importer will have of DECL. */ + +static importer_interface +get_importer_interface (tree decl) +{ + /* Internal linkage entities must be emitted in each importer if + there is a definition available. */ + if (!TREE_PUBLIC (decl)) + return importer_interface::always_emit; + + /* Entities that aren't vague linkage are either not definitions or + will be emitted in this TU, so importers can just refer to an + external definition. */ + if (!vague_linkage_p (decl)) + return importer_interface::always_import; + + /* For explicit instantiations, importers can always rely on there + being a definition in another TU, unless this is a definition + in a header module: in which case the importer will always need + to emit it. */ + if (DECL_LANG_SPECIFIC (decl) + && DECL_EXPLICIT_INSTANTIATION (decl)) + return (header_module_p () && !DECL_EXTERNAL (decl) + ? importer_interface::always_emit + : importer_interface::always_import); + + /* A gnu_inline function is never emitted in any TU. */ + if (TREE_CODE (decl) == FUNCTION_DECL + && DECL_DECLARED_INLINE_P (decl) + && lookup_attribute ("gnu_inline", DECL_ATTRIBUTES (decl))) + return importer_interface::always_import; + + /* Everything else has vague linkage. */ + return importer_interface::unknown; +} + /* The structure streamers access the raw fields, because the alternative, of using the accessor macros can require using different accessors for the same underlying field, depending on the @@ -5660,7 +5704,8 @@ trees_out::core_bools (tree t, bits_out& bits) we need to import or export any vague-linkage entities on stream-in. */ bool interface_known = t->decl_common.lang_flag_5; - if (interface_known && vague_linkage_p (t)) + if (interface_known + && get_importer_interface (t) == importer_interface::unknown) interface_known = false; WB (interface_known); } @@ -5694,8 +5739,8 @@ trees_out::core_bools (tree t, bits_out& bits) is_external = true; gcc_fallthrough (); case FUNCTION_DECL: - if (TREE_PUBLIC (t) - && !vague_linkage_p (t)) + if (get_importer_interface (t) + == importer_interface::always_import) is_external = true; break; } @@ -12159,7 +12204,21 @@ trees_in::is_matching_decl (tree existing, tree decl, bool is_typedef) if (TREE_CODE (d_inner) == FUNCTION_DECL && DECL_DECLARED_INLINE_P (d_inner)) - DECL_DECLARED_INLINE_P (e_inner) = true; + { + DECL_DECLARED_INLINE_P (e_inner) = true; + if (!DECL_SAVED_TREE (e_inner) + && lookup_attribute ("gnu_inline", DECL_ATTRIBUTES (d_inner)) + && !lookup_attribute ("gnu_inline", DECL_ATTRIBUTES (e_inner))) + { + DECL_INTERFACE_KNOWN (e_inner) + |= DECL_INTERFACE_KNOWN (d_inner); + DECL_DISREGARD_INLINE_LIMITS (e_inner) + |= DECL_DISREGARD_INLINE_LIMITS (d_inner); + // TODO: we will eventually want to merge all decl attributes + duplicate_one_attribute (&DECL_ATTRIBUTES (e_inner), + DECL_ATTRIBUTES (d_inner), "gnu_inline"); + } + } if (!DECL_EXTERNAL (d_inner)) DECL_EXTERNAL (e_inner) = false; @@ -12626,6 +12685,8 @@ trees_in::read_var_def (tree decl, tree maybe_template) DECL_INITIALIZED_BY_CONSTANT_EXPRESSION_P (decl) = true; tentative_decl_linkage (decl); if (DECL_IMPLICIT_INSTANTIATION (decl) + || (DECL_EXPLICIT_INSTANTIATION (decl) + && !DECL_EXTERNAL (decl)) || (DECL_CLASS_SCOPE_P (decl) && !DECL_VTABLE_OR_VTT_P (decl) && !DECL_TEMPLATE_INFO (decl))) diff --git a/gcc/testsuite/g++.dg/modules/extern-tpl-3_a.C b/gcc/testsuite/g++.dg/modules/extern-tpl-3_a.C new file mode 100644 index 000000000000..def3cd1413d9 --- /dev/null +++ b/gcc/testsuite/g++.dg/modules/extern-tpl-3_a.C @@ -0,0 +1,11 @@ +// { dg-additional-options "-fmodules -Wno-global-module" } +// { dg-module-cmi M } + +module; +template <typename> +struct S { + S() {} +}; +export module M; +extern template class S<int>; +S<int> s; diff --git a/gcc/testsuite/g++.dg/modules/extern-tpl-3_b.C b/gcc/testsuite/g++.dg/modules/extern-tpl-3_b.C new file mode 100644 index 000000000000..5d96937ce02e --- /dev/null +++ b/gcc/testsuite/g++.dg/modules/extern-tpl-3_b.C @@ -0,0 +1,12 @@ +// { dg-additional-options "-fmodules" } + +template <typename> +struct S { + S() {} +}; + +void foo() { S<double> x;} + +import M; + +// Lazy loading of extern S<int> at EOF should not ICE diff --git a/gcc/testsuite/g++.dg/modules/extern-tpl-4_a.H b/gcc/testsuite/g++.dg/modules/extern-tpl-4_a.H new file mode 100644 index 000000000000..8238c2a8fab4 --- /dev/null +++ b/gcc/testsuite/g++.dg/modules/extern-tpl-4_a.H @@ -0,0 +1,22 @@ +// { dg-additional-options "-fmodule-header" } +// { dg-module-cmi {} } + +template <typename T> inline void ha() {} +extern template void ha<int>(); +extern template void ha<bool>(); +template void ha<char>(); + +template <typename T> void hb() {} +extern template void hb<int>(); +extern template void hb<bool>(); +template void hb<char>(); + +template <typename T> inline int hc = 123; +extern template int hc<int>; +extern template int hc<bool>; +template int hc<char>; + +template <typename T> int hd = 123; +extern template int hd<int>; +extern template int hd<bool>; +template int hd<char>; diff --git a/gcc/testsuite/g++.dg/modules/extern-tpl-4_b.C b/gcc/testsuite/g++.dg/modules/extern-tpl-4_b.C new file mode 100644 index 000000000000..9b46f7fd9437 --- /dev/null +++ b/gcc/testsuite/g++.dg/modules/extern-tpl-4_b.C @@ -0,0 +1,24 @@ +// { dg-additional-options "-fmodules" } +// { dg-module-cmi M } + +export module M; + +export template <typename T> inline void ma() {} +extern template void ma<int>(); +extern template void ma<bool>(); +template void ma<char>(); + +export template <typename T> void mb() {} +extern template void mb<int>(); +extern template void mb<bool>(); +template void mb<char>(); + +export template <typename T> inline int mc = 123; +extern template int mc<int>; +extern template int mc<bool>; +template int mc<char>; + +export template <typename T> int md = 123; +extern template int md<int>; +extern template int md<bool>; +template int md<char>; diff --git a/gcc/testsuite/g++.dg/modules/extern-tpl-4_c.C b/gcc/testsuite/g++.dg/modules/extern-tpl-4_c.C new file mode 100644 index 000000000000..71858978f778 --- /dev/null +++ b/gcc/testsuite/g++.dg/modules/extern-tpl-4_c.C @@ -0,0 +1,80 @@ +// { dg-additional-options "-fmodules" } + +import "extern-tpl-4_a.H"; +import M; + +int main() { + ha<int>(); + ha<char>(); + ha<double>(); + + ma<int>(); + ma<char>(); + ma<double>(); + + hb<int>(); + hb<char>(); + hb<double>(); + + mb<int>(); + mb<char>(); + mb<double>(); + + int x1 = hc<int> + hc<char> + hc<double>; + int x2 = hd<int> + hd<char> + hd<double>; + int x3 = mc<int> + mc<char> + mc<double>; + int x4 = md<int> + md<char> + md<double>; + return x1 + x2 + x3 + x4; +} + + +// 'int': imported explicit instantiation decls should not be emitted here: +// { dg-final { scan-assembler-not "_Z2haIiEvv:" } } +// { dg-final { scan-assembler-not "_Z2hbIiEvv:" } } +// { dg-final { scan-assembler-not "_Z2hcIiE:" } } +// { dg-final { scan-assembler-not "_Z2hdIiE:" } } +// { dg-final { scan-assembler-not "_ZW1M2maIiEvv:" } } +// { dg-final { scan-assembler-not "_ZW1M2mbIiEvv:" } } +// { dg-final { scan-assembler-not "_ZW1M2mcIiE:" } } +// { dg-final { scan-assembler-not "_ZW1M2mdIiE:" } } + +// 'char': explicit instantiation definitions don't need to be emitted for +// modules, but need to be emitted for header units (as there's no other TU): +// { dg-final { scan-assembler "_Z2haIcEvv:" } } +// { dg-final { scan-assembler "_Z2hbIcEvv:" } } +// { dg-final { scan-assembler "_Z2hcIcE:" } } +// { dg-final { scan-assembler "_Z2hdIcE:" } } +// { dg-final { scan-assembler-not "_ZW1M2maIcEvv:" } } +// { dg-final { scan-assembler-not "_ZW1M2mbIcEvv:" } } +// { dg-final { scan-assembler-not "_ZW1M2mcIcE:" } } +// { dg-final { scan-assembler-not "_ZW1M2mdIcE:" } } + +// 'double': these are not explicitly instantiated and should be emitted here: +// { dg-final { scan-assembler "_Z2haIdEvv:" } } +// { dg-final { scan-assembler "_Z2hbIdEvv:" } } +// { dg-final { scan-assembler "_Z2hcIdE:" } } +// { dg-final { scan-assembler "_Z2hdIdE:" } } +// { dg-final { scan-assembler "_ZW1M2maIdEvv:" } } +// { dg-final { scan-assembler "_ZW1M2mbIdEvv:" } } +// { dg-final { scan-assembler "_ZW1M2mcIdE:" } } +// { dg-final { scan-assembler "_ZW1M2mdIdE:" } } + +template void ha<bool>(); +template void hb<bool>(); +template int hc<bool>; +template int hd<bool>; + +template void ma<bool>(); +template void mb<bool>(); +template int mc<bool>; +template int md<bool>; + +// 'bool': instantiated in this file, and so must be emitted here: +// { dg-final { scan-assembler "_Z2haIbEvv:" } } +// { dg-final { scan-assembler "_Z2hbIbEvv:" } } +// { dg-final { scan-assembler "_Z2hcIbE:" } } +// { dg-final { scan-assembler "_Z2hdIbE:" } } +// { dg-final { scan-assembler "_ZW1M2maIbEvv:" } } +// { dg-final { scan-assembler "_ZW1M2mbIbEvv:" } } +// { dg-final { scan-assembler "_ZW1M2mcIbE:" } } +// { dg-final { scan-assembler "_ZW1M2mdIbE:" } } diff --git a/gcc/testsuite/g++.dg/modules/gnu-inline-1_a.C b/gcc/testsuite/g++.dg/modules/gnu-inline-1_a.C new file mode 100644 index 000000000000..41a1b2850d42 --- /dev/null +++ b/gcc/testsuite/g++.dg/modules/gnu-inline-1_a.C @@ -0,0 +1,7 @@ +// PR c++/119154 +// { dg-additional-options "-fmodules" } +// { dg-module-cmi foo } + +export module foo; +export extern "C++" inline __attribute__((__gnu_inline__)) void bar() {} +export extern "C++" inline __attribute__((__gnu_inline__)) void decl(); diff --git a/gcc/testsuite/g++.dg/modules/gnu-inline-1_b.C b/gcc/testsuite/g++.dg/modules/gnu-inline-1_b.C new file mode 100644 index 000000000000..a91486d84031 --- /dev/null +++ b/gcc/testsuite/g++.dg/modules/gnu-inline-1_b.C @@ -0,0 +1,14 @@ +// PR c++/119154 +// { dg-additional-options "-fmodules" } + +void bar(); +void decl(); // { dg-warning "used but never defined" } +import foo; + +void test_b() { + bar(); + decl(); +} + +// A function only defined with gnu_inline should not be emitted here. +// { dg-final { scan-assembler-not "_Z3barv:" } } diff --git a/gcc/testsuite/g++.dg/modules/gnu-inline-1_c.C b/gcc/testsuite/g++.dg/modules/gnu-inline-1_c.C new file mode 100644 index 000000000000..34dde8787455 --- /dev/null +++ b/gcc/testsuite/g++.dg/modules/gnu-inline-1_c.C @@ -0,0 +1,16 @@ +// PR c++/119154 +// { dg-additional-options "-fmodules" } + +void bar() {} +void decl() {} +import foo; + +void test_c() { + bar(); + decl(); +}; + +// Make sure importing a gnu_inline definition didn't stop us from emitting +// the non-gnu_inline definition we had before the module import. +// { dg-final { scan-assembler "_Z3barv:" } } +// { dg-final { scan-assembler "_Z4declv:" } } diff --git a/gcc/testsuite/g++.dg/modules/gnu-inline-1_d.C b/gcc/testsuite/g++.dg/modules/gnu-inline-1_d.C new file mode 100644 index 000000000000..5b5d802dcd9a --- /dev/null +++ b/gcc/testsuite/g++.dg/modules/gnu-inline-1_d.C @@ -0,0 +1,16 @@ +// PR c++/119154 +// { dg-additional-options "-fmodules -fno-module-lazy" } + +import foo; +void bar() {} +void decl() {} + +void test_c() { + bar(); + decl(); +}; + +// Make sure importing a gnu_inline definition didn't stop us from emitting +// the non-gnu_inline definition we had after the module import. +// { dg-final { scan-assembler "_Z3barv:" } } +// { dg-final { scan-assembler "_Z4declv:" } } diff --git a/gcc/testsuite/g++.dg/modules/gnu-inline-2_a.C b/gcc/testsuite/g++.dg/modules/gnu-inline-2_a.C new file mode 100644 index 000000000000..7f59fb7f716c --- /dev/null +++ b/gcc/testsuite/g++.dg/modules/gnu-inline-2_a.C @@ -0,0 +1,11 @@ +// PR c++/119154 +// { dg-additional-options "-fmodules" } +// { dg-module-cmi xstd } + +export module xstd; + +inline __attribute__((__gnu_inline__)) void wmemset() {} + +extern "C++" template <class> struct char_traits { + void assign() { wmemset(); } +}; diff --git a/gcc/testsuite/g++.dg/modules/gnu-inline-2_b.C b/gcc/testsuite/g++.dg/modules/gnu-inline-2_b.C new file mode 100644 index 000000000000..e2f12d2d0b62 --- /dev/null +++ b/gcc/testsuite/g++.dg/modules/gnu-inline-2_b.C @@ -0,0 +1,14 @@ +// PR c++/119154 +// { dg-additional-options "-fmodules" } + +template <typename> struct char_traits { + void assign(); +}; + +void foo(char_traits<wchar_t> s) { + s.assign(); +} + +import xstd; + +// Lazy loading at EOF of a gnu_inline declaration should not ICE. diff --git a/gcc/testsuite/g++.dg/modules/pr119154_a.C b/gcc/testsuite/g++.dg/modules/pr119154_a.C deleted file mode 100644 index 23bd186fe191..000000000000 --- a/gcc/testsuite/g++.dg/modules/pr119154_a.C +++ /dev/null @@ -1,6 +0,0 @@ -// PR c++/119154 -// { dg-additional-options "-fmodules" } -// { dg-module-cmi foo } - -export module foo; -extern "C++" inline __attribute__((__gnu_inline__)) void bar() {} diff --git a/gcc/testsuite/g++.dg/modules/pr119154_b.C b/gcc/testsuite/g++.dg/modules/pr119154_b.C deleted file mode 100644 index 1558e717761d..000000000000 --- a/gcc/testsuite/g++.dg/modules/pr119154_b.C +++ /dev/null @@ -1,10 +0,0 @@ -// PR c++/119154 -// { dg-module-do link } -// { dg-additional-options "-fmodules" } - -void bar(); -import foo; - -int main() { - bar(); -}