Code no longer compiles after -Zdrop-tracking-mir was enabled by default

[tmandry]

Some code that used to compile no longer does after #107421. We don't think was ever supposed to compile. Don't have an MCVE yet.

Snippet:

let selecting_fut = selecting
    .do_selecting(
        &client_config,
        &test_socket_provider,
        &mut rng,
        &time,
        &mut stop_receiver,
    )
    .fuse();

let time = &time;

let wait_fut = async {
    // Wait some arbitrary amount of time to ensure `do_selecting` is waiting on a reply.
    // Note that this is fake time, not 30 actual seconds.
    time.wait_until(std::time::Duration::from_secs(30)).await;
}
.fuse();

pin_mut!(selecting_fut, wait_fut);

let main_future = async {
    select! {
        _ = selecting_fut => unreachable!("should keep retransmitting DHCPDISCOVER forever"),
        () = wait_fut => (),
    }
};
pin_mut!(main_future);

run_with_accelerated_time(&mut executor, time, &mut main_future);

stop_sender.unbounded_send(()).expect("sending stop signal should succeed");

let selecting_result = selecting_fut.now_or_never().expect(
    "selecting_fut should complete after single poll after stop signal has been sent",
);

assert_matches!(selecting_result, Ok(SelectingOutcome::GracefulShutdown));

Full code is here: https://cs.opensource.google/fuchsia/fuchsia/+/main:src/connectivity/network/dhcpv4/client/core/src/client.rs;l=1521-1576;drc=4a7d2eb6793e61c0021f8dd7dab35804590d36b6

I expected to see this happen: compiles as before

Instead, this happened:

error[E0505]: cannot move out of `selecting_fut` because it is borrowed
    --> ../../src/connectivity/network/dhcpv4/client/core/src/client.rs:1571:32
     |
1559 |           let main_future = async {
     |  ___________________________-
1560 | |             select! {
1561 | |                 _ = selecting_fut => unreachable!("should keep retransmitting DHCPDISCOVER forever"),
     | |                     ------------- borrow occurs due to use in generator
1562 | |                 () = wait_fut => (),
1563 | |             }
1564 | |         };
     | |_________- borrow of `selecting_fut` occurs here
...
1571 |           let selecting_result = selecting_fut.now_or_never().expect(
     |                                  ^^^^^^^^^^^^^ move out of `selecting_fut` occurs here
...
1576 |       }
     |       - borrow might be used here, when `main_future` is dropped and runs the destructor for generator

error: aborting due to previous error

For more information about this error, try `rustc --explain E0505`.

Version it worked on

It most recently worked on: Nightly 2023-09-22

@rustbot modify labels: +regression-from-stable-to-nightly -regression-untriaged

[Noratrieb]

@cjgillot

[jamuraa]

I've made a minimized version of this using tokio which confirmed the behavior of passing on the stable compiler and failing to compile on nightly in the playground:

use futures::pin_mut;
use futures::select;
use futures::FutureExt;

fn main() {
    let rt = tokio::runtime::Runtime::new().unwrap();
   
    let (send, recv) = futures::channel::oneshot::channel::<()>();
    let ready_fut = futures::future::ready(());
    
    pin_mut!(ready_fut, recv);
    let combined_fut = async {
        select! {
            _ = recv => unreachable!("should not send"),
            () = ready_fut => (),
        }
    };
    pin_mut!(combined_fut);
    let _ = rt.block_on(combined_fut);
   
    send.send(()).unwrap(); 
    assert_eq!(Some(Ok(())), recv.now_or_never());
}

[cjgillot]

Thanks for the repro @jamuraa.

What changed is the the dropping of combined_fut. Stable is able to determine that combined_fut does not need dropping, as it only contains a PollFn<some closure>. Nightly supposes that all generators need dropping to avoid cycles when building MIR.

I'm not sure how this can be fixed.

[apiraino]

Nominating for the next T-compiler meeting. I'd like to get a feeling of the priority and actionables.

@rustbot label +I-compiler-nominated

[wesleywiser]

During the compiler team triage meeting, @jackh726 said:

We should probably just nominate it for T-types ... to get some idea if there is something we can do to mitigate the breakage

so nominating for T-types as well.

[apiraino]

T-compiler discussed the issue: in addition to adding T-types to the discussion, we defer until next week to see the results of the crater run for #114624 that is solved by MIR drop tracking, too. Crater run is in progress at #116567.

(Zulip link)

[nikomatsakis]

We discussed this in the @rust-lang/types meeting today.

@lcnr is going to mentor @spastorino to prepare a fix by modifying the drop outlives code for generators to be more accurate.

[nikomatsakis]

@rustbot assign @spastorino

[traviscross]

@rustbot labels +AsyncAwait-Triaged

We discussed this in a WG-async meeting and are leaving this in the capable hands of T-types for resolution.

[lqd]

Reduced

use std::future;

fn main() {
    let mut recv = future::ready(());
    let _combined_fut = async {
        let _ = || read(&mut recv);
    };

    drop(recv);
}

fn read<F: future::Future>(_: &mut F) -> F::Output {
    todo!()
}

You could likely inline std::future::Ready if you wanted to, but this seems self-contained enough for a test?

[apiraino]

Briefly mentioned this one during T-compiler meeting (Zulip notes).

Leaving this as I-compiler-nominated and tracking backport of #117134. We would like to see this fixed in the next stable release (on 2023-11-16)

[wesleywiser]

Reopening to track beta-backport.

[apiraino]

Removing the T-compiler nomination as the backport of #117134 was approved (Zulip link).

@rustbot label -I-compiler-nominated

[apiraino]

@tmandry when you have a chance can you confirm if the issue is solved, now that #117134 has been merged? Feel free to close this issue if it's the case. thanks 👍

[Nashenas88]

I can confirm that the failing code now compiles on a recent rust build (revision d97bb19).

[lqd]

Great, thank you!