Issue 21693 - Lower scoped destruction of extern(C++) class to destroy/dtor call

[MoonlightSentinel]

Ensures proper RAII behaviour for stack allocated instances but doesn't affect the behaviour for heap allocated instances.

The previous rewrite used delete which relies on TypeInfo and crashed at runtime. Using object.destroy circumvents this issue (and is also beneficial due to the deprecation of delete).

destroy can be used instead of delete because the instances live on the heap and hence don't need to be deallocated.

[dlang-bot]

Thanks for your pull request and interest in making D better, @MoonlightSentinel! We are looking forward to reviewing it, and you should be hearing from a maintainer soon.
Please verify that your PR follows this checklist:

• My PR is fully covered with tests (you can see the coverage diff by visiting the details link of the codecov check)
• My PR is as minimal as possible (smaller, focused PRs are easier to review than big ones)
• I have provided a detailed rationale explaining my changes
• New or modified functions have Ddoc comments (with Params: and Returns:)

Please see CONTRIBUTING.md for more information.

---

If you have addressed all reviews or aren't sure how to proceed, don't hesitate to ping us with a simple comment.

Bugzilla references

Auto-close	Bugzilla	Severity	Description
✗	21693	critical	extern(C++) class instance dtors are never called, breaking RAII

Testing this PR locally

If you don't have a local development environment setup, you can use Digger to test this PR:

dub run digger -- build "stable + dmd#12265"

[tsbockman]

destroy should be used instead of delete for for D classes, too: https://issues.dlang.org/show_bug.cgi?id=21692

[MoonlightSentinel]

destroy doesn't work for const/immutable either (unless we insert an explicit cast to mutable)

[MoonlightSentinel]

Implemented another approach, it's now directly calling the dtor (effectively inlining the call to destroy).
This matches the implementation for structs and allows the hack to call dtors on immutable instances.

[kinke]

destroy doesn't work for const/immutable either (unless we insert an explicit cast to mutable)

Seems like it does (tested with v2.096.0). Anyway, the direct dtor call is better. Is there a reason you don't use it for D classes too? I find it inconsistent (can use scope const/immutable with C++ classes, but not with D ones) and don't really like deprecated expressions in compiler-generated AST nodes. ;)

[MoonlightSentinel]

Seems like it does (tested with v2.096.0).

Nevermind, seems like it was an issue with the lowering.

Is there a reason you don't use it for D classes too? I find it inconsistent (can use scope const/immutable with C++ classes, but not with D ones) and don't really like deprecated expressions in compiler-generated AST nodes. ;)

Mostly because destroy (and delete IIRC) refer to rt_finalize2 which does more cleanup than just calling the dtor. Not sure if those additional steps are required for stack allocated instances...

But lowering to destroy would still work for D classes.

[kinke]

But lowering to destroy would still work for D classes.

Yeah that'd be great.

You're right, for D classes, there's apparently the need to descend to all base classes and invoke the dtors manually (via TypeInfo... argh), and the monitor might need cleanup too. LDC has a special case for these delete expressions for scope classes and tries to avoid calling druntime if the class has no finalizer and the monitor is null: https://github.com/ldc-developers/ldc/blob/937c57b8354d4821daededb35abdb77d16ce3737/gen/classes.cpp#L200-L222

Doing this here in the frontend directly would obviously be much nicer.

[MoonlightSentinel]

Maybe the best way forward is to replace the TypeInfo based dtor-loop in DRuntime with a real dtor chain as done for C++ classes and to move the cleanup into `Object.~this()). That would cause less (magic) code scattered across dmd/druntime/ldc/... .

Anyway, lowering to destroy doesn't work yet.

• it isn't alway @nogc
• CTFE cannot handle destroy (it needs some special casing as done for delete)