* [PATCH] c++: ICE with noexcept in class in member function [PR96623]
@ 2021-01-21 22:45 Marek Polacek
2021-01-22 2:47 ` Jason Merrill
0 siblings, 1 reply; 5+ messages in thread
From: Marek Polacek @ 2021-01-21 22:45 UTC (permalink / raw)
To: Jason Merrill, GCC Patches
I discovered very strange code in inject_parm_decls:
if (args && is_this_parameter (args))
{
gcc_checking_assert (current_class_ptr == NULL_TREE);
current_class_ptr = NULL_TREE;
We are tripping up on the assert because when we call inject_parm_decls,
current_class_ptr is set to 'A'. It was set by inject_this_parameter
after we've parsed the parameter-declaration-clause of the member
function foo. It seems correct to set ccp/ccr to A::B when we're
late parsing the noexcept-specifiers of bar* functions in B, so that
this-> does the right thing. Since inject_parm_decls can mess with
ccp/ccr, I think best if we properly restore it after the late parsing
of noexcept-specifiers.
It should also work to clear ccp before calling inject_parm_decls, and
removing the assignment following the assert, should the assert stay.
Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
gcc/cp/ChangeLog:
PR c++/96623
* parser.c (inject_parm_decls): Remove a gcc_checking_assert.
(cp_parser_class_specifier_1): Restore current_class_{ptr,ref}
after late parsing of noexcept-specifiers.
gcc/testsuite/ChangeLog:
PR c++/96623
* g++.dg/cpp0x/noexcept64.C: New test.
---
gcc/cp/parser.c | 8 ++++----
gcc/testsuite/g++.dg/cpp0x/noexcept64.C | 24 ++++++++++++++++++++++++
2 files changed, 28 insertions(+), 4 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/cpp0x/noexcept64.C
diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index 4b2bca3fd11..8e86e92e273 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -24709,7 +24709,6 @@ inject_parm_decls (tree decl)
if (args && is_this_parameter (args))
{
- gcc_checking_assert (current_class_ptr == NULL_TREE);
current_class_ptr = NULL_TREE;
current_class_ref = cp_build_fold_indirect_ref (args);
current_class_ptr = args;
@@ -24967,7 +24966,6 @@ cp_parser_class_specifier_1 (cp_parser* parser)
tree pushed_scope = NULL_TREE;
unsigned ix;
cp_default_arg_entry *e;
- tree save_ccp, save_ccr;
if (!type_definition_ok_p || any_erroneous_template_args_p (type))
{
@@ -25012,6 +25010,8 @@ cp_parser_class_specifier_1 (cp_parser* parser)
/* If there are noexcept-specifiers that have not yet been processed,
take care of them now. Do this before processing NSDMIs as they
may depend on noexcept-specifiers already having been processed. */
+ tree save_ccp = current_class_ptr;
+ tree save_ccr = current_class_ref;
FOR_EACH_VEC_SAFE_ELT (unparsed_noexcepts, ix, decl)
{
tree ctx = DECL_CONTEXT (decl);
@@ -25063,10 +25063,10 @@ cp_parser_class_specifier_1 (cp_parser* parser)
maybe_end_member_template_processing ();
}
vec_safe_truncate (unparsed_noexcepts, 0);
+ current_class_ptr = save_ccp;
+ current_class_ref = save_ccr;
/* Now parse any NSDMIs. */
- save_ccp = current_class_ptr;
- save_ccr = current_class_ref;
FOR_EACH_VEC_SAFE_ELT (unparsed_nsdmis, ix, decl)
{
if (class_type != DECL_CONTEXT (decl))
diff --git a/gcc/testsuite/g++.dg/cpp0x/noexcept64.C b/gcc/testsuite/g++.dg/cpp0x/noexcept64.C
new file mode 100644
index 00000000000..8b7303cd8a1
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/noexcept64.C
@@ -0,0 +1,24 @@
+// PR c++/96623
+// { dg-do compile { target c++11 } }
+
+constexpr int x = 0;
+struct A {
+ int a1;
+ void foo (int p) {
+ int foovar;
+ struct B {
+ int b1;
+ void bar1 () noexcept(x);
+ void bar2 () noexcept(noexcept(this->b1));
+ void bar3 () noexcept(noexcept(this->b2));
+ void bar4 () noexcept(noexcept(a1));
+ void bar5 () noexcept(noexcept(a2));
+ void bar6 () noexcept(noexcept(b1));
+ void bar7 () noexcept(noexcept(b2));
+ void bar8 () noexcept(noexcept(foovar));
+ void bar9 () noexcept(noexcept(p));
+ int b2;
+ };
+ }
+ int a2;
+};
base-commit: f645da0e4ab9438dfd0c047c710c7ec6a7d6d8f3
--
2.29.2
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] c++: ICE with noexcept in class in member function [PR96623]
2021-01-21 22:45 [PATCH] c++: ICE with noexcept in class in member function [PR96623] Marek Polacek
@ 2021-01-22 2:47 ` Jason Merrill
2021-01-22 21:01 ` Marek Polacek
0 siblings, 1 reply; 5+ messages in thread
From: Jason Merrill @ 2021-01-22 2:47 UTC (permalink / raw)
To: Marek Polacek, GCC Patches
On 1/21/21 5:45 PM, Marek Polacek wrote:
> I discovered very strange code in inject_parm_decls:
>
> if (args && is_this_parameter (args))
> {
> gcc_checking_assert (current_class_ptr == NULL_TREE);
> current_class_ptr = NULL_TREE;
>
> We are tripping up on the assert because when we call inject_parm_decls,
> current_class_ptr is set to 'A'. It was set by inject_this_parameter
> after we've parsed the parameter-declaration-clause of the member
> function foo.
But then it should be restored (to null) by the ccp = save_ccp a few
lines later.
> It seems correct to set ccp/ccr to A::B when we're
> late parsing the noexcept-specifiers of bar* functions in B, so that
> this-> does the right thing.
Agreed.
> Since inject_parm_decls can mess with
> ccp/ccr, I think best if we properly restore it after the late parsing
> of noexcept-specifiers.
pop_injected_parms clears them, which is restoring them if we keep the
assert.
> It should also work to clear ccp before calling inject_parm_decls, and
> removing the assignment following the assert, should the assert stay.
But why is it non-null before parsing the unparsed_noexcepts?
Jason
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] c++: ICE with noexcept in class in member function [PR96623]
2021-01-22 2:47 ` Jason Merrill
@ 2021-01-22 21:01 ` Marek Polacek
2021-01-22 21:44 ` Jason Merrill
0 siblings, 1 reply; 5+ messages in thread
From: Marek Polacek @ 2021-01-22 21:01 UTC (permalink / raw)
To: Jason Merrill; +Cc: GCC Patches
On Thu, Jan 21, 2021 at 09:47:35PM -0500, Jason Merrill via Gcc-patches wrote:
> On 1/21/21 5:45 PM, Marek Polacek wrote:
> > I discovered very strange code in inject_parm_decls:
> >
> > if (args && is_this_parameter (args))
> > {
> > gcc_checking_assert (current_class_ptr == NULL_TREE);
> > current_class_ptr = NULL_TREE;
> >
> > We are tripping up on the assert because when we call inject_parm_decls,
> > current_class_ptr is set to 'A'. It was set by inject_this_parameter
> > after we've parsed the parameter-declaration-clause of the member
> > function foo.
>
> But then it should be restored (to null) by the ccp = save_ccp a few lines
> later.
Indeed. I glossed over that. :(
> > It seems correct to set ccp/ccr to A::B when we're
> > late parsing the noexcept-specifiers of bar* functions in B, so that
> > this-> does the right thing.
>
> Agreed.
>
> > Since inject_parm_decls can mess with
> > ccp/ccr, I think best if we properly restore it after the late parsing
> > of noexcept-specifiers.
>
> pop_injected_parms clears them, which is restoring them if we keep the
> assert.
>
> > It should also work to clear ccp before calling inject_parm_decls, and
> > removing the assignment following the assert, should the assert stay.
>
> But why is it non-null before parsing the unparsed_noexcepts?
Now that I've taken another look I see that ccp/ccr are being set
in start_preparsed_function (around line 16566), when we parse the
body of foo in cp_parser_late_parsing_for_member. Therefore it's
sort of expected that it's still set when we get to B in foo. And
I also continue to think that when we're late parsing NSDMIs, we
should use the original ccr/ccp, not the ones cleared by pop_i_p.
So, I don't have a better patch than the original one.
Marek
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] c++: ICE with noexcept in class in member function [PR96623]
2021-01-22 21:01 ` Marek Polacek
@ 2021-01-22 21:44 ` Jason Merrill
2021-01-22 22:03 ` Marek Polacek
0 siblings, 1 reply; 5+ messages in thread
From: Jason Merrill @ 2021-01-22 21:44 UTC (permalink / raw)
To: Marek Polacek; +Cc: GCC Patches
On 1/22/21 4:01 PM, Marek Polacek wrote:
> On Thu, Jan 21, 2021 at 09:47:35PM -0500, Jason Merrill via Gcc-patches wrote:
>> On 1/21/21 5:45 PM, Marek Polacek wrote:
>>> I discovered very strange code in inject_parm_decls:
>>>
>>> if (args && is_this_parameter (args))
>>> {
>>> gcc_checking_assert (current_class_ptr == NULL_TREE);
>>> current_class_ptr = NULL_TREE;
>>>
>>> We are tripping up on the assert because when we call inject_parm_decls,
>>> current_class_ptr is set to 'A'. It was set by inject_this_parameter
>>> after we've parsed the parameter-declaration-clause of the member
>>> function foo.
>>
>> But then it should be restored (to null) by the ccp = save_ccp a few lines
>> later.
>
> Indeed. I glossed over that. :(
>
>>> It seems correct to set ccp/ccr to A::B when we're
>>> late parsing the noexcept-specifiers of bar* functions in B, so that
>>> this-> does the right thing.
>>
>> Agreed.
>>
>>> Since inject_parm_decls can mess with
>>> ccp/ccr, I think best if we properly restore it after the late parsing
>>> of noexcept-specifiers.
>>
>> pop_injected_parms clears them, which is restoring them if we keep the
>> assert.
>>
>>> It should also work to clear ccp before calling inject_parm_decls, and
>>> removing the assignment following the assert, should the assert stay.
>>
>> But why is it non-null before parsing the unparsed_noexcepts?
>
> Now that I've taken another look I see that ccp/ccr are being set
> in start_preparsed_function (around line 16566), when we parse the
> body of foo in cp_parser_late_parsing_for_member. Therefore it's
> sort of expected that it's still set when we get to B in foo.
Ah! Yes, it's set because we're parsing a local class of A::foo. This
all makes more sense to me now. So:
> - gcc_checking_assert (current_class_ptr == NULL_TREE);
Let's keep the assert, and...
> + tree save_ccp = current_class_ptr;
> + tree save_ccr = current_class_ref;
...clear cc* after saving so the assert succeeds
> vec_safe_truncate (unparsed_noexcepts, 0);
> + current_class_ptr = save_ccp;
> + current_class_ref = save_ccr;
No point restoring the original values (for A::foo) here, it won't be
usable for any DMI in a local class of A.
OK with those changes.
Jason
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] c++: ICE with noexcept in class in member function [PR96623]
2021-01-22 21:44 ` Jason Merrill
@ 2021-01-22 22:03 ` Marek Polacek
0 siblings, 0 replies; 5+ messages in thread
From: Marek Polacek @ 2021-01-22 22:03 UTC (permalink / raw)
To: Jason Merrill; +Cc: GCC Patches
On Fri, Jan 22, 2021 at 04:44:42PM -0500, Jason Merrill wrote:
> On 1/22/21 4:01 PM, Marek Polacek wrote:
> > On Thu, Jan 21, 2021 at 09:47:35PM -0500, Jason Merrill via Gcc-patches wrote:
> > > On 1/21/21 5:45 PM, Marek Polacek wrote:
> > > > I discovered very strange code in inject_parm_decls:
> > > >
> > > > if (args && is_this_parameter (args))
> > > > {
> > > > gcc_checking_assert (current_class_ptr == NULL_TREE);
> > > > current_class_ptr = NULL_TREE;
> > > >
> > > > We are tripping up on the assert because when we call inject_parm_decls,
> > > > current_class_ptr is set to 'A'. It was set by inject_this_parameter
> > > > after we've parsed the parameter-declaration-clause of the member
> > > > function foo.
> > >
> > > But then it should be restored (to null) by the ccp = save_ccp a few lines
> > > later.
> >
> > Indeed. I glossed over that. :(
> > > > It seems correct to set ccp/ccr to A::B when we're
> > > > late parsing the noexcept-specifiers of bar* functions in B, so that
> > > > this-> does the right thing.
> > >
> > > Agreed.
> > >
> > > > Since inject_parm_decls can mess with
> > > > ccp/ccr, I think best if we properly restore it after the late parsing
> > > > of noexcept-specifiers.
> > >
> > > pop_injected_parms clears them, which is restoring them if we keep the
> > > assert.
> > >
> > > > It should also work to clear ccp before calling inject_parm_decls, and
> > > > removing the assignment following the assert, should the assert stay.
> > >
> > > But why is it non-null before parsing the unparsed_noexcepts?
> >
> > Now that I've taken another look I see that ccp/ccr are being set
> > in start_preparsed_function (around line 16566), when we parse the
> > body of foo in cp_parser_late_parsing_for_member. Therefore it's
> > sort of expected that it's still set when we get to B in foo.
>
> Ah! Yes, it's set because we're parsing a local class of A::foo. This all
> makes more sense to me now. So:
>
> > - gcc_checking_assert (current_class_ptr == NULL_TREE);
>
> Let's keep the assert, and...
>
> > + tree save_ccp = current_class_ptr;
> > + tree save_ccr = current_class_ref;
>
> ...clear cc* after saving so the assert succeeds
>
> > vec_safe_truncate (unparsed_noexcepts, 0);
> > + current_class_ptr = save_ccp;
> > + current_class_ref = save_ccr;
>
> No point restoring the original values (for A::foo) here, it won't be usable
> for any DMI in a local class of A.
>
> OK with those changes.
Thanks a lot. Here's what I'm going to check in after another round o
testing.
-- >8 --
I discovered very strange code in inject_parm_decls:
if (args && is_this_parameter (args))
{
gcc_checking_assert (current_class_ptr == NULL_TREE);
current_class_ptr = NULL_TREE;
We are tripping up on the assert because when we call inject_parm_decls,
current_class_ptr is set to 'A'. It was set by inject_this_parameter
after we've parsed the parameter-declaration-clause of the member
function foo. It seems correct to set ccp/ccr to A::B when we're
late parsing the noexcept-specifiers of bar* functions in B, so that
this-> does the right thing. Since inject_parm_decls doesn't expect
to see non-null ccp/ccr, reset it before calling inject_parm_decls.
gcc/cp/ChangeLog:
PR c++/96623
* parser.c (inject_parm_decls): Remove a redundant assignment.
(cp_parser_class_specifier_1): Clear current_class_{ptr,ref}
before calling inject_parm_decls.
gcc/testsuite/ChangeLog:
PR c++/96623
* g++.dg/cpp0x/noexcept64.C: New test.
---
gcc/cp/parser.c | 10 +++++-----
gcc/testsuite/g++.dg/cpp0x/noexcept64.C | 24 ++++++++++++++++++++++++
2 files changed, 29 insertions(+), 5 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/cpp0x/noexcept64.C
diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index 4b2bca3fd11..e0208d02bdc 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -24710,7 +24710,6 @@ inject_parm_decls (tree decl)
if (args && is_this_parameter (args))
{
gcc_checking_assert (current_class_ptr == NULL_TREE);
- current_class_ptr = NULL_TREE;
current_class_ref = cp_build_fold_indirect_ref (args);
current_class_ptr = args;
}
@@ -24967,7 +24966,6 @@ cp_parser_class_specifier_1 (cp_parser* parser)
tree pushed_scope = NULL_TREE;
unsigned ix;
cp_default_arg_entry *e;
- tree save_ccp, save_ccr;
if (!type_definition_ok_p || any_erroneous_template_args_p (type))
{
@@ -25012,6 +25010,8 @@ cp_parser_class_specifier_1 (cp_parser* parser)
/* If there are noexcept-specifiers that have not yet been processed,
take care of them now. Do this before processing NSDMIs as they
may depend on noexcept-specifiers already having been processed. */
+ tree save_ccp = current_class_ptr;
+ tree save_ccr = current_class_ref;
FOR_EACH_VEC_SAFE_ELT (unparsed_noexcepts, ix, decl)
{
tree ctx = DECL_CONTEXT (decl);
@@ -25029,7 +25029,9 @@ cp_parser_class_specifier_1 (cp_parser* parser)
/* Make sure that any template parameters are in scope. */
maybe_begin_member_template_processing (decl);
- /* Make sure that any member-function parameters are in scope. */
+ /* Make sure that any member-function parameters are in scope.
+ This function doesn't expect ccp to be set. */
+ current_class_ptr = current_class_ref = NULL_TREE;
inject_parm_decls (decl);
/* 'this' is not allowed in static member functions. */
@@ -25065,8 +25067,6 @@ cp_parser_class_specifier_1 (cp_parser* parser)
vec_safe_truncate (unparsed_noexcepts, 0);
/* Now parse any NSDMIs. */
- save_ccp = current_class_ptr;
- save_ccr = current_class_ref;
FOR_EACH_VEC_SAFE_ELT (unparsed_nsdmis, ix, decl)
{
if (class_type != DECL_CONTEXT (decl))
diff --git a/gcc/testsuite/g++.dg/cpp0x/noexcept64.C b/gcc/testsuite/g++.dg/cpp0x/noexcept64.C
new file mode 100644
index 00000000000..8b7303cd8a1
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/noexcept64.C
@@ -0,0 +1,24 @@
+// PR c++/96623
+// { dg-do compile { target c++11 } }
+
+constexpr int x = 0;
+struct A {
+ int a1;
+ void foo (int p) {
+ int foovar;
+ struct B {
+ int b1;
+ void bar1 () noexcept(x);
+ void bar2 () noexcept(noexcept(this->b1));
+ void bar3 () noexcept(noexcept(this->b2));
+ void bar4 () noexcept(noexcept(a1));
+ void bar5 () noexcept(noexcept(a2));
+ void bar6 () noexcept(noexcept(b1));
+ void bar7 () noexcept(noexcept(b2));
+ void bar8 () noexcept(noexcept(foovar));
+ void bar9 () noexcept(noexcept(p));
+ int b2;
+ };
+ }
+ int a2;
+};
base-commit: d08677c11dc4b43cc8bab862d1c986563897ce3f
--
2.29.2
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-01-22 22:03 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-21 22:45 [PATCH] c++: ICE with noexcept in class in member function [PR96623] Marek Polacek
2021-01-22 2:47 ` Jason Merrill
2021-01-22 21:01 ` Marek Polacek
2021-01-22 21:44 ` Jason Merrill
2021-01-22 22:03 ` Marek Polacek
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).