diff mbox series

c++/modules: complete_vars ICE with non-exported constexpr var

Message ID 20240226205230.2403133-1-ppalka@redhat.com
State New
Headers
Series c++/modules: complete_vars ICE with non-exported constexpr var |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gcc_build--master-aarch64 success Testing passed
linaro-tcwg-bot/tcwg_gcc_build--master-arm success Testing passed
linaro-tcwg-bot/tcwg_gcc_check--master-arm success Testing passed
linaro-tcwg-bot/tcwg_gcc_check--master-aarch64 success Testing passed

Commit Message

Patrick Palka Feb. 26, 2024, 8:52 p.m. UTC 
  Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for
trunk?

-- >8 --

Here during stream-in of the non-exported constexpr var 'a' we call
maybe_register_incomplete_var, which ends up taking the second branch
and pushing {a, NULL_TREE} onto incomplete_vars.  We later ICE from
complete_vars due to this NULL_TREE class context.

Judging by the two commits that introduced/modified this part of
maybe_register_incomplete_var, r196852 and r214333, ISTM this branch
is really only concerned with constexpr static data members (whose
initializer may contain a pointer-to-member for a currently open class).
So this patch restricts this branch accordingly so it's not inadvertently
taken during stream-in.

gcc/cp/ChangeLog:

	* decl.cc (maybe_register_incomplete_var): Restrict second
	branch to static data members from a currently open class.

gcc/testsuite/ChangeLog:

	* g++.dg/modules/cexpr-4_a.C: New test.
	* g++.dg/modules/cexpr-4_b.C: New test.
---
 gcc/cp/decl.cc                           |  2 ++
 gcc/testsuite/g++.dg/modules/cexpr-4_a.C | 12 ++++++++++++
 gcc/testsuite/g++.dg/modules/cexpr-4_b.C |  6 ++++++
 3 files changed, 20 insertions(+)
 create mode 100644 gcc/testsuite/g++.dg/modules/cexpr-4_a.C
 create mode 100644 gcc/testsuite/g++.dg/modules/cexpr-4_b.C

Comments

Jason Merrill March 1, 2024, 7:15 p.m. UTC | #1 
On 2/26/24 15:52, Patrick Palka wrote:
> Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for
> trunk?
> 
> -- >8 --
> 
> Here during stream-in of the non-exported constexpr var 'a' we call
> maybe_register_incomplete_var, which ends up taking the second branch
> and pushing {a, NULL_TREE} onto incomplete_vars.  We later ICE from
> complete_vars due to this NULL_TREE class context.
> 
> Judging by the two commits that introduced/modified this part of
> maybe_register_incomplete_var, r196852 and r214333, ISTM this branch
> is really only concerned with constexpr static data members (whose
> initializer may contain a pointer-to-member for a currently open class).
> So this patch restricts this branch accordingly so it's not inadvertently
> taken during stream-in.
> 
> gcc/cp/ChangeLog:
> 
> 	* decl.cc (maybe_register_incomplete_var): Restrict second
> 	branch to static data members from a currently open class.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* g++.dg/modules/cexpr-4_a.C: New test.
> 	* g++.dg/modules/cexpr-4_b.C: New test.
> ---
>   gcc/cp/decl.cc                           |  2 ++
>   gcc/testsuite/g++.dg/modules/cexpr-4_a.C | 12 ++++++++++++
>   gcc/testsuite/g++.dg/modules/cexpr-4_b.C |  6 ++++++
>   3 files changed, 20 insertions(+)
>   create mode 100644 gcc/testsuite/g++.dg/modules/cexpr-4_a.C
>   create mode 100644 gcc/testsuite/g++.dg/modules/cexpr-4_b.C
> 
> diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc
> index e47f694e4e5..82b5bc83927 100644
> --- a/gcc/cp/decl.cc
> +++ b/gcc/cp/decl.cc
> @@ -18976,6 +18976,8 @@ maybe_register_incomplete_var (tree var)
>   	  vec_safe_push (incomplete_vars, iv);
>   	}
>         else if (!(DECL_LANG_SPECIFIC (var) && DECL_TEMPLATE_INFO (var))
> +	       && DECL_CLASS_SCOPE_P (var)
> +	       && currently_open_class (DECL_CONTEXT (var))

I think TYPE_BEING_DEFINED (as used a few lines up) would be a bit 
better than currently_open_class?  OK with that change.

Jason
diff mbox series

Patch

diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc
index e47f694e4e5..82b5bc83927 100644
--- a/gcc/cp/decl.cc
+++ b/gcc/cp/decl.cc
@@ -18976,6 +18976,8 @@  maybe_register_incomplete_var (tree var)
 	  vec_safe_push (incomplete_vars, iv);
 	}
       else if (!(DECL_LANG_SPECIFIC (var) && DECL_TEMPLATE_INFO (var))
+	       && DECL_CLASS_SCOPE_P (var)
+	       && currently_open_class (DECL_CONTEXT (var))
 	       && decl_constant_var_p (var)
 	       && (TYPE_PTRMEM_P (inner_type) || CLASS_TYPE_P (inner_type)))
 	{
diff --git a/gcc/testsuite/g++.dg/modules/cexpr-4_a.C b/gcc/testsuite/g++.dg/modules/cexpr-4_a.C
new file mode 100644
index 00000000000..cec2e351898
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/cexpr-4_a.C
@@ -0,0 +1,12 @@ 
+// { dg-additional-options "-fmodules-ts" }
+export module Cexpr4;
+// { dg-module-cmi "Cexpr4" }
+
+struct A { int v = 42; };
+
+constexpr A a;
+
+export
+inline int f() {
+  return a.v;
+}
diff --git a/gcc/testsuite/g++.dg/modules/cexpr-4_b.C b/gcc/testsuite/g++.dg/modules/cexpr-4_b.C
new file mode 100644
index 00000000000..6fbe652bf22
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/cexpr-4_b.C
@@ -0,0 +1,6 @@ 
+// { dg-additional-options "-fmodules-ts" }
+import Cexpr4;
+
+int w = f();
+
+struct A { };