-Zfused-futures

[GrigorenkoPV]

Fixes (kinda) #147041

Introduces -Zfused-futures, a flag that makes compiler-generated futures return Poll::Pending instead of panicking when polled after completion.

[rustbot]

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

[rustbot]

r? @nnethercote

rustbot has assigned @nnethercote.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

[jieyouxu]

Introduces -Zfused-futures, a flag that makes compiler-generated futures return Poll::Pending instead of panicking when polled after completion.

This feels fishy as a compiler flag tbh

[nnethercote]

This feels fishy as a compiler flag tbh

I agree. I'm no expert on futures and coroutines, but this feels like a very quick and dirty partial solution to the problem, with low discoverability and no clear path to a cleaner, more general solution. Maybe panic = "abort" could be a model for how this might be solved... not sure. I think this needs discussion with lang and/or libs people.

[jieyouxu]

To elaborate, I would expect this to at least be visible somehow in the source language so it does not catch the reader completely off guard. I do not have a good grasp on what that would look like, perhaps an attribute, perhaps some other syntactical notation. The point is, however, with a compiler flag like this, we get completely different runtime semantics for the same source program. For instance, if you have some unsafe code which relies on the Future::poll not panicking (under -Zfused-futures), then if you compile the exact same source code without -Zfused-futures, suddenly your program exhibits UB.

This feels very surprising that I wouldn't even want to land this as-is as an unstable compiler flag. Especially given there's no way this present approach would be stabilized IMHO.

[nnethercote]

Does this need to be pub?

[GrigorenkoPV]

Probably not

[ADD-SP]

Casual observer here, I think returning Poll::Pending and hang-forever is not a good idea, this is equivalent to silent failure, which makes the trouble-shooting more difficult on complicated system.

View changes since this review

[GrigorenkoPV]

This feels fishy as a compiler flag tbh

Maybe a better way would be some sort of a global handler (hello, externally implementable items #125418), but at this point it feels a bit like reinventing panic!s.

Casual observer here, I think returning Poll::Pending and hang-forever is not a good idea, this is equivalent to silent failure, which makes the trouble-shooting more difficult on complicated system.

This is why it is opt-in.

---

Maybe something like a #[fused] attribute might work, but this would be partially blocked on stmt_expr_attributes ( #15701, damn, 11 years old).

[ZhekaS]

Maybe something like a #[fused] attribute might work, but this would be partially blocked on stmt_expr_attributes ( #15701, damn, 11 years old).

(FWIW I am the one who filed #147041 )

I feel like the attribute way would be cleaner and clearer as well. However I don't like either tbh. I have no idea how hard it is in terms of implementation and/or pushing it toward stabilization, but the attribute could make some more drastic behavioral changes, such as changing the async fn foo() -> T to generate impl Future<Output=Result<T, SomeError>> instead of impl Future<Output=T> and consequentially changing the panic block into returning Poll::Ready(Err(SomeError)). Surely this will have to be taken care of on the executor level or above.

[nnethercote]

I think opening a discussion on Zulip might be worthwhile for this topic.

[yoshuawuyts]

Currently the documentation of Poll::Pending reads:

Represents that a value is not ready yet. When a function returns Pending, the function must also ensure that the current task is scheduled to be awoken when progress can be made.

All futures to date have been written according to this definition. Changing that to now also capture "done" semantics is not backwards-compatible and will cause breakage in the ecosystem. Speaking as a maintainer of futures-concurrency: this would require us to go back and validate every future we have authored to check whether it would work under the newly proposed semantics. Not because we got something wrong, but because Rust changed the rules on us.

I believe the solution proposed here should be rejected on compatibility grounds. The fact that those semantics are hidden behind a flag obscures this somewhat; but if we make this an option people can toggle library authors will have to assume some people will enable it. And that means that code that works perfectly fine today will need to be updated because Rust changed its semantics. And that's the kind of breakage that Rust promises not to make.

[ADD-SP]

Casual observer here, I think returning Poll::Pending and hang-forever is not a good idea, this is equivalent to silent failure, which makes the trouble-shooting more difficult on complicated system.

This is why it is opt-in.

In my opinion, It may not be a good choice to introduce this opt-in flag that creates possibilities of silent failures.

[nnethercote]

It feels like there is consensus that this flag isn't the right solution to this problem, so I'm going to close it. Thanks for the attempt, @GrigorenkoPV! Starting Zulip discussions about an attribute might be a good path forward from here.