Skip to content

Fix #23458 - Add pragma for C++ deleting destructors - #23459

Open
tim-dlang wants to merge 1 commit into
dlang:masterfrom
tim-dlang:deldtor
Open

Fix #23458 - Add pragma for C++ deleting destructors#23459
tim-dlang wants to merge 1 commit into
dlang:masterfrom
tim-dlang:deldtor

Conversation

@tim-dlang

Copy link
Copy Markdown
Contributor

Currently deleting destructors for C++ are implemented in D by only running the normal destructors and not freeing memory. This can result in memory leaks if instances are deleted from C++.

Implementing deleting destructors like in C++ would break existing D code, because it requires linking the C++ standard library. Instead this PR adds a pragma for enabling freeing memory in deleting destructors for classes. The pragma is inherited by subclasses, so it normally only needs to be used on root classes.
For classes, which are not extern(C++), or other declarations the pragma is ignored.

In a future edition the pragma could be enabled by default.

Currently deleting destructors for C++ are implemented in D by only
running the normal destructors and not freeing memory.
This can result in memory leaks if instances are deleted from C++.

Implementing deleting destructors like in C++ would break existing
D code, because it requires linking the C++ standard library.
Instead this PR adds a pragma for enabling freeing memory in deleting
destructors for classes. The pragma is inherited by subclasses, so it
normally only needs to be used on root classes.
For classes, which are not `extern(C++)`, or other declarations
the pragma is ignored.

In a future edition the pragma could be enabled by default.
@tim-dlang
tim-dlang requested a review from ibuclaw as a code owner July 25, 2026 12:53
Comment thread compiler/src/dmd/dclass.d
int cppDtorVtblIndex = -1;

/// free memory in deleting destructor
bool cppUseDelDtor;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm these really should be turned into a bitfield if this is merged.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that would be good for the future. The added bool fields don't change the class size here, because the alignment leaves space, but other fields could also be combined into a bitfield.

@github-actions

Copy link
Copy Markdown

DMD perf check

Metric Base PR delta
compile hello.d (instr) 215.7 M 215.7 M +0.01%
compile hello.d -O (instr) 234.2 M 234.3 M +0.02%
compile Phobos (instr) 5,073.8 M 5,075.5 M +0.03%
dmd binary size (stripped) 6.88 MB 6.89 MB +0.17%
hello binary size 0.72 MB 0.72 MB 0.00%
peak RSS (compile hello.d) 44 MB 44 MB +0.13%
peak RSS (compile Phobos) 633 MB 635 MB +0.27%

Comment on lines +34 to +35
pragma(cpp_use_deleting_destructor, true)
class DBase

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

test also struct? Or os the deleting destructor an inheritance thing?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The deleting destructor is only needed for classes, because it is placed in the vtbl. The pragma is ignored for structs and other symbols.

Adding tests for struct could still make sense, because I don't think there are existing tests checking if the C++ memory is really freed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants