Expose nameable future for TaskLocal::scope

[NeoLegends]

Motivation

See #3256.

We are implementing tower::Services and would now like to use task-locals as a part of that. tower::Service currently requires the returned future to be nameable, and those returned by Localkey::scope are not.

We could box all the futures we return from the services but we'd like to avoid that for performance reasons.

Solution

LocalKey::scope internally uses a hand-rolled, nameable future anyways which is just not exported (likely because of a dummy trait used as a workaround for pin_project_lite's limitations). I sealed the dummy trait and exported the future such that it can be used by downstream consumers.

I hope this is a solution you agree with. I'm happy for suggestions on the docs, english is not my native language.

Closes #3256

[carllerche]

Thanks. My primary concern with exposing future types is that all these types clutter the docs.

if we go with exposing these futures, I would like them to be in a separate module, similar to what we do w/ error types (.e.g. sync::error.

My guess is we should put these future types in task::future or something like that? Thoughts @tokio-rs/maintainers ?

[LucioFranco]

You should be able to just box to name and in the future use existential_types, so I don't think its worth it, esp for our push for 1.0.

[Darksonn]

I would be ok with exposing it and putting it in tokio::task::future. That said, I dislike having this fancy trait shenanigans in our public API just to appease pin-project-lite. I think that we should strongly consider just using T: 'static instead of the trait, even if it comes at the cost of using unsafe code rather than pin-project-lite.

[carllerche]

Left a note in case we move forward. Thanks for calling this out @Darksonn

[carllerche]

If we move forward, we would not want to expose StaticLifetime here. It is an implementation detail.

[NeoLegends]

Thanks for the feedback. That's very helpful!

I just made the required changes. Please let me know if there is anything else you'd like changed.

---

You should be able to just box to name and in the future use existential_types, so I don't think its worth it, esp for our push for 1.0.

According to rust-lang/rust#63066, existential_types seems quite far away still.

[taiki-e]

a workaround for pin_project_lite's limitations

BTW what were the limitations? (If it is taiki-e/pin-project-lite#2, you can avoid it by moving bounds to where clause and splitting it.)

[NeoLegends]

Hey @taiki-e, thanks for the suggestion!

This is way better now w/o unsafe.

[LucioFranco]

I thought we were moving away from providing explicit futures like this and instead trying to take advantage of erased types and in the future existential types?

[carllerche]

@LucioFranco, whether we are going to expose combinators or not, is still an open question: #2723

I agree that it would be nice to take advantage of existential types, but there is no timeline for that. We probably should come to a decision on whether or not we are going to expose combinators sooner than later.

[NeoLegends]

@LucioFranco I'd love to!

I'd also like to invite you to read rust-lang/rust#63066, where you'll see that none of the boxes are ticked off yet, which leads me to assume that existential_types is actually quite far away still from stable rust, so perhaps for the time being the maintenance overhead this brings is a tradeoff worth having.

[LucioFranco]

Well I believe last I heard existential types on traits works well, its the other issues with free form. I do think though boxing your future is a totally fine solution as well. Its not zero-cost but rust is so fast anyways I've yet to see (except for one unique) case where it doesn't work out. I personally, am in the boat of not exposing any future types and going all in on async/await. I think for 1.0 this makes even more sense.

[Darksonn]

Regarding the combinator discussion, I think there is an important difference between combinators that are futures and those that are streams.

[Darksonn]

We have already merged the similar PR #3840. I think it is fine to make this change.

[carllerche]

I'm ok w/ exposing named futures.