Skip to content

Commit

Permalink
c++: Fix handling of extern templates in modules [PR112820]
Browse files Browse the repository at this point in the history
Currently, extern templates are detected by looking for the
DECL_EXTERNAL flag on a TYPE_DECL. However, this is incorrect:
TYPE_DECLs don't actually set this flag, and it happens to work by
coincidence due to TYPE_DECL_SUPPRESS_DEBUG happening to use the same
underlying bit. This however causes issues with other TYPE_DECLs that
also happen to have suppressed debug information.

Instead, this patch reworks the logic so CLASSTYPE_INTERFACE_ONLY is
always emitted into the module BMI and can then be used to check for an
extern template correctly.

Otherwise, for other declarations we always want to redetermine this:
even for declarations from the GMF, we may change our mind on whether to
import or export depending on decisions made later in the TU after
importing so we shouldn't decide this now, or necessarily reuse what the
module we'd imported had decided.

Some of this may need to change in the future to account for
itanium-cxx-abi/cxx-abi#170.

	PR c++/112820
	PR c++/102607

gcc/cp/ChangeLog:

	* module.cc (trees_out::lang_type_bools): Write interface_only
	and interface_unknown.
	(trees_in::lang_type_bools): Read the above flags.
	(trees_in::decl_value): Reset CLASSTYPE_INTERFACE_* except for
	extern templates.
	(trees_in::read_class_def): Remove buggy extern template
	handling.

gcc/testsuite/ChangeLog:

	* g++.dg/modules/debug-2_a.C: New test.
	* g++.dg/modules/debug-2_b.C: New test.
	* g++.dg/modules/debug-2_c.C: New test.
	* g++.dg/modules/debug-3_a.C: New test.
	* g++.dg/modules/debug-3_b.C: New test.

Signed-off-by: Nathaniel Shead <[email protected]>
  • Loading branch information
wreien authored and Liaoshihua committed Mar 11, 2024
1 parent 6d2b6b3 commit a5ca022
Show file tree
Hide file tree
Showing 6 changed files with 64 additions and 16 deletions.
37 changes: 21 additions & 16 deletions gcc/cp/module.cc
Original file line number Diff line number Diff line change
Expand Up @@ -5806,10 +5806,8 @@ trees_out::lang_type_bools (tree t)

WB ((lang->gets_delete >> 0) & 1);
WB ((lang->gets_delete >> 1) & 1);
// Interfaceness is recalculated upon reading. May have to revisit?
// How do dllexport and dllimport interact across a module?
// lang->interface_only
// lang->interface_unknown
WB (lang->interface_only);
WB (lang->interface_unknown);
WB (lang->contains_empty_class_p);
WB (lang->anon_aggr);
WB (lang->non_zero_init);
Expand Down Expand Up @@ -5877,9 +5875,8 @@ trees_in::lang_type_bools (tree t)
v = b () << 0;
v |= b () << 1;
lang->gets_delete = v;
// lang->interface_only
// lang->interface_unknown
lang->interface_unknown = true; // Redetermine interface
RB (lang->interface_only);
RB (lang->interface_unknown);
RB (lang->contains_empty_class_p);
RB (lang->anon_aggr);
RB (lang->non_zero_init);
Expand Down Expand Up @@ -8246,6 +8243,23 @@ trees_in::decl_value ()
/* Set the TEMPLATE_DECL's type. */
TREE_TYPE (decl) = TREE_TYPE (inner);

/* Redetermine whether we need to import or export this declaration
for this TU. But for extern templates we know we must import:
they'll be defined in a different TU.
FIXME: How do dllexport and dllimport interact across a module?
See also https://github.com/itanium-cxx-abi/cxx-abi/issues/170.
May have to revisit? */
if (type
&& CLASS_TYPE_P (type)
&& TYPE_LANG_SPECIFIC (type)
&& !(CLASSTYPE_EXPLICIT_INSTANTIATION (type)
&& CLASSTYPE_INTERFACE_KNOWN (type)
&& CLASSTYPE_INTERFACE_ONLY (type)))
{
CLASSTYPE_INTERFACE_ONLY (type) = false;
CLASSTYPE_INTERFACE_UNKNOWN (type) = true;
}

/* Add to specialization tables now that constraints etc are
added. */
if (mk == MK_partial)
Expand Down Expand Up @@ -12068,15 +12082,6 @@ trees_in::read_class_def (tree defn, tree maybe_template)
bool installing = maybe_dup && !TYPE_SIZE (type);
if (installing)
{
if (DECL_EXTERNAL (defn) && TYPE_LANG_SPECIFIC (type))
{
/* We don't deal with not-really-extern, because, for a
module you want the import to be the interface, and for a
header-unit, you're doing it wrong. */
CLASSTYPE_INTERFACE_UNKNOWN (type) = false;
CLASSTYPE_INTERFACE_ONLY (type) = true;
}

if (maybe_dup != defn)
{
// FIXME: This is needed on other defns too, almost
Expand Down
9 changes: 9 additions & 0 deletions gcc/testsuite/g++.dg/modules/debug-2_a.C
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
// PR c++/112820
// { dg-additional-options "-fmodules-ts -g" }
// { dg-module-cmi io }

export module io;

export struct error {
virtual const char* what() const noexcept;
};
8 changes: 8 additions & 0 deletions gcc/testsuite/g++.dg/modules/debug-2_b.C
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
// PR c++/112820
// { dg-additional-options "-fmodules-ts -g" }

module io;

const char* error::what() const noexcept {
return "bla";
}
9 changes: 9 additions & 0 deletions gcc/testsuite/g++.dg/modules/debug-2_c.C
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
// PR c++/112820
// { dg-module-do link }
// { dg-additional-options "-fmodules-ts -g" }

import io;

int main() {
error{};
}
8 changes: 8 additions & 0 deletions gcc/testsuite/g++.dg/modules/debug-3_a.C
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
// PR c++/102607
// { dg-additional-options "-fmodules-ts -g" }
// { dg-module-cmi mod }

export module mod;
export struct B {
virtual ~B() = default;
};
9 changes: 9 additions & 0 deletions gcc/testsuite/g++.dg/modules/debug-3_b.C
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
// PR c++/102607
// { dg-module-do link }
// { dg-additional-options "-fmodules-ts -g" }

import mod;
int main() {
struct D : B {};
(void)D{};
}

0 comments on commit a5ca022

Please sign in to comment.