Skip to content

Draft: Non-working attempt to solve #838

This is a non-working attempt to get #838 (closed) working. It starts out fine, but when I try to take the final steps (adding () as the type of "no sink wanted", and removing the old code) it all collapses under the weight of its own type inference.

@Diziet, feel free to take a crack at this if you like? It may also be mooted by #837.

Update: Never mind; I got it working-ish. I'll add a further update once it's in a better state.

Okay, it compiles. There are aspects I like and aspects I don't like.

I like these things:

  1. Doing less in the macro is pretty excellent.

  2. Getting rid of ConstTypeId is also a win.

  3. It's a bit nice to have a simpler syntax to use when invoking the macro. (Though IMO there is enough complexity in the required implementation-function types that remembering the syntax for the macro is not much more to ask.)

I don't like these things:

  1. I don't like the bit where we have to declare no-update implementations as having a fourth () argument. We could solve this by reintroducing a little syntax to the macro invocation, or by figuring out if there isn't some alternative to the as fn(_,_,_,_) -> _ that we're doing now

  2. I don't like the bit where we have to declare update implementations as taking an UpdateSink<U> when formerly we could tell the user to use impl Sink + Unpin. Yes, I know that Box<dyn Sink> isn't that much slower than impl Sink, but I kind of dislike this amount of coupling. (I wonder if we could solve this with a macro-generated shim? Probably not. I know we can't tell it the actual unboxed type that we want to pass it, since we can't name the type of a closure.)

  3. I don't like the code duplication in our two different impls.

IMO, we should back-burner this change at least until we know for certain what we're doing with #837: I think some of the possibilities there would moot this work, and I think I'm not currently sold on it.

Edited by Nick Mathewson

Merge request reports