Re: [PATCH] c++/modules: fix virtual destructors [PR103499]

[Nathan Sidwell <nathan@acm.org>]

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