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 <nathanieloshead@gmail.com>
Reviewed-by: Jason Merrill <jason@redhat.com>

diff --git a/gcc/cp/decl2.cc b/gcc/cp/decl2.cc
index 4987987daed7adfc1b5f9a88487ebed40caef3ce..21156f1dd3b7a6ba40cc389a61229b4c1d25931d 100644 (file)
--- 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 beceafe05f6b5aea1b1b223705cbc3379af75987..2cded878c64bbf79d4c8032d19b2d0542ea88581 100644 (file)
--- 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 (file)
index 0000000..def3cd1
--- /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 (file)
index 0000000..5d96937
--- /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 (file)
index 0000000..8238c2a
--- /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 (file)
index 0000000..9b46f7f
--- /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 (file)
index 0000000..7185897
--- /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 (file)
index 0000000..41a1b28
--- /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 (file)
index 0000000..a91486d
--- /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 (file)
index 0000000..34dde87
--- /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 (file)
index 0000000..5b5d802
--- /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 (file)
index 0000000..7f59fb7
--- /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 (file)
index 0000000..e2f12d2
--- /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 (file)
index 23bd186..0000000
--- 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 (file)
index 1558e71..0000000
--- 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();
-}