Stream: wg-async-foundations

Topic: #65244 - IntoFuture


centril (Oct 09 2019 at 18:11, on Zulip):

So re. #65244 -- maybe we should request a mini-RFC for the community consensus aspect of this and ideally the RFC includes some motivational examples... cc @nikomatsakis @Taylor Cramer

centril (Oct 09 2019 at 18:14, on Zulip):

We can land it as unstable in parallel as well or something

centril (Oct 09 2019 at 18:14, on Zulip):

@nikomatsakis also feel free to reassign to me for the remaining code review if you are busy -- mostly wanted to cc you ^^

nikomatsakis (Oct 09 2019 at 18:16, on Zulip):

Yes, I agree we need some kind of report. Not sure if a full RFC is needed, but we need more than a PR.

centril (Oct 09 2019 at 18:18, on Zulip):

@nikomatsakis A more experimental approach perhaps: We land it as unstable for now and allow use cases to be collected in the wild -- then eventually we have a stabilization report which includes use cases + the usual stuff

Taylor Cramer (Oct 09 2019 at 18:53, on Zulip):

@centril I'd be in favor of that, although the biggest unstable usecase I'd be interested in we can't test on nightly since it'd be insta-stable (tuples)

nikomatsakis (Oct 10 2019 at 00:38, on Zulip):

@Taylor Cramer what would be the semantics of (a, b).await?

Also, I'm good with landing it as an unstable experiment and having a stabilization report, yes.

Taylor Cramer (Oct 10 2019 at 00:39, on Zulip):

@nikomatsakis in the old IntoFuture, (a, b).await was equivalent to today's join!(a, b)

nikomatsakis (Oct 10 2019 at 00:39, on Zulip):

OK, that's what I hoped you would say

nikomatsakis (Oct 10 2019 at 00:40, on Zulip):

Personally i'd also be fine to move fairly quickly to stabilize -- give it a bit of time to see if there is some unexpected interaction

nikomatsakis (Oct 10 2019 at 00:41, on Zulip):

but we've got two strong use cases and plenty of precedent around the whole into design

nikomatsakis (Oct 10 2019 at 00:41, on Zulip):

(IntoIterator, of course)

centril (Oct 10 2019 at 00:43, on Zulip):

That doesn't seem like an entirely obvious choice to me that should be made for the user; could also be equivalent to (a.await, b.await).
It's also noteworthy that IntoIterator is not implemented for tuples (zip vs. cartesian products) and neither do we implement a similar construct for Fn

Taylor Cramer (Oct 10 2019 at 00:47, on Zulip):

yup, I agree it's not super intuitive

centril (Oct 10 2019 at 00:47, on Zulip):

ui/async-await/async-fn-size-moved-locals.rs has noticed a mem::size_of_val change of 4 bytes on joined. I suppose this change adds a new local that impact layout?

This seems ungreat tho? -- Seems like an optimization should be able to deal with it but maybe we want that first?

Taylor Cramer (Oct 10 2019 at 00:47, on Zulip):

I think join! is much more discoverable

Taylor Cramer (Oct 10 2019 at 00:47, on Zulip):

I'm not sure I understood the desired usecases correctly-- it seems like a big hammer to omit a single .build() at the end of a builder or similar

Taylor Cramer (Oct 10 2019 at 00:48, on Zulip):

But I really don't hold a strong opinion on this

centril (Oct 10 2019 at 00:52, on Zulip):

(Aside: not a massive fan of join! as a name; it suggests μ : T (T a) → T a... ^^)

centril (Oct 10 2019 at 00:53, on Zulip):

@Taylor Cramer should we block on not regressing size of futures tho?

centril (Oct 10 2019 at 00:55, on Zulip):

Feels like something that a bit of inlining would fix in the common case of already_a_future.await

centril (Oct 10 2019 at 00:55, on Zulip):

cc @tmandry

tmandry (Oct 10 2019 at 02:45, on Zulip):

if that's the only regression I wouldn't worry about it

tmandry (Oct 10 2019 at 02:46, on Zulip):

the size of that type is already 3084

tmandry (Oct 10 2019 at 02:47, on Zulip):

I mean presumably other types of lesser size will also get bigger, but if that's the only test that changes.. shrug the layout optimization is already something I want to change again

tmandry (Oct 10 2019 at 02:48, on Zulip):

and since it's probably an NP-complete problem, let's accept that any change that improves a majority of cases might regress some others

tmandry (Oct 10 2019 at 02:49, on Zulip):

IOW, I want to make changes in the future that might make some generator sizes worse, and I don't want to be blocked by that :)

centril (Oct 10 2019 at 02:51, on Zulip):

@tmandry doesn't this increase the size of async fn by 4 * N_await_points?

tmandry (Oct 10 2019 at 02:52, on Zulip):

not the way I read it

tmandry (Oct 10 2019 at 02:52, on Zulip):

it sounded like a single future in one test changed

tmandry (Oct 10 2019 at 02:53, on Zulip):

I haven't looked at the implementation at all though

centril (Oct 10 2019 at 02:53, on Zulip):

@tmandry the implementation is just match expr { ... } => match IntoFuture::into_future(expr) { ... }

tmandry (Oct 10 2019 at 02:55, on Zulip):

well, I or someone should probably look into what's going on

tmandry (Oct 10 2019 at 02:55, on Zulip):

maybe tomorrow

centril (Oct 10 2019 at 09:46, on Zulip):

@boats :wave:

Hmm; personally it seems to me that an FCP for nightly is a bit counter productive towards experimentation

Nemo157 (Oct 10 2019 at 09:55, on Zulip):

One question would be how this interacts with futures 0.3, it seems like quite a few of the utilities could be updated to use IntoFuture where they currently use Future, e.g. flatten

Nemo157 (Oct 10 2019 at 09:56, on Zulip):

Maybe having futures-util 0.4 after only a couple of months would be a good test of the semver stability crate divisions

centril (Oct 10 2019 at 09:58, on Zulip):

@Nemo157 I would personally like to see .flatten() and other functor/monadic/... combinators be moved to the standard library

boats (Oct 10 2019 at 10:43, on Zulip):

just a note: IntoFuture introduces a new type projection which could be !Send

boats (Oct 10 2019 at 10:43, on Zulip):

its possible this could interact with error messages pretty badly

boats (Oct 10 2019 at 10:43, on Zulip):

whereas today there's no projection from the type of the expression you await

centril (Oct 10 2019 at 11:03, on Zulip):

Yeah that's pretty clear from https://github.com/rust-lang/rust/pull/65244/files#diff-81f33f28041996539ee55e2f9c099f90

boats (Oct 10 2019 at 12:17, on Zulip):

yea exactly, I'd like to see some diagnostic improvements there (at least before stabilizing this extension)

Sean McArthur (Oct 10 2019 at 18:10, on Zulip):

the reason i filed a PR instead of an RFC is because the RFC already mentioned it, and i couldn't find any concrete reason why it wasn't included.

centril (Oct 10 2019 at 18:11, on Zulip):

yea figured as much

Sean McArthur (Oct 10 2019 at 18:12, on Zulip):

as for the implementation, i don't really know why i'm seeing an explosion of Into, is there flags i can pass to get better diagnostics? or just turn on the log firehose?

nikomatsakis (Oct 11 2019 at 16:08, on Zulip):

Hmm; personally it seems to me that an FCP for nightly is a bit counter productive towards experimentation

You think? I think it makes sense for us to "decide" to experiment, and FCP seems ok for that. This seems like a case where I'd be ok with not "waiting", though. I think what I mostly mean is that I do think we should have some "check-in" process and we should log the decision to experiment as a kind of decision.

nikomatsakis (Oct 11 2019 at 16:10, on Zulip):

Truth is I think this is a more general question that we should discuss as part of figuring out how we revamp the feature dev process to be more "incremental" (which is part of what I'm calling project groups).

centril (Oct 11 2019 at 16:11, on Zulip):

@nikomatsakis I guess I think deciding via FCP to experiment for something rather small is ungreat

centril (Oct 11 2019 at 16:11, on Zulip):

and that a meeting was enough

nikomatsakis (Oct 11 2019 at 16:11, on Zulip):

I think join! is much more discoverable

Yeah it's true. Though I continue to strongly prefer foo.join(bar). I guess I just don't love macros or something. Anyway this seems like a separable topic -- to me, the builders alone justify the hook, which doesn't strike me as overly complex? Basically await uses a trait to create something you can await. Feels natural, just as for you uses a trait to create something you can iterate.

nikomatsakis (Oct 11 2019 at 16:11, on Zulip):

and that a meeting was enough

yes, I agree a meeting is probably enough

centril (Oct 11 2019 at 16:11, on Zulip):

but OTOH the diagnostics regressions are also ungreat

nikomatsakis (Oct 11 2019 at 16:11, on Zulip):

that seems like it a separate concern

centril (Oct 11 2019 at 16:12, on Zulip):

true

nikomatsakis (Oct 11 2019 at 16:12, on Zulip):

that just seems like a quality question

nikomatsakis (Oct 11 2019 at 16:12, on Zulip):

we shouldn't let PRs land that regress quality of stable compiler full stop (or at least not without discussion)

centril (Oct 11 2019 at 16:12, on Zulip):

@nikomatsakis guess the question is "are we OK regressing diagnostics on nightly for a while"?

nikomatsakis (Oct 11 2019 at 16:12, on Zulip):

it's not only nightly is the main concern :)

nikomatsakis (Oct 11 2019 at 16:12, on Zulip):

(to me)

nikomatsakis (Oct 11 2019 at 16:12, on Zulip):

at least if I understand

nikomatsakis (Oct 11 2019 at 16:13, on Zulip):

sorry, what I mean by that is

nikomatsakis (Oct 11 2019 at 16:13, on Zulip):

the diagnostics are regressed for anybody using await -- of course it will only affect nightly until it rides the trains

nikomatsakis (Oct 11 2019 at 16:13, on Zulip):

but that would put us "on the hook" to fix

centril (Oct 11 2019 at 16:13, on Zulip):

@nikomatsakis something something if sess.is_nightly_compiler() { change_lowering(); } ^^

nikomatsakis (Oct 11 2019 at 16:13, on Zulip):

Yeah, I'm not into that :) at least not without some thought about why the diagnostics are regressing

nikomatsakis (Oct 11 2019 at 16:14, on Zulip):

I'd be happy to put that on my queue and investigate next wed when I have async-await time

nikomatsakis (Oct 11 2019 at 16:14, on Zulip):

(currently, my async-await time is Wed mornings)

centril (Oct 11 2019 at 16:14, on Zulip):

that sounds better :slight_smile:

nikomatsakis (Oct 11 2019 at 16:14, on Zulip):

assign to me?

nikomatsakis (Oct 11 2019 at 16:14, on Zulip):

I'll add a note

nikomatsakis (Oct 11 2019 at 16:14, on Zulip):

(if somebody else wants to investigate in the meantime, sgtm)

centril (Oct 11 2019 at 16:14, on Zulip):

it's already r? you?

nikomatsakis (Oct 11 2019 at 16:15, on Zulip):

ok good

nikomatsakis (Oct 11 2019 at 16:15, on Zulip):

I couldn't remember :)

centril (Oct 17 2019 at 20:05, on Zulip):

@tmandry we reassigned https://github.com/rust-lang/rust/pull/65244 to you to consider the generator size stuff and stuff :slight_smile:

Last update: Nov 18 2019 at 01:30UTC