Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Remove PhantomData<*mut Waiter> so we don't need to manually impl Send #237

Merged
merged 1 commit into from Jun 4, 2023

Conversation

Enselic
Copy link
Contributor

@Enselic Enselic commented Jun 4, 2023

The effect of having PhantomData<*mut Waiter> is that the OnceCell<T> stops being impl<T: Send> Send for OnceCell<T>. But we want it to be impl<T: Send> Send for OnceCell<T>, because we manually mark it as such.

By removing the PhantomData we can also remove the manual impl Send. The net effect is zero changes to the public API surface (including auto traits) of once_cell:

$ cargo install cargo-public-api --locked
$ cargo public-api diff origin/master..remove-unneeded-phantomdata
Removed items from the public API
=================================
(none)

Changed items in the public API
===============================
(none)

Added items to the public API
=============================
(none)

A variant of this PR is to remove the PhantomData but keep the manual impl Send. Maybe you prefer the explicitness.

Background of this PR

I am studying how once_cell is implemented, and could not at first understand why we needed the PhantomData<*mut Waiter>. Digging deeper, I concluded that we do not in fact need it.

The effect of having `PhantomData<*mut Waiter>` is that the `OnceCell<T>`
stops being `impl<T: Send> Send for OnceCell<T>`. But we want it to be
`impl<T: Send> Send for OnceCell<T>`, because we manually mark it as
such.

Remove the `PhantomData` so we can also remove the manual `impl Send`.
@matklad
Copy link
Owner

matklad commented Jun 4, 2023

Yeah, phantom data is no longer needed, as we switched to AtomicPtr a while ago. Let’s keep the explicit Send bound here — send/sync bounds are versus subtle here, so better to spell them out explicitly

@matklad
Copy link
Owner

matklad commented Jun 4, 2023

Let me merge as is while I am at a laptop..

@matklad matklad merged commit 51a9724 into matklad:master Jun 4, 2023
1 check passed
@Enselic Enselic deleted the remove-unneeded-phantomdata branch June 4, 2023 17:32
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.

None yet

2 participants