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

Add bounds to combinators #2114

Closed
Kixunil opened this issue Apr 3, 2020 · 6 comments
Closed

Add bounds to combinators #2114

Kixunil opened this issue Apr 3, 2020 · 6 comments

Comments

@Kixunil
Copy link
Contributor

Kixunil commented Apr 3, 2020

I suggest adding Future and Stream bounds to combinators wherever appropriate based on this argument: rust-lang/api-guidelines#217

Acording to recent reddit post type errors around futures are common (I share the same experience with 0.1, didn't check 0.3)

At least until someone comes up with a magical solution for those type errors (which would be awesome because we could safely drop types from function signatures too).

@taiki-e
Copy link
Member

taiki-e commented Oct 1, 2020

Note that adding trait bounds is a breaking change.

@Kixunil
Copy link
Contributor Author

Kixunil commented Oct 1, 2020

Yes, and I think it's worth it. Futures is still not 1.0.

@taiki-e
Copy link
Member

taiki-e commented Nov 2, 2020

One of the issues of doing this is the handling of complex bounds.
For example, try_skip_while has the following bounds:

where St: TryStream,
F: FnMut(&St::Ok) -> Fut,
Fut: TryFuture<Ok = bool, Error = St::Error>,

This bounds is needed for try_skip_while to work as a stream, but is it really preferable to always write it (even code that doesn't require try_skip_while to be stream)? For example, try_skip_while implements Sink if the underlying type is Sink. If you are trying to use try_skip_while as a Sink, the suggestions in this issue will make bounds complex but not help you.

The above example may be a bit extreme, but in the end, the actual issue is a diagnostics bug.

(See hyper::server::Server for example of more complex bounds in Future impl)

@taiki-e
Copy link
Member

taiki-e commented Nov 2, 2020

Closing this in favor of upstream discussion. We can revisit this if rust-lang/api-guidelines#217 is accepted.

@taiki-e taiki-e closed this as completed Nov 2, 2020
@Kixunil
Copy link
Contributor Author

Kixunil commented Nov 2, 2020

You bring a good point and I think in this specific case my point still holds. TrySkipWhile is useless without Stream as Sink impl is essentially no-op.

However one could imagine a type that does something useful for both stream and sink (e.g buffering) in such case I would suggest not bounding the struct but still have two constructors - one for sink, one for stream. So thanks for reminding about this scenario.

@Kixunil
Copy link
Contributor Author

Kixunil commented Nov 2, 2020

I expanded on the idea in the related issue, hope it's more clear now.

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

No branches or pull requests

2 participants