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

macros: add pin! macro #2163

Merged
merged 3 commits into from
Jan 23, 2020
Merged

macros: add pin! macro #2163

merged 3 commits into from
Jan 23, 2020

Conversation

carllerche
Copy link
Member

Used for stack pinning and based on pin_mut! from the pin-util crate.

Pinning is used often when working with stream operators and the select!
macro. Given the small size of pin! it makes more sense to include a
version than re-export one from a separate crate or require the user to
depend on pin-util themselves.

Used for stack pinning and based on `pin_mut!` from the pin-util crate.

Pinning is used often when working with stream operators and the select!
macro. Given the small size of `pin!` it makes more sense to include a
version than re-export one from a separate crate or require the user to
depend on `pin-util` themselves.
/// }
/// ```
#[macro_export]
macro_rules! pin {
Copy link
Member

Choose a reason for hiding this comment

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

Why pin not pin_mut?

Copy link
Member Author

Choose a reason for hiding this comment

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

Why pin_mut?

The macro takes the value by ownership. I don't see when you would want a &self version... you can also always go to &self from mut.

Copy link
Member

Choose a reason for hiding this comment

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

Because it pins a &mut Self reference even though it moves the value. It still produces a pinned mutable reference.

Copy link
Member Author

Choose a reason for hiding this comment

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

mut is just noise in this case as there is no other variant.

Copy link
Member Author

Choose a reason for hiding this comment

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

For example, it is io::Read::read and not io::Read::read_mut

Copy link
Member

Choose a reason for hiding this comment

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

idk I think its not that nosiy and there is already a precedent in the ecosystem 🤷‍♂

I also think in this case its fine to be explicit that its a pin of a mutable ref over just a pin.

Copy link
Member Author

Choose a reason for hiding this comment

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

Idiomatically, using an _mut suffix means there is a version w/o _mut. If there is a pin_mut! there should be a pin!.

Copy link
Member

Choose a reason for hiding this comment

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

ill concede if we can at least reference that this is the same as pin_mut

Copy link
Member

@LucioFranco LucioFranco left a comment

Choose a reason for hiding this comment

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

Docs can be updated later

@carllerche carllerche merged commit a70f720 into master Jan 23, 2020
@carllerche carllerche deleted the pin-macro branch January 26, 2020 22:47
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