Skip to content
This repository was archived by the owner on Oct 12, 2022. It is now read-only.
/ druntime Public archive

Translate _d_delstruct to template#3639

Merged
dlang-bot merged 3 commits intodlang:masterfrom
teodutu:translate-_d_delstruct
Dec 3, 2021
Merged

Translate _d_delstruct to template#3639
dlang-bot merged 3 commits intodlang:masterfrom
teodutu:translate-_d_delstruct

Conversation

@teodutu
Copy link
Member

@teodutu teodutu commented Dec 1, 2021

This implements a template version of _d_delstruct to replace the homonymous function from rt.lifetime.d.

Signed-off-by: Teodor Dutu <teodor.dutu@gmail.com>
@dlang-bot
Copy link
Contributor

Thanks for your pull request and interest in making D better, @teodutu! 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

Your PR doesn't reference any Bugzilla issue.

If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog.

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 "master + druntime#3639"

Signed-off-by: Teodor Dutu <teodor.dutu@gmail.com>
@PetarKirov
Copy link
Member

Can you refactor the original (non-template) version of the hook to call the new (template) one?

That way, the change would be easier to review, since the net result would be that most of the code would be moved to a new function and all of the existing tests would still exercise the code paths.

@teodutu
Copy link
Member Author

teodutu commented Dec 2, 2021

Can you refactor the original (non-template) version of the hook to call the new (template) one?

The signature of the old hook is this:

extern (C) void _d_delstruct(void** p, TypeInfo_Struct inf) @weak

In order to call the new hook from it, I would need to cast *p to the correct type, which I cannot do. So I don't think I can make this change.

Signed-off-by: Teodor Dutu <teodor.dutu@gmail.com>
@teodutu teodutu requested a review from MartinNowak as a code owner December 2, 2021 15:35
@PetarKirov
Copy link
Member

PetarKirov commented Dec 3, 2021

You're right. For some reason I didn't check carefully the new implementation, but instead I assumed that it would use the same typeinfo-based logic that this would later be changed to template-driven, which I see you have already done.

@dlang-bot dlang-bot merged commit cb9f7c5 into dlang:master Dec 3, 2021
@teodutu teodutu deleted the translate-_d_delstruct branch December 3, 2021 19:01
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants