Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Mark NoFinalize as unsafe and document it. #40

Merged
merged 1 commit into from
Aug 10, 2021

Conversation

jacob-hughes
Copy link
Collaborator

The NoFinalize trait prevents a type T from being finalized when its
passed to Gc<T>.

Now that we've settled on a preferred semantics for NoFinalize, the implementation here was already correct. I've tweaked it to be unsafe, added a couple more tests, and documented it.

/// Unsafe because this should be used with care. Preventing drop from
/// running can lead to surprising behaviour. In particular, this will also
/// prevent the finalization of all component types of T.
pub unsafe fn new(value: T) -> NonFinalizable<T> {
Copy link
Member

Choose a reason for hiding this comment

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

Just to be awkward, since this is ManuallyDrop in disguise, I think it's OK for this to not be marked as unsafe (ManuallyDrop::new is not unsafe). I don't feel strongly about that though!

@ltratt
Copy link
Member

ltratt commented Aug 9, 2021

I notice that the PR has conflicts? Maybe push a merge commit to master in here?

@ltratt
Copy link
Member

ltratt commented Aug 10, 2021

@jacob-hughes Where are we with this one?

@jacob-hughes
Copy link
Collaborator Author

I notice that the PR has conflicts? Maybe push a merge commit to master in here?

I'm not sure what happened there which made my local branch so out of sync with the softdev repo, anyway I've added a merge commit which can be squashed away later.

@ltratt
Copy link
Member

ltratt commented Aug 10, 2021

Please squash.

@jacob-hughes jacob-hughes force-pushed the fix_no_finalize_semantics branch from 5838776 to d62c55b Compare August 10, 2021 09:37
The `NoFinalize` trait prevents a type `T` from being finalized when its
passed to `Gc<T>`.
@jacob-hughes jacob-hughes force-pushed the fix_no_finalize_semantics branch from d62c55b to 5ebb0fa Compare August 10, 2021 09:44
@jacob-hughes
Copy link
Collaborator Author

Squashed

@ltratt
Copy link
Member

ltratt commented Aug 10, 2021

bors r+

@bors
Copy link

bors bot commented Aug 10, 2021

Build succeeded:

@bors bors bot merged commit ff09bcc into softdevteam:master Aug 10, 2021
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants