| Message ID | 20240305205605.447458-1-polacek@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | c++: ICE with noexcept and local specialization [PR114114] | expand |
On 3/5/24 15:56, Marek Polacek wrote: > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk? OK. > -- >8 -- > Here we ICE because we call register_local_specialization while > local_specializations is null, so > > local_specializations->put (); > > crashes on null this. It's null since maybe_instantiate_noexcept calls > push_to_top_level which creates a new scope. Normally, I would have > guessed that we need a new local_specialization_stack. But here we're > dealing with an operand of a noexcept, which is an unevaluated operand, > and those aren't registered in the hash map. maybe_instantiate_noexcept > wasn't signalling that it's substituting an unevaluated operand though. > > PR c++/114114 > > gcc/cp/ChangeLog: > > * pt.cc (maybe_instantiate_noexcept): Save/restore > cp_unevaluated_operand, c_inhibit_evaluation_warnings, and > cp_noexcept_operand around the tsubst_expr call. > > gcc/testsuite/ChangeLog: > > * g++.dg/cpp0x/noexcept84.C: New test. > --- > gcc/cp/pt.cc | 6 +++++ > gcc/testsuite/g++.dg/cpp0x/noexcept84.C | 32 +++++++++++++++++++++++++ > 2 files changed, 38 insertions(+) > create mode 100644 gcc/testsuite/g++.dg/cpp0x/noexcept84.C > > diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc > index c4bc54a8fdb..11f7d33c766 100644 > --- a/gcc/cp/pt.cc > +++ b/gcc/cp/pt.cc > @@ -26869,10 +26869,16 @@ maybe_instantiate_noexcept (tree fn, tsubst_flags_t complain) > if (orig_fn) > ++processing_template_decl; > > + ++cp_unevaluated_operand; > + ++c_inhibit_evaluation_warnings; > + ++cp_noexcept_operand; > /* Do deferred instantiation of the noexcept-specifier. */ > noex = tsubst_expr (DEFERRED_NOEXCEPT_PATTERN (noex), > DEFERRED_NOEXCEPT_ARGS (noex), > tf_warning_or_error, fn); > + --cp_unevaluated_operand; > + --c_inhibit_evaluation_warnings; > + --cp_noexcept_operand; > > /* Build up the noexcept-specification. */ > spec = build_noexcept_spec (noex, tf_warning_or_error); > diff --git a/gcc/testsuite/g++.dg/cpp0x/noexcept84.C b/gcc/testsuite/g++.dg/cpp0x/noexcept84.C > new file mode 100644 > index 00000000000..06f33264f77 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/cpp0x/noexcept84.C > @@ -0,0 +1,32 @@ > +// PR c++/114114 > +// { dg-do compile { target c++11 } } > + > +template<bool B> > +constexpr void > +test () > +{ > + constexpr bool is_yes = B; > + struct S { > + constexpr S() noexcept(is_yes) { } > + }; > + S s; > +} > + > +constexpr bool foo() { return true; } > + > +template<typename T> > +constexpr void > +test2 () > +{ > + constexpr T (*pfn)() = &foo; > + struct S { > + constexpr S() noexcept(pfn()) { } > + }; > + S s; > +} > + > +int main() > +{ > + test<true>(); > + test2<bool>(); > +} > > base-commit: 8776468d9e57ace5f832c1368243a6dbce9984d5
On Tue, 5 Mar 2024, Marek Polacek wrote: > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk? > > -- >8 -- > Here we ICE because we call register_local_specialization while > local_specializations is null, so > > local_specializations->put (); > > crashes on null this. It's null since maybe_instantiate_noexcept calls > push_to_top_level which creates a new scope. Normally, I would have > guessed that we need a new local_specialization_stack. But here we're > dealing with an operand of a noexcept, which is an unevaluated operand, > and those aren't registered in the hash map. maybe_instantiate_noexcept > wasn't signalling that it's substituting an unevaluated operand though. It thought it was noexcept-exprs rather than noexcept-specs that are unevaluated contexts? > > PR c++/114114 > > gcc/cp/ChangeLog: > > * pt.cc (maybe_instantiate_noexcept): Save/restore > cp_unevaluated_operand, c_inhibit_evaluation_warnings, and > cp_noexcept_operand around the tsubst_expr call. > > gcc/testsuite/ChangeLog: > > * g++.dg/cpp0x/noexcept84.C: New test. > --- > gcc/cp/pt.cc | 6 +++++ > gcc/testsuite/g++.dg/cpp0x/noexcept84.C | 32 +++++++++++++++++++++++++ > 2 files changed, 38 insertions(+) > create mode 100644 gcc/testsuite/g++.dg/cpp0x/noexcept84.C > > diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc > index c4bc54a8fdb..11f7d33c766 100644 > --- a/gcc/cp/pt.cc > +++ b/gcc/cp/pt.cc > @@ -26869,10 +26869,16 @@ maybe_instantiate_noexcept (tree fn, tsubst_flags_t complain) > if (orig_fn) > ++processing_template_decl; > > + ++cp_unevaluated_operand; > + ++c_inhibit_evaluation_warnings; > + ++cp_noexcept_operand; > /* Do deferred instantiation of the noexcept-specifier. */ > noex = tsubst_expr (DEFERRED_NOEXCEPT_PATTERN (noex), > DEFERRED_NOEXCEPT_ARGS (noex), > tf_warning_or_error, fn); > + --cp_unevaluated_operand; > + --c_inhibit_evaluation_warnings; > + --cp_noexcept_operand; > > /* Build up the noexcept-specification. */ > spec = build_noexcept_spec (noex, tf_warning_or_error); > diff --git a/gcc/testsuite/g++.dg/cpp0x/noexcept84.C b/gcc/testsuite/g++.dg/cpp0x/noexcept84.C > new file mode 100644 > index 00000000000..06f33264f77 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/cpp0x/noexcept84.C > @@ -0,0 +1,32 @@ > +// PR c++/114114 > +// { dg-do compile { target c++11 } } > + > +template<bool B> > +constexpr void > +test () > +{ > + constexpr bool is_yes = B; > + struct S { > + constexpr S() noexcept(is_yes) { } > + }; > + S s; > +} > + > +constexpr bool foo() { return true; } > + > +template<typename T> > +constexpr void > +test2 () > +{ > + constexpr T (*pfn)() = &foo; > + struct S { > + constexpr S() noexcept(pfn()) { } > + }; > + S s; > +} > + > +int main() > +{ > + test<true>(); > + test2<bool>(); > +} > > base-commit: 8776468d9e57ace5f832c1368243a6dbce9984d5 > -- > 2.44.0 > >
On Fri, Mar 15, 2024 at 10:35:07AM -0400, Patrick Palka wrote: > On Tue, 5 Mar 2024, Marek Polacek wrote: > > > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk? > > > > -- >8 -- > > Here we ICE because we call register_local_specialization while > > local_specializations is null, so > > > > local_specializations->put (); > > > > crashes on null this. It's null since maybe_instantiate_noexcept calls > > push_to_top_level which creates a new scope. Normally, I would have > > guessed that we need a new local_specialization_stack. But here we're > > dealing with an operand of a noexcept, which is an unevaluated operand, > > and those aren't registered in the hash map. maybe_instantiate_noexcept > > wasn't signalling that it's substituting an unevaluated operand though. > > It thought it was noexcept-exprs rather than noexcept-specs that are > unevaluated contexts? Yes, sigh. It would have to be noexcept(noexcept(x)). I was looking at cp_parser_unary_expression/RID_NOEXCEPT but that's a noexcept-expr. So what can we do here, set a new local_specialization_stack? That wasn't that straightforward when I tried. Or maybe just --- a/gcc/cp/pt.cc +++ b/gcc/cp/pt.cc @@ -15649,7 +15649,7 @@ tsubst_decl (tree t, tree args, tsubst_flags_t complain, { if (DECL_LANG_SPECIFIC (r)) DECL_TEMPLATE_INFO (r) = NULL_TREE; - if (!cp_unevaluated_operand) + if (!cp_unevaluated_operand && local_specializations) register_local_specialization (r, t); } ?
On Fri, 15 Mar 2024, Marek Polacek wrote: > On Fri, Mar 15, 2024 at 10:35:07AM -0400, Patrick Palka wrote: > > On Tue, 5 Mar 2024, Marek Polacek wrote: > > > > > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk? > > > > > > -- >8 -- > > > Here we ICE because we call register_local_specialization while > > > local_specializations is null, so > > > > > > local_specializations->put (); > > > > > > crashes on null this. It's null since maybe_instantiate_noexcept calls > > > push_to_top_level which creates a new scope. Normally, I would have > > > guessed that we need a new local_specialization_stack. But here we're > > > dealing with an operand of a noexcept, which is an unevaluated operand, > > > and those aren't registered in the hash map. maybe_instantiate_noexcept > > > wasn't signalling that it's substituting an unevaluated operand though. > > > > It thought it was noexcept-exprs rather than noexcept-specs that are > > unevaluated contexts? > > Yes, sigh. It would have to be noexcept(noexcept(x)). I was looking at > cp_parser_unary_expression/RID_NOEXCEPT but that's a noexcept-expr. So > what can we do here, set a new local_specialization_stack? That wasn't > that straightforward when I tried. Or maybe just Maybe we can avoid doing push_to_top_level (which clears local_specializations) from maybe_instantiate_noexcept if current_function_decl == fn? Relatedly I wonder if we can avoid calling regenerate_decl_from_template for local class member functions since they can't be redeclared? > > --- a/gcc/cp/pt.cc > +++ b/gcc/cp/pt.cc > @@ -15649,7 +15649,7 @@ tsubst_decl (tree t, tree args, tsubst_flags_t complain, > { > if (DECL_LANG_SPECIFIC (r)) > DECL_TEMPLATE_INFO (r) = NULL_TREE; > - if (!cp_unevaluated_operand) > + if (!cp_unevaluated_operand && local_specializations) > register_local_specialization (r, t); > } > > ? > >
On Fri, Mar 15, 2024 at 12:12:49PM -0400, Patrick Palka wrote: > On Fri, 15 Mar 2024, Marek Polacek wrote: > > > On Fri, Mar 15, 2024 at 10:35:07AM -0400, Patrick Palka wrote: > > > On Tue, 5 Mar 2024, Marek Polacek wrote: > > > > > > > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk? > > > > > > > > -- >8 -- > > > > Here we ICE because we call register_local_specialization while > > > > local_specializations is null, so > > > > > > > > local_specializations->put (); > > > > > > > > crashes on null this. It's null since maybe_instantiate_noexcept calls > > > > push_to_top_level which creates a new scope. Normally, I would have > > > > guessed that we need a new local_specialization_stack. But here we're > > > > dealing with an operand of a noexcept, which is an unevaluated operand, > > > > and those aren't registered in the hash map. maybe_instantiate_noexcept > > > > wasn't signalling that it's substituting an unevaluated operand though. > > > > > > It thought it was noexcept-exprs rather than noexcept-specs that are > > > unevaluated contexts? > > > > Yes, sigh. It would have to be noexcept(noexcept(x)). I was looking at > > cp_parser_unary_expression/RID_NOEXCEPT but that's a noexcept-expr. So > > what can we do here, set a new local_specialization_stack? That wasn't > > that straightforward when I tried. Or maybe just > > Maybe we can avoid doing push_to_top_level (which clears > local_specializations) from maybe_instantiate_noexcept if > current_function_decl == fn? Thanks, I agree that not doing push_to_top_level in the first place is a better fix. I just sent a patch that does that. > Relatedly I wonder if we can avoid calling regenerate_decl_from_template > for local class member functions since they can't be redeclared? Good point. I've tried the below, but that breaks a lot of contracts tests. I have not pursued it further than that. diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc index a7ba8b5af92..5352453a5d3 100644 --- a/gcc/cp/pt.cc +++ b/gcc/cp/pt.cc @@ -26623,6 +26623,12 @@ regenerate_decl_from_template (tree decl, tree tmpl, tree args) if (DECL_UNIQUE_FRIEND_P (decl)) goto done; + /* [class.mem.general]/5 says that a member shall not be declared twice + in the member-specification (unless it's a nested class or member class + template or an enumeration). */ + if (DECL_CLASS_SCOPE_P (decl)) + goto done; + /* Use the source location of the definition. */ DECL_SOURCE_LOCATION (decl) = DECL_SOURCE_LOCATION (tmpl); Marek
diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc index c4bc54a8fdb..11f7d33c766 100644 --- a/gcc/cp/pt.cc +++ b/gcc/cp/pt.cc @@ -26869,10 +26869,16 @@ maybe_instantiate_noexcept (tree fn, tsubst_flags_t complain) if (orig_fn) ++processing_template_decl; + ++cp_unevaluated_operand; + ++c_inhibit_evaluation_warnings; + ++cp_noexcept_operand; /* Do deferred instantiation of the noexcept-specifier. */ noex = tsubst_expr (DEFERRED_NOEXCEPT_PATTERN (noex), DEFERRED_NOEXCEPT_ARGS (noex), tf_warning_or_error, fn); + --cp_unevaluated_operand; + --c_inhibit_evaluation_warnings; + --cp_noexcept_operand; /* Build up the noexcept-specification. */ spec = build_noexcept_spec (noex, tf_warning_or_error); diff --git a/gcc/testsuite/g++.dg/cpp0x/noexcept84.C b/gcc/testsuite/g++.dg/cpp0x/noexcept84.C new file mode 100644 index 00000000000..06f33264f77 --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp0x/noexcept84.C @@ -0,0 +1,32 @@ +// PR c++/114114 +// { dg-do compile { target c++11 } } + +template<bool B> +constexpr void +test () +{ + constexpr bool is_yes = B; + struct S { + constexpr S() noexcept(is_yes) { } + }; + S s; +} + +constexpr bool foo() { return true; } + +template<typename T> +constexpr void +test2 () +{ + constexpr T (*pfn)() = &foo; + struct S { + constexpr S() noexcept(pfn()) { } + }; + S s; +} + +int main() +{ + test<true>(); + test2<bool>(); +}