* [PATCH] c++/modules: fix virtual destructors [PR103499]
@ 2023-11-09 9:55 Nathaniel Shead
2023-11-09 22:57 ` Nathan Sidwell
0 siblings, 1 reply; 4+ messages in thread
From: Nathaniel Shead @ 2023-11-09 9:55 UTC (permalink / raw)
To: gcc-patches; +Cc: Jason Merrill, Nathan Sidwell
I'm not sure if this is just papering over a general issue of clones not being
exported/imported, or if this is just an exception to the general case of
clones being able to be freely regenerated with no other issues.
Alternatively, would it be better to override the DECL_VINDEX of the original
declaration after filling it in for the clones as well? I wasn't able to see
anything depending on the current behaviour (though I didn't look very hard).
Bootstrapped and regtexted on x86_64-pc-linux-gnu.
-- >8 --
Currently, cloned functions are not included in the CMI. However, for
virtual destructors the clones must have a different DECL_VINDEX from
their base declaration: the former have an INTEGER_CST indicating the
index into the vtable, while the latter indicate the FUNCTION_DECL that
they're overriding.
As such, this patch ensures that DECL_VINDEX is properly passed on for
cloned functions as well to prevent this from causing issues.
PR c++/103499
gcc/cp/ChangeLog:
* module.cc (trees_out::decl_node): Write DECL_VINDEX for
virtual clones.
(trees_in::tree_node): Read DECL_VINDEX for virtual clones.
gcc/testsuite/ChangeLog:
* g++.dg/modules/pr103499_a.C: New test.
* g++.dg/modules/pr103499_b.C: New test.
Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com>
---
gcc/cp/module.cc | 6 ++++++
gcc/testsuite/g++.dg/modules/pr103499_a.C | 12 ++++++++++++
gcc/testsuite/g++.dg/modules/pr103499_b.C | 8 ++++++++
3 files changed, 26 insertions(+)
create mode 100644 gcc/testsuite/g++.dg/modules/pr103499_a.C
create mode 100644 gcc/testsuite/g++.dg/modules/pr103499_b.C
diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
index c1c8c226bc1..416a7c414cc 100644
--- a/gcc/cp/module.cc
+++ b/gcc/cp/module.cc
@@ -8648,6 +8648,8 @@ trees_out::decl_node (tree decl, walk_kind ref)
tree_node (target);
tree_node (DECL_NAME (decl));
+ if (TREE_CODE (decl) == FUNCTION_DECL && DECL_VIRTUAL_P (decl))
+ tree_node (DECL_VINDEX (decl));
int tag = insert (decl);
if (streaming_p ())
dump (dumper::TREE)
@@ -9869,6 +9871,10 @@ trees_in::tree_node (bool is_use)
}
}
+ /* A clone might have a different vtable entry. */
+ if (res && TREE_CODE (res) == FUNCTION_DECL && DECL_VIRTUAL_P (res))
+ DECL_VINDEX (res) = tree_node ();
+
if (!res)
set_overrun ();
int tag = insert (res);
diff --git a/gcc/testsuite/g++.dg/modules/pr103499_a.C b/gcc/testsuite/g++.dg/modules/pr103499_a.C
new file mode 100644
index 00000000000..0497c2c5504
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/pr103499_a.C
@@ -0,0 +1,12 @@
+// PR c++/103499
+// { dg-module-do compile }
+// { dg-additional-options "-fmodules-ts" }
+// { dg-module-cmi pr103499 }
+
+export module pr103499;
+
+export struct base {
+ virtual ~base() = default;
+};
+
+export struct derived : base {};
diff --git a/gcc/testsuite/g++.dg/modules/pr103499_b.C b/gcc/testsuite/g++.dg/modules/pr103499_b.C
new file mode 100644
index 00000000000..b7468562ba9
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/pr103499_b.C
@@ -0,0 +1,8 @@
+// PR c++/103499
+// { dg-additional-options "-fmodules-ts" }
+
+import pr103499;
+
+void test(derived* p) {
+ delete p;
+}
--
2.42.0
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] c++/modules: fix virtual destructors [PR103499]
2023-11-09 9:55 [PATCH] c++/modules: fix virtual destructors [PR103499] Nathaniel Shead
@ 2023-11-09 22:57 ` Nathan Sidwell
2023-11-09 23:29 ` Nathaniel Shead
0 siblings, 1 reply; 4+ messages in thread
From: Nathan Sidwell @ 2023-11-09 22:57 UTC (permalink / raw)
To: Nathaniel Shead, gcc-patches; +Cc: Jason Merrill
On 11/9/23 04:55, Nathaniel Shead wrote:
> I'm not sure if this is just papering over a general issue of clones not being
> exported/imported, or if this is just an exception to the general case of
> clones being able to be freely regenerated with no other issues.
>
> Alternatively, would it be better to override the DECL_VINDEX of the original
> declaration after filling it in for the clones as well? I wasn't able to see
> anything depending on the current behaviour (though I didn't look very hard).
I think your patch is a fine approach. IIRC just streaming out the clones
directly ran into a bunch of issues, hence the current implementation.
ok
nathan
>
> Bootstrapped and regtexted on x86_64-pc-linux-gnu.
>
> -- >8 --
>
> Currently, cloned functions are not included in the CMI. However, for
> virtual destructors the clones must have a different DECL_VINDEX from
> their base declaration: the former have an INTEGER_CST indicating the
> index into the vtable, while the latter indicate the FUNCTION_DECL that
> they're overriding.
>
> As such, this patch ensures that DECL_VINDEX is properly passed on for
> cloned functions as well to prevent this from causing issues.
>
> PR c++/103499
>
> gcc/cp/ChangeLog:
>
> * module.cc (trees_out::decl_node): Write DECL_VINDEX for
> virtual clones.
> (trees_in::tree_node): Read DECL_VINDEX for virtual clones.
>
> gcc/testsuite/ChangeLog:
>
> * g++.dg/modules/pr103499_a.C: New test.
> * g++.dg/modules/pr103499_b.C: New test.
>
> Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com>
> ---
> gcc/cp/module.cc | 6 ++++++
> gcc/testsuite/g++.dg/modules/pr103499_a.C | 12 ++++++++++++
> gcc/testsuite/g++.dg/modules/pr103499_b.C | 8 ++++++++
> 3 files changed, 26 insertions(+)
> create mode 100644 gcc/testsuite/g++.dg/modules/pr103499_a.C
> create mode 100644 gcc/testsuite/g++.dg/modules/pr103499_b.C
>
> diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
> index c1c8c226bc1..416a7c414cc 100644
> --- a/gcc/cp/module.cc
> +++ b/gcc/cp/module.cc
> @@ -8648,6 +8648,8 @@ trees_out::decl_node (tree decl, walk_kind ref)
>
> tree_node (target);
> tree_node (DECL_NAME (decl));
> + if (TREE_CODE (decl) == FUNCTION_DECL && DECL_VIRTUAL_P (decl))
> + tree_node (DECL_VINDEX (decl));
> int tag = insert (decl);
> if (streaming_p ())
> dump (dumper::TREE)
> @@ -9869,6 +9871,10 @@ trees_in::tree_node (bool is_use)
> }
> }
>
> + /* A clone might have a different vtable entry. */
> + if (res && TREE_CODE (res) == FUNCTION_DECL && DECL_VIRTUAL_P (res))
> + DECL_VINDEX (res) = tree_node ();
> +
> if (!res)
> set_overrun ();
> int tag = insert (res);
> diff --git a/gcc/testsuite/g++.dg/modules/pr103499_a.C b/gcc/testsuite/g++.dg/modules/pr103499_a.C
> new file mode 100644
> index 00000000000..0497c2c5504
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/modules/pr103499_a.C
> @@ -0,0 +1,12 @@
> +// PR c++/103499
> +// { dg-module-do compile }
> +// { dg-additional-options "-fmodules-ts" }
> +// { dg-module-cmi pr103499 }
> +
> +export module pr103499;
> +
> +export struct base {
> + virtual ~base() = default;
> +};
> +
> +export struct derived : base {};
> diff --git a/gcc/testsuite/g++.dg/modules/pr103499_b.C b/gcc/testsuite/g++.dg/modules/pr103499_b.C
> new file mode 100644
> index 00000000000..b7468562ba9
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/modules/pr103499_b.C
> @@ -0,0 +1,8 @@
> +// PR c++/103499
> +// { dg-additional-options "-fmodules-ts" }
> +
> +import pr103499;
> +
> +void test(derived* p) {
> + delete p;
> +}
--
Nathan Sidwell
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] c++/modules: fix virtual destructors [PR103499]
2023-11-09 22:57 ` Nathan Sidwell
@ 2023-11-09 23:29 ` Nathaniel Shead
2023-11-09 23:45 ` Nathan Sidwell
0 siblings, 1 reply; 4+ messages in thread
From: Nathaniel Shead @ 2023-11-09 23:29 UTC (permalink / raw)
To: Nathan Sidwell; +Cc: gcc-patches, Jason Merrill
On Thu, Nov 09, 2023 at 05:57:39PM -0500, Nathan Sidwell wrote:
> On 11/9/23 04:55, Nathaniel Shead wrote:
> > I'm not sure if this is just papering over a general issue of clones not being
> > exported/imported, or if this is just an exception to the general case of
> > clones being able to be freely regenerated with no other issues.
> >
> > Alternatively, would it be better to override the DECL_VINDEX of the original
> > declaration after filling it in for the clones as well? I wasn't able to see
> > anything depending on the current behaviour (though I didn't look very hard).
>
> I think your patch is a fine approach. IIRC just streaming out the clones
> directly ran into a bunch of issues, hence the current implementation.
>
>
> ok
>
> nathan
Sorry, I don't have write access, would you be able to push? Thanks.
(And for my other patch.)
> >
> > Bootstrapped and regtexted on x86_64-pc-linux-gnu.
> >
> > -- >8 --
> >
> > Currently, cloned functions are not included in the CMI. However, for
> > virtual destructors the clones must have a different DECL_VINDEX from
> > their base declaration: the former have an INTEGER_CST indicating the
> > index into the vtable, while the latter indicate the FUNCTION_DECL that
> > they're overriding.
> >
> > As such, this patch ensures that DECL_VINDEX is properly passed on for
> > cloned functions as well to prevent this from causing issues.
> >
> > PR c++/103499
> >
> > gcc/cp/ChangeLog:
> >
> > * module.cc (trees_out::decl_node): Write DECL_VINDEX for
> > virtual clones.
> > (trees_in::tree_node): Read DECL_VINDEX for virtual clones.
> >
> > gcc/testsuite/ChangeLog:
> >
> > * g++.dg/modules/pr103499_a.C: New test.
> > * g++.dg/modules/pr103499_b.C: New test.
> >
> > Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com>
> > ---
> > gcc/cp/module.cc | 6 ++++++
> > gcc/testsuite/g++.dg/modules/pr103499_a.C | 12 ++++++++++++
> > gcc/testsuite/g++.dg/modules/pr103499_b.C | 8 ++++++++
> > 3 files changed, 26 insertions(+)
> > create mode 100644 gcc/testsuite/g++.dg/modules/pr103499_a.C
> > create mode 100644 gcc/testsuite/g++.dg/modules/pr103499_b.C
> >
> > diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
> > index c1c8c226bc1..416a7c414cc 100644
> > --- a/gcc/cp/module.cc
> > +++ b/gcc/cp/module.cc
> > @@ -8648,6 +8648,8 @@ trees_out::decl_node (tree decl, walk_kind ref)
> > tree_node (target);
> > tree_node (DECL_NAME (decl));
> > + if (TREE_CODE (decl) == FUNCTION_DECL && DECL_VIRTUAL_P (decl))
> > + tree_node (DECL_VINDEX (decl));
> > int tag = insert (decl);
> > if (streaming_p ())
> > dump (dumper::TREE)
> > @@ -9869,6 +9871,10 @@ trees_in::tree_node (bool is_use)
> > }
> > }
> > + /* A clone might have a different vtable entry. */
> > + if (res && TREE_CODE (res) == FUNCTION_DECL && DECL_VIRTUAL_P (res))
> > + DECL_VINDEX (res) = tree_node ();
> > +
> > if (!res)
> > set_overrun ();
> > int tag = insert (res);
> > diff --git a/gcc/testsuite/g++.dg/modules/pr103499_a.C b/gcc/testsuite/g++.dg/modules/pr103499_a.C
> > new file mode 100644
> > index 00000000000..0497c2c5504
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/modules/pr103499_a.C
> > @@ -0,0 +1,12 @@
> > +// PR c++/103499
> > +// { dg-module-do compile }
> > +// { dg-additional-options "-fmodules-ts" }
> > +// { dg-module-cmi pr103499 }
> > +
> > +export module pr103499;
> > +
> > +export struct base {
> > + virtual ~base() = default;
> > +};
> > +
> > +export struct derived : base {};
> > diff --git a/gcc/testsuite/g++.dg/modules/pr103499_b.C b/gcc/testsuite/g++.dg/modules/pr103499_b.C
> > new file mode 100644
> > index 00000000000..b7468562ba9
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/modules/pr103499_b.C
> > @@ -0,0 +1,8 @@
> > +// PR c++/103499
> > +// { dg-additional-options "-fmodules-ts" }
> > +
> > +import pr103499;
> > +
> > +void test(derived* p) {
> > + delete p;
> > +}
>
> --
> Nathan Sidwell
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] c++/modules: fix virtual destructors [PR103499]
2023-11-09 23:29 ` Nathaniel Shead
@ 2023-11-09 23:45 ` Nathan Sidwell
0 siblings, 0 replies; 4+ messages in thread
From: Nathan Sidwell @ 2023-11-09 23:45 UTC (permalink / raw)
To: Nathaniel Shead; +Cc: gcc-patches, Jason Merrill
On 11/9/23 18:29, Nathaniel Shead wrote:
> On Thu, Nov 09, 2023 at 05:57:39PM -0500, Nathan Sidwell wrote:
>> On 11/9/23 04:55, Nathaniel Shead wrote:
>>> I'm not sure if this is just papering over a general issue of clones not being
>>> exported/imported, or if this is just an exception to the general case of
>>> clones being able to be freely regenerated with no other issues.
>>>
>>> Alternatively, would it be better to override the DECL_VINDEX of the original
>>> declaration after filling it in for the clones as well? I wasn't able to see
>>> anything depending on the current behaviour (though I didn't look very hard).
>>
>> I think your patch is a fine approach. IIRC just streaming out the clones
>> directly ran into a bunch of issues, hence the current implementation.
>>
>>
>> ok
>>
>> nathan
>
> Sorry, I don't have write access, would you be able to push? Thanks.
> (And for my other patch.)
ok, no worries
>
>>>
>>> Bootstrapped and regtexted on x86_64-pc-linux-gnu.
>>>
>>> -- >8 --
>>>
>>> Currently, cloned functions are not included in the CMI. However, for
>>> virtual destructors the clones must have a different DECL_VINDEX from
>>> their base declaration: the former have an INTEGER_CST indicating the
>>> index into the vtable, while the latter indicate the FUNCTION_DECL that
>>> they're overriding.
>>>
>>> As such, this patch ensures that DECL_VINDEX is properly passed on for
>>> cloned functions as well to prevent this from causing issues.
>>>
>>> PR c++/103499
>>>
>>> gcc/cp/ChangeLog:
>>>
>>> * module.cc (trees_out::decl_node): Write DECL_VINDEX for
>>> virtual clones.
>>> (trees_in::tree_node): Read DECL_VINDEX for virtual clones.
>>>
>>> gcc/testsuite/ChangeLog:
>>>
>>> * g++.dg/modules/pr103499_a.C: New test.
>>> * g++.dg/modules/pr103499_b.C: New test.
>>>
>>> Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com>
>>> ---
>>> gcc/cp/module.cc | 6 ++++++
>>> gcc/testsuite/g++.dg/modules/pr103499_a.C | 12 ++++++++++++
>>> gcc/testsuite/g++.dg/modules/pr103499_b.C | 8 ++++++++
>>> 3 files changed, 26 insertions(+)
>>> create mode 100644 gcc/testsuite/g++.dg/modules/pr103499_a.C
>>> create mode 100644 gcc/testsuite/g++.dg/modules/pr103499_b.C
>>>
>>> diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
>>> index c1c8c226bc1..416a7c414cc 100644
>>> --- a/gcc/cp/module.cc
>>> +++ b/gcc/cp/module.cc
>>> @@ -8648,6 +8648,8 @@ trees_out::decl_node (tree decl, walk_kind ref)
>>> tree_node (target);
>>> tree_node (DECL_NAME (decl));
>>> + if (TREE_CODE (decl) == FUNCTION_DECL && DECL_VIRTUAL_P (decl))
>>> + tree_node (DECL_VINDEX (decl));
>>> int tag = insert (decl);
>>> if (streaming_p ())
>>> dump (dumper::TREE)
>>> @@ -9869,6 +9871,10 @@ trees_in::tree_node (bool is_use)
>>> }
>>> }
>>> + /* A clone might have a different vtable entry. */
>>> + if (res && TREE_CODE (res) == FUNCTION_DECL && DECL_VIRTUAL_P (res))
>>> + DECL_VINDEX (res) = tree_node ();
>>> +
>>> if (!res)
>>> set_overrun ();
>>> int tag = insert (res);
>>> diff --git a/gcc/testsuite/g++.dg/modules/pr103499_a.C b/gcc/testsuite/g++.dg/modules/pr103499_a.C
>>> new file mode 100644
>>> index 00000000000..0497c2c5504
>>> --- /dev/null
>>> +++ b/gcc/testsuite/g++.dg/modules/pr103499_a.C
>>> @@ -0,0 +1,12 @@
>>> +// PR c++/103499
>>> +// { dg-module-do compile }
>>> +// { dg-additional-options "-fmodules-ts" }
>>> +// { dg-module-cmi pr103499 }
>>> +
>>> +export module pr103499;
>>> +
>>> +export struct base {
>>> + virtual ~base() = default;
>>> +};
>>> +
>>> +export struct derived : base {};
>>> diff --git a/gcc/testsuite/g++.dg/modules/pr103499_b.C b/gcc/testsuite/g++.dg/modules/pr103499_b.C
>>> new file mode 100644
>>> index 00000000000..b7468562ba9
>>> --- /dev/null
>>> +++ b/gcc/testsuite/g++.dg/modules/pr103499_b.C
>>> @@ -0,0 +1,8 @@
>>> +// PR c++/103499
>>> +// { dg-additional-options "-fmodules-ts" }
>>> +
>>> +import pr103499;
>>> +
>>> +void test(derived* p) {
>>> + delete p;
>>> +}
>>
>> --
>> Nathan Sidwell
>>
--
Nathan Sidwell
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2023-11-09 23:45 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-11-09 9:55 [PATCH] c++/modules: fix virtual destructors [PR103499] Nathaniel Shead
2023-11-09 22:57 ` Nathan Sidwell
2023-11-09 23:29 ` Nathaniel Shead
2023-11-09 23:45 ` Nathan Sidwell
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).