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

m: Remove unnecessary dependencies from event-listener-strategy #50

Closed
wants to merge 2 commits into from

Conversation

notgull
Copy link
Member

@notgull notgull commented Mar 15, 2023

  • We never actually end up using pin_utils, it was a mistake to leave that in.
  • We can get around pin_project_lite by implementing manual pin projection.

@fogti
Copy link
Member

fogti commented Mar 15, 2023

what does m: stand for?

@fogti
Copy link
Member

fogti commented Mar 15, 2023

it seems that this breaks MSRV but idk why...

@notgull
Copy link
Member Author

notgull commented Mar 15, 2023

what does m: stand for?

m stands for "maintenance". I use it for PRs with no public changes.

@fogti
Copy link
Member

fogti commented Mar 15, 2023

it would be nice to compare this side-by-side with the expanded version using pin-project-lite, to see how they get around the MSRV thingie...

@notgull
Copy link
Member Author

notgull commented Apr 2, 2023

It look like pin_project_lite just calls Pin::new_unchecked on the result of Pin::get_unchecked_mut. I adjusted this PR to use that strategy instead.

@taiki-e
Copy link
Collaborator

taiki-e commented Apr 3, 2023

pin_project_lite just calls Pin::new_unchecked on the result of Pin::get_unchecked_mut

pin-project{,-lite} does not just call those unsafe functions. In this PR's macro, implementing Unpin or Drop on the generated structure or adding the #[repr(packed)] attribute to the input struct can cause unsoundness.

(Even though it may seem simple at first glance, using the unsafe parts of the Pin API correctly is not always easy. e.g., rust-lang/futures-rs#2649, tokio-rs/tokio#2612, etc.)

@notgull
Copy link
Member Author

notgull commented Apr 3, 2023

pin-project{,-lite} does not just call those unsafe functions. In this PR's macro, implementing Unpin or Drop on the generated structure or adding the #[repr(packed)] attribute to the input struct can cause unsoundness.

(Even though it may seem simple at first glance, using the unsafe parts of the Pin API correctly is not always easy. e.g., rust-lang/futures-rs#2649, tokio-rs/tokio#2612, etc.)

Hmm, this is a good point. Maybe we shouldn't remove pin-project-lite, just so that we can piggyback off of that package's guarantees.

@notgull
Copy link
Member Author

notgull commented Apr 28, 2023

Closed in favor of #56

@notgull notgull closed this Apr 28, 2023
@notgull notgull deleted the notgull/no-pin-utils branch April 28, 2023 20:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants