| Message ID | 20240226205230.2403133-1-ppalka@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | c++/modules: complete_vars ICE with non-exported constexpr var | expand |
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 --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 { };
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