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

Fix type-inference in sink::unfold() by specifying more of its types #2311

Merged
merged 1 commit into from
Jan 12, 2021

Conversation

sdroege
Copy link
Contributor

@sdroege sdroege commented Jan 12, 2021

This is an API change, at least if

  1. Any code uses different types for the existing type parameters, in which case the Sink impl was not available
  2. Any code specifies the type parameters (their number changed)

Without this it's impossible to use a "unnameable" type for the state, which happened to me unfortunately.

pub fn async_write<W: AsyncWrite + Unpin + Send>(
    write: W,
) -> impl Sink<Message>, Error = std::io::Error> + Send {
    struct State<W> {
        write: W,
        buffer: Vec<u8>,
    }

    let state = State {
        write,
        buffer: Vec::with_capacity(8192),
    };

    sink::unfold(state, |mut state, item: Message| {
        async move {
            state.buffer.clear();
            item.write(&mut state.buffer).expect("can't fail");

            match state.write.write_all(&state.buffer).await {
                Ok(_) => {
                    Ok(state)
                }
                Err(err) => {
                    Err(err)
                }
            }
        }
    })
}

Without the changes from this PR this fails with

error[E0282]: type annotations needed
   --> src/utils.rs:175:35
    |
175 |     futures::sink::unfold(state, |mut state, item: Message| {
    |                                   ^^^^^^^^^ consider giving this closure parameter a type
    |
    = note: type must be known at this point

Specifying it as state: State<_> is not sufficient and the actual type can't be named in this context.

With my changes it works as expected.

@sdroege sdroege mentioned this pull request Jan 12, 2021
Copy link
Member

@taiki-e taiki-e left a comment

Choose a reason for hiding this comment

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

Thanks! I'll prepare a new release soon.


EDIT(2021-01-13)

1. Any code uses different types for the existing type parameters, in which case the Sink impl was not available

Technically this is definitely a breaking change, but it was accepted because Sink is sink::unfold's only important trait implementation, and the code broken by this change is believed to have never worked before.

2. Any code specifies the type parameters (their number changed)

RFC 1105 "Policy on semver and API evolution" considers the addition of type parameters as a minor change.

@taiki-e taiki-e merged commit b008f34 into rust-lang:master Jan 12, 2021
@taiki-e taiki-e added the A-sink Area: futures::sink label Jan 12, 2021
@sdroege sdroege deleted the sink-unfold-type-inference branch January 12, 2021 16:27
@sdroege
Copy link
Contributor Author

sdroege commented Jan 12, 2021

Thanks!

@taiki-e taiki-e mentioned this pull request Jan 12, 2021
@taiki-e
Copy link
Member

taiki-e commented Jan 13, 2021

Published in 0.3.10 (and yanked 0.3.9)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-sink Area: futures::sink
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants