search

hyperium / h2

HTTP 2.0 client & server implementation for Rust.
MIT License
1.34k stars 269 forks source link

Remove `B: Buf` bound on `SendStream`'s parameter #614

Closed djkoloski closed 1 year ago

djkoloski commented 2 years ago

This bound is only necessary when calling methods on SendStream because we don't use any associated types of B's Buf impl in the type. Relaxing this bound allows us to construct SendStreams with non-Buf B (though we can't do anything with them).

djkoloski commented 2 years ago

https://github.com/hyperium/h2/pull/616 should fix the nightly CI.

seanmonstar commented 2 years ago

(though we can't do anything with them).

Is this beneficial somehow? I'd love to understand the goal.

djkoloski commented 2 years ago

Sorry, I didn't put enough context in here. https://github.com/hyperium/hyper/pull/2830 contains some work to remove a transmute from hyper. Rather than write new Buf impls with unreachable!, I figured we could clean things up bit by relaxing the bounds in h2. That way we don't have to write uncallable Buf impls with impossible methods. It's not very important, but it does help with code quality downstream.

djkoloski commented 2 years ago

Bumping this after #616 was merged.

djkoloski commented 2 years ago

Quick bump on this, if there are no objections I think this would be a good improvement. 🙂

LucioFranco commented 1 year ago

Oh sorry, we can get this merged :D