Skip to content

Remove FutureExt::boxed to unify project style#1923

Merged
seanmonstar merged 2 commits into
hyperium:masterfrom
AtkinsChang:remove-boxed
Sep 3, 2019
Merged

Remove FutureExt::boxed to unify project style#1923
seanmonstar merged 2 commits into
hyperium:masterfrom
AtkinsChang:remove-boxed

Conversation

@AtkinsChang

Copy link
Copy Markdown
Contributor

introduced by #1890 while Pin::new_unchecked is used across this project which is more efficient

@taiki-e taiki-e left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

futures_util::pin_mut! is a safe solution for this.

Comment thread src/server/tcp.rs Outdated
let mut accept = self.listener.accept();
loop {
match accept_fut.poll_unpin(cx) {
match unsafe { Pin::new_unchecked(&mut accept) }.poll(cx) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

let accept = self.listener.accept();
futures_util::pin_mut!(accept);

loop {
    match accept.poll_unpin(cx) {

Comment thread src/common/drain.rs Outdated

match recv_fut.poll_unpin(cx) {
let mut recv = me.watch.rx.recv_ref();
match unsafe { Pin::new_unchecked(&mut recv) }.poll(cx) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

let recv = me.watch.rx.recv_ref();
futures_util::pin_mut!(recv);

match recv.poll_unpin(cx) {

@AtkinsChang

Copy link
Copy Markdown
Contributor Author

replaced with futures_util::pin_mut!

@seanmonstar seanmonstar left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks, nice catch!

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.

3 participants