Message ID | 659bc714.170a0220.c04dd.cc6a@mx.google.com |
---|---|
State | New |
Headers | show |
Series | [v2] c++/modules: Differentiate extern templates and TYPE_DECL_SUPPRESS_DEBUG [PR112820] | expand |
On Mon, Jan 8, 2024 at 10:58 AM Nathaniel Shead <nathanieloshead@gmail.com> wrote: > > On Thu, Jan 04, 2024 at 03:39:15PM -0500, Patrick Palka wrote: > > On Sun, 3 Dec 2023, Nathaniel Shead wrote: > > > > > Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk? > > > > > > -- >8 -- > > > > > > The TYPE_DECL_SUPPRESS_DEBUG and DECL_EXTERNAL flags use the same > > > underlying bit. This is causing confusion when attempting to determine > > > the interface for a streamed-in class type, since the modules code > > > currently assumes that all DECL_EXTERNAL types are extern templates. > > > However, when -g is specified then TYPE_DECL_SUPPRESS_DEBUG (and hence > > > DECL_EXTERNAL) is marked on various other kinds of declarations, such as > > > vtables, which causes them to never be emitted. > > > > Good catch.. Maybe we should use different bits for these flags? I wouldn't be > > surprised if this bit sharing causes issues elsewhere in the compiler. The > > documentation in tree.h / tree-core.h says DECL_EXTERNAL is only valid for > > VAR_DECL and FUNCTION_DECL, so at one point it was safe to share the same bit > > but that's not true anymore it seems. > > > > Looking at tree-core.h:tree_decl_common luckily we have plenty of spare bits. > > We could also e.g. make TYPE_DECL_SUPPRESS_DEBUG use the decl_not_flexarray bit > > which is otherwise only used for FIELD_DECL. > > > > That seems like a good idea, thanks. How does this look? > > Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk? OK if C++ folks are fine. Richard. > -- >8 -- > > Currently, DECL_EXTERNAL and TYPE_DECL_SUPPRESS_DEBUG share a bit. This > causes issues with module code, which then incorrectly assumes that > anything with suppressed debug info (such as vtables when '-g' is > specified) is an extern template and thus prevents their emission. > > This patch splits the two flags up; extern templates continue to use the > DECL_EXTERNAL flag (and the documentation is updated to indicate this), > but TYPE_DECL_SUPPRESS_DEBUG now uses the 'decl_not_flexarray' flag, > which currently is only used by FIELD_DECLs. > > PR c++/112820 > PR c++/102607 > > gcc/cp/ChangeLog: > > * pt.cc (mark_class_instantiated): Set DECL_EXTERNAL explicitly. > > gcc/ChangeLog: > > * tree-core.h (struct tree_decl_common): Update comments. > * tree.h (DECL_EXTERNAL): Update comments. > (TYPE_DECL_SUPPRESS_DEBUG): Use 'decl_not_flexarray' instead. > > 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 <nathanieloshead@gmail.com> > --- > gcc/cp/pt.cc | 1 + > gcc/testsuite/g++.dg/modules/debug-2_a.C | 9 +++++++++ > gcc/testsuite/g++.dg/modules/debug-2_b.C | 8 ++++++++ > gcc/testsuite/g++.dg/modules/debug-2_c.C | 9 +++++++++ > gcc/testsuite/g++.dg/modules/debug-3_a.C | 8 ++++++++ > gcc/testsuite/g++.dg/modules/debug-3_b.C | 9 +++++++++ > gcc/tree-core.h | 6 +++--- > gcc/tree.h | 8 ++++---- > 8 files changed, 51 insertions(+), 7 deletions(-) > create mode 100644 gcc/testsuite/g++.dg/modules/debug-2_a.C > create mode 100644 gcc/testsuite/g++.dg/modules/debug-2_b.C > create mode 100644 gcc/testsuite/g++.dg/modules/debug-2_c.C > create mode 100644 gcc/testsuite/g++.dg/modules/debug-3_a.C > create mode 100644 gcc/testsuite/g++.dg/modules/debug-3_b.C > > diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc > index e38e7a773f0..7839745035b 100644 > --- a/gcc/cp/pt.cc > +++ b/gcc/cp/pt.cc > @@ -26256,6 +26256,7 @@ mark_class_instantiated (tree t, int extern_p) > SET_CLASSTYPE_EXPLICIT_INSTANTIATION (t); > SET_CLASSTYPE_INTERFACE_KNOWN (t); > CLASSTYPE_INTERFACE_ONLY (t) = extern_p; > + DECL_EXTERNAL (TYPE_NAME (t)) = extern_p; > TYPE_DECL_SUPPRESS_DEBUG (TYPE_NAME (t)) = extern_p; > if (! extern_p) > { > diff --git a/gcc/testsuite/g++.dg/modules/debug-2_a.C b/gcc/testsuite/g++.dg/modules/debug-2_a.C > new file mode 100644 > index 00000000000..eed0905542b > --- /dev/null > +++ b/gcc/testsuite/g++.dg/modules/debug-2_a.C > @@ -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; > +}; > diff --git a/gcc/testsuite/g++.dg/modules/debug-2_b.C b/gcc/testsuite/g++.dg/modules/debug-2_b.C > new file mode 100644 > index 00000000000..fc9afbc02e0 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/modules/debug-2_b.C > @@ -0,0 +1,8 @@ > +// PR c++/112820 > +// { dg-additional-options "-fmodules-ts -g" } > + > +module io; > + > +const char* error::what() const noexcept { > + return "bla"; > +} > diff --git a/gcc/testsuite/g++.dg/modules/debug-2_c.C b/gcc/testsuite/g++.dg/modules/debug-2_c.C > new file mode 100644 > index 00000000000..37117f69dcd > --- /dev/null > +++ b/gcc/testsuite/g++.dg/modules/debug-2_c.C > @@ -0,0 +1,9 @@ > +// PR c++/112820 > +// { dg-module-do link } > +// { dg-additional-options "-fmodules-ts -g" } > + > +import io; > + > +int main() { > + error{}; > +} > diff --git a/gcc/testsuite/g++.dg/modules/debug-3_a.C b/gcc/testsuite/g++.dg/modules/debug-3_a.C > new file mode 100644 > index 00000000000..9e33d8260fd > --- /dev/null > +++ b/gcc/testsuite/g++.dg/modules/debug-3_a.C > @@ -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; > +}; > diff --git a/gcc/testsuite/g++.dg/modules/debug-3_b.C b/gcc/testsuite/g++.dg/modules/debug-3_b.C > new file mode 100644 > index 00000000000..03c78b71b5d > --- /dev/null > +++ b/gcc/testsuite/g++.dg/modules/debug-3_b.C > @@ -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{}; > +} > diff --git a/gcc/tree-core.h b/gcc/tree-core.h > index 8a89462bd7e..1ca4d4c07bd 100644 > --- a/gcc/tree-core.h > +++ b/gcc/tree-core.h > @@ -1814,8 +1814,7 @@ struct GTY(()) tree_decl_common { > In FIELD_DECL, this is DECL_FIELD_ABI_IGNORED. */ > unsigned decl_flag_0 : 1; > /* In FIELD_DECL, this is DECL_BIT_FIELD > - In VAR_DECL and FUNCTION_DECL, this is DECL_EXTERNAL. > - In TYPE_DECL, this is TYPE_DECL_SUPPRESS_DEBUG. */ > + In VAR_DECL, FUNCTION_DECL, and TYPE_DECL, this is DECL_EXTERNAL. */ > unsigned decl_flag_1 : 1; > /* In FIELD_DECL, this is DECL_NONADDRESSABLE_P > In VAR_DECL, PARM_DECL and RESULT_DECL, this is > @@ -1845,7 +1844,8 @@ struct GTY(()) tree_decl_common { > TYPE_WARN_IF_NOT_ALIGN. */ > unsigned int warn_if_not_align : 6; > > - /* In FIELD_DECL, this is DECL_NOT_FLEXARRAY. */ > + /* In FIELD_DECL, this is DECL_NOT_FLEXARRAY. > + In TYPE_DECL, this is TYPE_DECL_SUPPRESS_DEBUG. */ > unsigned int decl_not_flexarray : 1; > > /* 5 bits unused. */ > diff --git a/gcc/tree.h b/gcc/tree.h > index 972a067a1f7..8bdc471ad4e 100644 > --- a/gcc/tree.h > +++ b/gcc/tree.h > @@ -2848,9 +2848,9 @@ extern tree vector_element_bits_tree (const_tree); > #define DECL_LANG_SPECIFIC(NODE) \ > (DECL_COMMON_CHECK (NODE)->decl_common.lang_specific) > > -/* In a VAR_DECL or FUNCTION_DECL, nonzero means external reference: > - do not allocate storage, and refer to a definition elsewhere. Note that > - this does not necessarily imply the entity represented by NODE > +/* In a VAR_DECL, FUNCTION_DECL, or TYPE_DECL, nonzero means external > + reference: do not allocate storage, and refer to a definition elsewhere. > + Note that this does not necessarily imply the entity represented by NODE > has no program source-level definition in this translation unit. For > example, for a FUNCTION_DECL, DECL_SAVED_TREE may be non-NULL and > DECL_EXTERNAL may be true simultaneously; that can be the case for > @@ -3553,7 +3553,7 @@ extern vec<tree, va_gc> **decl_debug_args_insert (tree); > into stabs. Instead it will generate cross reference ('x') of names. > This uses the same flag as DECL_EXTERNAL. */ > #define TYPE_DECL_SUPPRESS_DEBUG(NODE) \ > - (TYPE_DECL_CHECK (NODE)->decl_common.decl_flag_1) > + (TYPE_DECL_CHECK (NODE)->decl_common.decl_not_flexarray) > > /* Getter of the imported declaration associated to the > IMPORTED_DECL node. */ > -- > 2.43.0 >
On Mon, 8 Jan 2024, Nathaniel Shead wrote: > On Thu, Jan 04, 2024 at 03:39:15PM -0500, Patrick Palka wrote: > > On Sun, 3 Dec 2023, Nathaniel Shead wrote: > > > > > Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk? > > > > > > -- >8 -- > > > > > > The TYPE_DECL_SUPPRESS_DEBUG and DECL_EXTERNAL flags use the same > > > underlying bit. This is causing confusion when attempting to determine > > > the interface for a streamed-in class type, since the modules code > > > currently assumes that all DECL_EXTERNAL types are extern templates. > > > However, when -g is specified then TYPE_DECL_SUPPRESS_DEBUG (and hence > > > DECL_EXTERNAL) is marked on various other kinds of declarations, such as > > > vtables, which causes them to never be emitted. > > > > Good catch.. Maybe we should use different bits for these flags? I wouldn't be > > surprised if this bit sharing causes issues elsewhere in the compiler. The > > documentation in tree.h / tree-core.h says DECL_EXTERNAL is only valid for > > VAR_DECL and FUNCTION_DECL, so at one point it was safe to share the same bit > > but that's not true anymore it seems. > > > > Looking at tree-core.h:tree_decl_common luckily we have plenty of spare bits. > > We could also e.g. make TYPE_DECL_SUPPRESS_DEBUG use the decl_not_flexarray bit > > which is otherwise only used for FIELD_DECL. > > > > That seems like a good idea, thanks. How does this look? > > Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk? > > -- >8 -- > > Currently, DECL_EXTERNAL and TYPE_DECL_SUPPRESS_DEBUG share a bit. This > causes issues with module code, which then incorrectly assumes that > anything with suppressed debug info (such as vtables when '-g' is > specified) is an extern template and thus prevents their emission. > > This patch splits the two flags up; extern templates continue to use the > DECL_EXTERNAL flag (and the documentation is updated to indicate this), > but TYPE_DECL_SUPPRESS_DEBUG now uses the 'decl_not_flexarray' flag, > which currently is only used by FIELD_DECLs. > > PR c++/112820 > PR c++/102607 > > gcc/cp/ChangeLog: > > * pt.cc (mark_class_instantiated): Set DECL_EXTERNAL explicitly. > > gcc/ChangeLog: > > * tree-core.h (struct tree_decl_common): Update comments. > * tree.h (DECL_EXTERNAL): Update comments. > (TYPE_DECL_SUPPRESS_DEBUG): Use 'decl_not_flexarray' instead. > > 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 <nathanieloshead@gmail.com> > --- > gcc/cp/pt.cc | 1 + > gcc/testsuite/g++.dg/modules/debug-2_a.C | 9 +++++++++ > gcc/testsuite/g++.dg/modules/debug-2_b.C | 8 ++++++++ > gcc/testsuite/g++.dg/modules/debug-2_c.C | 9 +++++++++ > gcc/testsuite/g++.dg/modules/debug-3_a.C | 8 ++++++++ > gcc/testsuite/g++.dg/modules/debug-3_b.C | 9 +++++++++ > gcc/tree-core.h | 6 +++--- > gcc/tree.h | 8 ++++---- > 8 files changed, 51 insertions(+), 7 deletions(-) > create mode 100644 gcc/testsuite/g++.dg/modules/debug-2_a.C > create mode 100644 gcc/testsuite/g++.dg/modules/debug-2_b.C > create mode 100644 gcc/testsuite/g++.dg/modules/debug-2_c.C > create mode 100644 gcc/testsuite/g++.dg/modules/debug-3_a.C > create mode 100644 gcc/testsuite/g++.dg/modules/debug-3_b.C > > diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc > index e38e7a773f0..7839745035b 100644 > --- a/gcc/cp/pt.cc > +++ b/gcc/cp/pt.cc > @@ -26256,6 +26256,7 @@ mark_class_instantiated (tree t, int extern_p) > SET_CLASSTYPE_EXPLICIT_INSTANTIATION (t); > SET_CLASSTYPE_INTERFACE_KNOWN (t); > CLASSTYPE_INTERFACE_ONLY (t) = extern_p; > + DECL_EXTERNAL (TYPE_NAME (t)) = extern_p; > TYPE_DECL_SUPPRESS_DEBUG (TYPE_NAME (t)) = extern_p; > if (! extern_p) > { > diff --git a/gcc/testsuite/g++.dg/modules/debug-2_a.C b/gcc/testsuite/g++.dg/modules/debug-2_a.C > new file mode 100644 > index 00000000000..eed0905542b > --- /dev/null > +++ b/gcc/testsuite/g++.dg/modules/debug-2_a.C > @@ -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; > +}; > diff --git a/gcc/testsuite/g++.dg/modules/debug-2_b.C b/gcc/testsuite/g++.dg/modules/debug-2_b.C > new file mode 100644 > index 00000000000..fc9afbc02e0 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/modules/debug-2_b.C > @@ -0,0 +1,8 @@ > +// PR c++/112820 > +// { dg-additional-options "-fmodules-ts -g" } > + > +module io; > + > +const char* error::what() const noexcept { > + return "bla"; > +} > diff --git a/gcc/testsuite/g++.dg/modules/debug-2_c.C b/gcc/testsuite/g++.dg/modules/debug-2_c.C > new file mode 100644 > index 00000000000..37117f69dcd > --- /dev/null > +++ b/gcc/testsuite/g++.dg/modules/debug-2_c.C > @@ -0,0 +1,9 @@ > +// PR c++/112820 > +// { dg-module-do link } > +// { dg-additional-options "-fmodules-ts -g" } > + > +import io; > + > +int main() { > + error{}; > +} > diff --git a/gcc/testsuite/g++.dg/modules/debug-3_a.C b/gcc/testsuite/g++.dg/modules/debug-3_a.C > new file mode 100644 > index 00000000000..9e33d8260fd > --- /dev/null > +++ b/gcc/testsuite/g++.dg/modules/debug-3_a.C > @@ -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; > +}; > diff --git a/gcc/testsuite/g++.dg/modules/debug-3_b.C b/gcc/testsuite/g++.dg/modules/debug-3_b.C > new file mode 100644 > index 00000000000..03c78b71b5d > --- /dev/null > +++ b/gcc/testsuite/g++.dg/modules/debug-3_b.C > @@ -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{}; > +} > diff --git a/gcc/tree-core.h b/gcc/tree-core.h > index 8a89462bd7e..1ca4d4c07bd 100644 > --- a/gcc/tree-core.h > +++ b/gcc/tree-core.h > @@ -1814,8 +1814,7 @@ struct GTY(()) tree_decl_common { > In FIELD_DECL, this is DECL_FIELD_ABI_IGNORED. */ > unsigned decl_flag_0 : 1; > /* In FIELD_DECL, this is DECL_BIT_FIELD > - In VAR_DECL and FUNCTION_DECL, this is DECL_EXTERNAL. > - In TYPE_DECL, this is TYPE_DECL_SUPPRESS_DEBUG. */ > + In VAR_DECL, FUNCTION_DECL, and TYPE_DECL, this is DECL_EXTERNAL. */ > unsigned decl_flag_1 : 1; > /* In FIELD_DECL, this is DECL_NONADDRESSABLE_P > In VAR_DECL, PARM_DECL and RESULT_DECL, this is > @@ -1845,7 +1844,8 @@ struct GTY(()) tree_decl_common { > TYPE_WARN_IF_NOT_ALIGN. */ > unsigned int warn_if_not_align : 6; > > - /* In FIELD_DECL, this is DECL_NOT_FLEXARRAY. */ > + /* In FIELD_DECL, this is DECL_NOT_FLEXARRAY. > + In TYPE_DECL, this is TYPE_DECL_SUPPRESS_DEBUG. */ > unsigned int decl_not_flexarray : 1; > > /* 5 bits unused. */ > diff --git a/gcc/tree.h b/gcc/tree.h > index 972a067a1f7..8bdc471ad4e 100644 > --- a/gcc/tree.h > +++ b/gcc/tree.h > @@ -2848,9 +2848,9 @@ extern tree vector_element_bits_tree (const_tree); > #define DECL_LANG_SPECIFIC(NODE) \ > (DECL_COMMON_CHECK (NODE)->decl_common.lang_specific) > > -/* In a VAR_DECL or FUNCTION_DECL, nonzero means external reference: > - do not allocate storage, and refer to a definition elsewhere. Note that > - this does not necessarily imply the entity represented by NODE > +/* In a VAR_DECL, FUNCTION_DECL, or TYPE_DECL, nonzero means external > + reference: do not allocate storage, and refer to a definition elsewhere. > + Note that this does not necessarily imply the entity represented by NODE > has no program source-level definition in this translation unit. For > example, for a FUNCTION_DECL, DECL_SAVED_TREE may be non-NULL and > DECL_EXTERNAL may be true simultaneously; that can be the case for > @@ -3553,7 +3553,7 @@ extern vec<tree, va_gc> **decl_debug_args_insert (tree); > into stabs. Instead it will generate cross reference ('x') of names. > This uses the same flag as DECL_EXTERNAL. */ I guess this last sentence should be removed since it's no longer true, LGTM otherwise, thanks! > #define TYPE_DECL_SUPPRESS_DEBUG(NODE) \ > - (TYPE_DECL_CHECK (NODE)->decl_common.decl_flag_1) > + (TYPE_DECL_CHECK (NODE)->decl_common.decl_not_flexarray) > > /* Getter of the imported declaration associated to the > IMPORTED_DECL node. */ > -- > 2.43.0 > >
On 1/8/24 10:27, Patrick Palka wrote: > On Mon, 8 Jan 2024, Nathaniel Shead wrote: >> On Thu, Jan 04, 2024 at 03:39:15PM -0500, Patrick Palka wrote: >>> On Sun, 3 Dec 2023, Nathaniel Shead wrote: >>>> >>>> The TYPE_DECL_SUPPRESS_DEBUG and DECL_EXTERNAL flags use the same >>>> underlying bit. This is causing confusion when attempting to determine >>>> the interface for a streamed-in class type, since the modules code >>>> currently assumes that all DECL_EXTERNAL types are extern templates. >>>> However, when -g is specified then TYPE_DECL_SUPPRESS_DEBUG (and hence >>>> DECL_EXTERNAL) is marked on various other kinds of declarations, such as >>>> vtables, which causes them to never be emitted. But a vtable isn't a TYPE_DECL? I suspect what you mean is that maybe_suppress_debug_info is setting TYPE_DECL_SUPPRESS_DEBUG to try to avoid duplication of debug info for classes with vtables, and then the modules code is wrongly assuming that you can check DECL_EXTERNAL for TYPE_DECL, and that it's set only if CLASSTYPE_INTERFACE_ONLY is also set, which is wrong in this case, so we avoid emitting the vtable or anything else for that class. It seems unnecessary to start setting DECL_EXTERNAL on the TYPE_DECL to mean the exact same thing as CLASSTYPE_INTERFACE_ONLY. Rather, the modules code should stop trying to check DECL_EXTERNAL on a TYPE_DECL. Under what circumstances does it make sense for CLASSTYPE_INTERFACE_ONLY to be set in the context of modules, anyway? We probably want to propagate it for things in the global module so that various libstdc++ explicit instantiations work the same with import std. For an class imported from a named module, this ties into the earlier discussion about vtables and inlines that hasn't resolved yet in the ABI committee. But it's certainly significantly interface-like. And I would expect maybe_suppress_debug_info to suppress the debug info for such a class on the assumption that the module unit has the needed debug info. Jason
On Mon, Jan 15, 2024 at 06:10:55PM -0500, Jason Merrill wrote: > On 1/8/24 10:27, Patrick Palka wrote: > > On Mon, 8 Jan 2024, Nathaniel Shead wrote: > > > On Thu, Jan 04, 2024 at 03:39:15PM -0500, Patrick Palka wrote: > > > > On Sun, 3 Dec 2023, Nathaniel Shead wrote: > > > > > > > > > > The TYPE_DECL_SUPPRESS_DEBUG and DECL_EXTERNAL flags use the same > > > > > underlying bit. This is causing confusion when attempting to determine > > > > > the interface for a streamed-in class type, since the modules code > > > > > currently assumes that all DECL_EXTERNAL types are extern templates. > > > > > However, when -g is specified then TYPE_DECL_SUPPRESS_DEBUG (and hence > > > > > DECL_EXTERNAL) is marked on various other kinds of declarations, such as > > > > > vtables, which causes them to never be emitted. > > But a vtable isn't a TYPE_DECL? > > I suspect what you mean is that maybe_suppress_debug_info is setting > TYPE_DECL_SUPPRESS_DEBUG to try to avoid duplication of debug info for > classes with vtables, and then the modules code is wrongly assuming that you > can check DECL_EXTERNAL for TYPE_DECL, and that it's set only if > CLASSTYPE_INTERFACE_ONLY is also set, which is wrong in this case, so we > avoid emitting the vtable or anything else for that class. > Ah, right; I definitely misread what was going on. > It seems unnecessary to start setting DECL_EXTERNAL on the TYPE_DECL to mean > the exact same thing as CLASSTYPE_INTERFACE_ONLY. Rather, the modules code > should stop trying to check DECL_EXTERNAL on a TYPE_DECL. > > Under what circumstances does it make sense for CLASSTYPE_INTERFACE_ONLY to > be set in the context of modules, anyway? We probably want to propagate it > for things in the global module so that various libstdc++ explicit > instantiations work the same with import std. > > For an class imported from a named module, this ties into the earlier > discussion about vtables and inlines that hasn't resolved yet in the ABI > committee. But it's certainly significantly interface-like. And I would > expect maybe_suppress_debug_info to suppress the debug info for such a class > on the assumption that the module unit has the needed debug info. > > Jason > I didn't consider messing with 'CLASSTYPE_INTERFACE_ONLY' because the modules code currently always sets 'lang->interface_unknown = true' on read with the comment "Redetermine interface". But I see now as well this comment: // Interfaceness is recalculated upon reading. May have to revisit? // How do dllexport and dllimport interact across a module? // lang->interface_only // lang->interface_unknown So it seems reasonable to reconsider this now. I hadn't considered declarations in the GMF but I think you're right that we'll want to propagate it; I guess in general things in the GMF should try to behave as close to how they would have if they were #included directly in the importing TU. I suspect this fix will be more involved but I'll try to wrap my head around it and see what I can come up with.
diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc index e38e7a773f0..7839745035b 100644 --- a/gcc/cp/pt.cc +++ b/gcc/cp/pt.cc @@ -26256,6 +26256,7 @@ mark_class_instantiated (tree t, int extern_p) SET_CLASSTYPE_EXPLICIT_INSTANTIATION (t); SET_CLASSTYPE_INTERFACE_KNOWN (t); CLASSTYPE_INTERFACE_ONLY (t) = extern_p; + DECL_EXTERNAL (TYPE_NAME (t)) = extern_p; TYPE_DECL_SUPPRESS_DEBUG (TYPE_NAME (t)) = extern_p; if (! extern_p) { diff --git a/gcc/testsuite/g++.dg/modules/debug-2_a.C b/gcc/testsuite/g++.dg/modules/debug-2_a.C new file mode 100644 index 00000000000..eed0905542b --- /dev/null +++ b/gcc/testsuite/g++.dg/modules/debug-2_a.C @@ -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; +}; diff --git a/gcc/testsuite/g++.dg/modules/debug-2_b.C b/gcc/testsuite/g++.dg/modules/debug-2_b.C new file mode 100644 index 00000000000..fc9afbc02e0 --- /dev/null +++ b/gcc/testsuite/g++.dg/modules/debug-2_b.C @@ -0,0 +1,8 @@ +// PR c++/112820 +// { dg-additional-options "-fmodules-ts -g" } + +module io; + +const char* error::what() const noexcept { + return "bla"; +} diff --git a/gcc/testsuite/g++.dg/modules/debug-2_c.C b/gcc/testsuite/g++.dg/modules/debug-2_c.C new file mode 100644 index 00000000000..37117f69dcd --- /dev/null +++ b/gcc/testsuite/g++.dg/modules/debug-2_c.C @@ -0,0 +1,9 @@ +// PR c++/112820 +// { dg-module-do link } +// { dg-additional-options "-fmodules-ts -g" } + +import io; + +int main() { + error{}; +} diff --git a/gcc/testsuite/g++.dg/modules/debug-3_a.C b/gcc/testsuite/g++.dg/modules/debug-3_a.C new file mode 100644 index 00000000000..9e33d8260fd --- /dev/null +++ b/gcc/testsuite/g++.dg/modules/debug-3_a.C @@ -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; +}; diff --git a/gcc/testsuite/g++.dg/modules/debug-3_b.C b/gcc/testsuite/g++.dg/modules/debug-3_b.C new file mode 100644 index 00000000000..03c78b71b5d --- /dev/null +++ b/gcc/testsuite/g++.dg/modules/debug-3_b.C @@ -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{}; +} diff --git a/gcc/tree-core.h b/gcc/tree-core.h index 8a89462bd7e..1ca4d4c07bd 100644 --- a/gcc/tree-core.h +++ b/gcc/tree-core.h @@ -1814,8 +1814,7 @@ struct GTY(()) tree_decl_common { In FIELD_DECL, this is DECL_FIELD_ABI_IGNORED. */ unsigned decl_flag_0 : 1; /* In FIELD_DECL, this is DECL_BIT_FIELD - In VAR_DECL and FUNCTION_DECL, this is DECL_EXTERNAL. - In TYPE_DECL, this is TYPE_DECL_SUPPRESS_DEBUG. */ + In VAR_DECL, FUNCTION_DECL, and TYPE_DECL, this is DECL_EXTERNAL. */ unsigned decl_flag_1 : 1; /* In FIELD_DECL, this is DECL_NONADDRESSABLE_P In VAR_DECL, PARM_DECL and RESULT_DECL, this is @@ -1845,7 +1844,8 @@ struct GTY(()) tree_decl_common { TYPE_WARN_IF_NOT_ALIGN. */ unsigned int warn_if_not_align : 6; - /* In FIELD_DECL, this is DECL_NOT_FLEXARRAY. */ + /* In FIELD_DECL, this is DECL_NOT_FLEXARRAY. + In TYPE_DECL, this is TYPE_DECL_SUPPRESS_DEBUG. */ unsigned int decl_not_flexarray : 1; /* 5 bits unused. */ diff --git a/gcc/tree.h b/gcc/tree.h index 972a067a1f7..8bdc471ad4e 100644 --- a/gcc/tree.h +++ b/gcc/tree.h @@ -2848,9 +2848,9 @@ extern tree vector_element_bits_tree (const_tree); #define DECL_LANG_SPECIFIC(NODE) \ (DECL_COMMON_CHECK (NODE)->decl_common.lang_specific) -/* In a VAR_DECL or FUNCTION_DECL, nonzero means external reference: - do not allocate storage, and refer to a definition elsewhere. Note that - this does not necessarily imply the entity represented by NODE +/* In a VAR_DECL, FUNCTION_DECL, or TYPE_DECL, nonzero means external + reference: do not allocate storage, and refer to a definition elsewhere. + Note that this does not necessarily imply the entity represented by NODE has no program source-level definition in this translation unit. For example, for a FUNCTION_DECL, DECL_SAVED_TREE may be non-NULL and DECL_EXTERNAL may be true simultaneously; that can be the case for @@ -3553,7 +3553,7 @@ extern vec<tree, va_gc> **decl_debug_args_insert (tree); into stabs. Instead it will generate cross reference ('x') of names. This uses the same flag as DECL_EXTERNAL. */ #define TYPE_DECL_SUPPRESS_DEBUG(NODE) \ - (TYPE_DECL_CHECK (NODE)->decl_common.decl_flag_1) + (TYPE_DECL_CHECK (NODE)->decl_common.decl_not_flexarray) /* Getter of the imported declaration associated to the IMPORTED_DECL node. */