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

refactor(lib): remove pin related unsafe code #2220

Merged
merged 2 commits into from
Jun 8, 2020

Conversation

taiki-e
Copy link
Contributor

@taiki-e taiki-e commented Jun 6, 2020

No description provided.

match mem::replace(&mut me.state, State::Tmp) {
State::NotReady(mut svc, req) => {
me.state = State::Called(svc.call(req));
match me.state.as_mut().project_replace(State::Tmp) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

project_replace is an optional method recently added.

It is equivalent to Pin::set, except that the unpinned fields are moved and returned, instead of being dropped in-place.

fn project_replace(self: Pin<&mut Self>, other: Self) -> ProjectionOwned;

The ProjectionOwned type is identical to the Self type, except that all pinned fields have been replaced by equivalent PhantomData types.

state: State<S, Req>,
}

#[pin_project(Replace, project = StateProj, project_replace = StateProjOwn)]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is not ideal that we need to have both Replace and project_replace = <name> as project_replace = <name> implies that a project_replace method is generated.
I'll fix it soon.

Copy link
Member

Choose a reason for hiding this comment

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

Do you mean I should wait to merge, or just some time later?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh sorry, a fix for this was already merged, but I forgot to release a new version.

Copy link
Contributor Author

@taiki-e taiki-e Jun 6, 2020

Choose a reason for hiding this comment

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

Updated to use a new version pin-project which includes a fix for this. (402506d)

@taiki-e taiki-e marked this pull request as draft June 6, 2020 03:41
bors bot added a commit to taiki-e/pin-project that referenced this pull request Jun 6, 2020
243: Allow `project_replace` argument to be used without `Replace` r=taiki-e a=taiki-e

Currently, both `Replace` and `project_replace = <name>` arguments are required to name the return value of `project_replace()` method.

```rust
#[pin_project(Replace, project_replace = EnumProjOwn)]
enum Enum<T> {
    Variant(#[pin] T)
}
```

It is not ideal that we need to have both `Replace` and `project_replace = <name>` as `project_replace = <name>` implies that a `project_replace` method is generated.

This PR makes `project_replace` argument to use without `Replace` argument.

```diff
- #[pin_project(Replace, project_replace = EnumProjOwn)]
+ #[pin_project(project_replace = EnumProjOwn)]
  enum Enum<T> {
      Variant(#[pin] T)
  }
```

Also, makes `project_replace` argument an alias for `Replace` so that it can be used without a value.


```rust
#[pin_project(project_replace)]
enum Enum<T> {
    Variant(#[pin] T)
}
```

Related: rust-lang/futures-rs#2176 (comment) hyperium/hyper#2220 (comment)

Co-authored-by: Taiki Endo <[email protected]>
@taiki-e taiki-e marked this pull request as ready for review June 6, 2020 18:36
@seanmonstar seanmonstar merged commit 9998f0f into hyperium:master Jun 8, 2020
@taiki-e taiki-e deleted the safe-pin branch June 8, 2020 23:04
BenxiangGe pushed a commit to BenxiangGe/hyper that referenced this pull request Jul 26, 2021
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.

2 participants