Skip to content

Conversation

@eeckstein
Copy link
Contributor

The drop_deinit instruction is a marker for a following destroy instruction to suppress the call of the move-only type's deinitializer.

SILGen needs to insert drop_deinit in move-only deinitializers to avoid that - due to other optimizations, like inlining - the deinitializer call is inserted multiple times.

rdar://105798769

his instruction is a marker for a following destroy instruction to suppress the call of the move-only type's deinitializer.
…gument

This happens for address-only move-only types.
@eeckstein eeckstein requested review from atrick, gottesmm and kavon April 11, 2023 08:31
@eeckstein
Copy link
Contributor Author

@swift-ci test

@gottesmm
Copy link
Contributor

I am fine with this.

@gottesmm gottesmm merged commit bb2fc82 into swiftlang:main Apr 11, 2023
@eeckstein eeckstein deleted the drop_deinit branch April 11, 2023 17:27
Copy link
Contributor

@atrick atrick left a comment

Choose a reason for hiding this comment

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

Thanks @eeckstein. Looks great!

%1 = drop_deinit %0 : $T
// T must be a move-only type
// %1 is an @owned T
%3 = drop_deinit %2 : $*T
Copy link
Contributor

@atrick atrick Apr 25, 2023

Choose a reason for hiding this comment

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

@eeckstein do we need to support an address form for drop_deinit?
I really hope we don't need that! It's easy to misuse and hard to verify.
In the meantime, can you remove it from the docs?

If we need an address form, we should discuss whether that should be drop_deinit_addr. Generally, instructions should have fixed number and type of operands except in special cases. debug_value is an exception where it works out ok. But an instruction that produces a new address is not straightforward.

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