[PATCH] c++/modules: Prevent treating suppressed debug info as extern template [PR112820]

[PATCH] c++/modules: Prevent treating suppressed debug info as extern template [PR112820]

[Nathaniel Shead]

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.

This patch constrains the checks for DECL_EXTERNAL for this to only
consider template instantiations, thus avoiding the issue.

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

gcc/cp/ChangeLog:

	* module.cc (trees_in::read_class_def): Only set interface for
	template instantiations.

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/module.cc                         | 4 +++-
 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 +++++++++
 6 files changed, 46 insertions(+), 1 deletion(-)
 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/module.cc b/gcc/cp/module.cc
index 33fcf396875..257f39421d0 100644
--- a/gcc/cp/module.cc
+++ b/gcc/cp/module.cc
@@ -12041,7 +12041,9 @@ 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))
+      if (DECL_EXTERNAL (defn)
+	  && TYPE_LANG_SPECIFIC (type)
+	  && CLASSTYPE_TEMPLATE_INSTANTIATION (type))
 	{
 	  /* We don't deal with not-really-extern, because, for a
 	     module you want the import to be the interface, and for a
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{};
+}
-- 
2.42.0

Re: [PATCH] c++/modules: Prevent treating suppressed debug info as extern template [PR112820]

[Nathaniel Shead]

Ping for https://gcc.gnu.org/pipermail/gcc-patches/2023-December/639082.html.

On Sun, Dec 03, 2023 at 11:46:36PM +1100, 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.
> 
> This patch constrains the checks for DECL_EXTERNAL for this to only
> consider template instantiations, thus avoiding the issue.
> 
> 	PR c++/102607
> 	PR c++/112820
> 
> gcc/cp/ChangeLog:
> 
> 	* module.cc (trees_in::read_class_def): Only set interface for
> 	template instantiations.
> 
> 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/module.cc                         | 4 +++-
>  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 +++++++++
>  6 files changed, 46 insertions(+), 1 deletion(-)
>  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/module.cc b/gcc/cp/module.cc
> index 33fcf396875..257f39421d0 100644
> --- a/gcc/cp/module.cc
> +++ b/gcc/cp/module.cc
> @@ -12041,7 +12041,9 @@ 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))
> +      if (DECL_EXTERNAL (defn)
> +	  && TYPE_LANG_SPECIFIC (type)
> +	  && CLASSTYPE_TEMPLATE_INSTANTIATION (type))
>  	{
>  	  /* We don't deal with not-really-extern, because, for a
>  	     module you want the import to be the interface, and for a
> 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{};
> +}
> -- 
> 2.42.0
> 

Re: [PATCH] c++/modules: Prevent treating suppressed debug info as extern template [PR112820]

[Patrick Palka]

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.

> 
> This patch constrains the checks for DECL_EXTERNAL for this to only
> consider template instantiations, thus avoiding the issue.
> 
> 	PR c++/102607
> 	PR c++/112820
> 
> gcc/cp/ChangeLog:
> 
> 	* module.cc (trees_in::read_class_def): Only set interface for
> 	template instantiations.
> 
> 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/module.cc                         | 4 +++-
>  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 +++++++++
>  6 files changed, 46 insertions(+), 1 deletion(-)
>  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/module.cc b/gcc/cp/module.cc
> index 33fcf396875..257f39421d0 100644
> --- a/gcc/cp/module.cc
> +++ b/gcc/cp/module.cc
> @@ -12041,7 +12041,9 @@ 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))
> +      if (DECL_EXTERNAL (defn)
> +	  && TYPE_LANG_SPECIFIC (type)
> +	  && CLASSTYPE_TEMPLATE_INSTANTIATION (type))
>  	{
>  	  /* We don't deal with not-really-extern, because, for a
>  	     module you want the import to be the interface, and for a
> 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{};
> +}
> -- 
> 2.42.0
> 
> 

[PATCH v2] c++/modules: Differentiate extern templates and TYPE_DECL_SUPPRESS_DEBUG [PR112820]

[Nathaniel Shead]

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.  */
 #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

Re: [PATCH v2] c++/modules: Differentiate extern templates and TYPE_DECL_SUPPRESS_DEBUG [PR112820]

[Richard Biener]

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
>

Re: [PATCH v2] c++/modules: Differentiate extern templates and TYPE_DECL_SUPPRESS_DEBUG [PR112820]

[Patrick Palka]

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
> 
> 

Re: [PATCH v2] c++/modules: Differentiate extern templates and TYPE_DECL_SUPPRESS_DEBUG [PR112820]

[Jason Merrill]

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

Re: [PATCH v2] c++/modules: Differentiate extern templates and TYPE_DECL_SUPPRESS_DEBUG [PR112820]

[Nathaniel Shead]

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.

[PATCH v3] c++/modules: Fix handling of extern templates in modules [PR112820]

[Nathaniel Shead]

On Mon, Jan 15, 2024 at 06:10:55PM -0500, Jason Merrill wrote:
> 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
> 

Here's another approach for this patch. This still only fixes the
specific issues in the PR, I think vtable handling etc. should wait till
stage 1 because it involves a lot of messing around in decl2.cc.

As mentioned in the commit message, after thinking more about it I don't
think we (in general) want to propagate CLASSTYPE_INTERFACE_ONLY, even
for declarations in the GMF. This makes sense to me because typically it
can only be accurately determined at the end of the TU, which we haven't
yet arrived at after importing. For instance, for a polymorphic class in
the GMF without a key method, that we import from a module and then
proceed to define the key method later on in this TU.

Bootstrapped and partially regtested on x86_64-pc-linux-gnu (so far only
modules.exp): OK for trunk if full regtesting passes?

-- >8 --

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 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.

	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 <nathanieloshead@gmail.com>
---
 gcc/cp/module.cc                         | 36 +++++++++++++-----------
 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 ++++++
 6 files changed, 63 insertions(+), 16 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/module.cc b/gcc/cp/module.cc
index 350ad15dc62..efc1d532a6e 100644
--- a/gcc/cp/module.cc
+++ b/gcc/cp/module.cc
@@ -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);
@@ -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);
@@ -8246,6 +8243,22 @@ 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?
+	 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)
@@ -12070,15 +12083,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
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{};
+}
-- 
2.43.0

Re: [PATCH v3] c++/modules: Fix handling of extern templates in modules [PR112820]

[Jason Merrill]

On 1/17/24 01:33, Nathaniel Shead wrote:
> On Mon, Jan 15, 2024 at 06:10:55PM -0500, Jason Merrill wrote:
>> 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
>>
> 
> Here's another approach for this patch. This still only fixes the
> specific issues in the PR, I think vtable handling etc. should wait till
> stage 1 because it involves a lot of messing around in decl2.cc.
> 
> As mentioned in the commit message, after thinking more about it I don't
> think we (in general) want to propagate CLASSTYPE_INTERFACE_ONLY, even
> for declarations in the GMF. This makes sense to me because typically it
> can only be accurately determined at the end of the TU, which we haven't
> yet arrived at after importing. For instance, for a polymorphic class in
> the GMF without a key method, that we import from a module and then
> proceed to define the key method later on in this TU.

That sounds right for a module implementation unit or the GMF.

> Bootstrapped and partially regtested on x86_64-pc-linux-gnu (so far only
> modules.exp): OK for trunk if full regtesting passes?

Please add a reference to ABI issue 170 
(https://github.com/itanium-cxx-abi/cxx-abi/issues/170).  OK with that 
change if Nathan doesn't have any further comments this week.

> -- >8 --
> 
> 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 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.
> 
> 	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 <nathanieloshead@gmail.com>
> ---
>   gcc/cp/module.cc                         | 36 +++++++++++++-----------
>   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 ++++++
>   6 files changed, 63 insertions(+), 16 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/module.cc b/gcc/cp/module.cc
> index 350ad15dc62..efc1d532a6e 100644
> --- a/gcc/cp/module.cc
> +++ b/gcc/cp/module.cc
> @@ -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);
> @@ -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);
> @@ -8246,6 +8243,22 @@ 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?
> +	 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)
> @@ -12070,15 +12083,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
> 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{};
> +}

Re: [PATCH v3] c++/modules: Fix handling of extern templates in modules [PR112820]

[Nathaniel Shead]

On Wed, Jan 17, 2024 at 10:51:16AM -0500, Jason Merrill wrote:
> On 1/17/24 01:33, Nathaniel Shead wrote:
> > On Mon, Jan 15, 2024 at 06:10:55PM -0500, Jason Merrill wrote:
> > > 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
> > > 
> > 
> > Here's another approach for this patch. This still only fixes the
> > specific issues in the PR, I think vtable handling etc. should wait till
> > stage 1 because it involves a lot of messing around in decl2.cc.
> > 
> > As mentioned in the commit message, after thinking more about it I don't
> > think we (in general) want to propagate CLASSTYPE_INTERFACE_ONLY, even
> > for declarations in the GMF. This makes sense to me because typically it
> > can only be accurately determined at the end of the TU, which we haven't
> > yet arrived at after importing. For instance, for a polymorphic class in
> > the GMF without a key method, that we import from a module and then
> > proceed to define the key method later on in this TU.
> 
> That sounds right for a module implementation unit or the GMF.
> 
> > Bootstrapped and partially regtested on x86_64-pc-linux-gnu (so far only
> > modules.exp): OK for trunk if full regtesting passes?
> 
> Please add a reference to ABI issue 170
> (https://github.com/itanium-cxx-abi/cxx-abi/issues/170).  OK with that
> change if Nathan doesn't have any further comments this week.
> 

Thanks. Here's what I'll push tomorrow unless I hear otherwise.

-- >8 --

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
https://github.com/itanium-cxx-abi/cxx-abi/issues/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 <nathanieloshead@gmail.com>
---
 gcc/cp/module.cc                         | 37 ++++++++++++++----------
 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 ++++++
 6 files changed, 64 insertions(+), 16 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/module.cc b/gcc/cp/module.cc
index 8db662c0267..70785493561 100644
--- a/gcc/cp/module.cc
+++ b/gcc/cp/module.cc
@@ -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);
@@ -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);
@@ -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)
@@ -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
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{};
+}
-- 
2.43.0