Drop tracking ICE involving await in left hand side of assignment (-Zdrop-tracking)

This was found during the most recent crater run for #97334. The test case below is reduced from https://github.com/haydnv/tinychain.

pub struct File<B> {
    block: B,
}

pub async fn commit<B: Clone>(this: &mut File<B>) {
    async {}.await;
    async {}.await;
    async {}.await;
    async {}.await;

    let file = async { &this }.await;
    *async { &mut this.block }.await = file.block.clone();
}

This ICEs with the following message:

error: internal compiler error: compiler/rustc_mir_transform/src/generator.rs:760:13: Broken MIR: generator contains type B in MIR, but typeck only knows about {ResumeTy, &mut File<B>, impl Future<Output = ()>, (), impl Future<Output = ()>, impl Future<Output = ()>, impl Future<Output = ()>, impl Future<Output = &&mut File<B>>, &&mut File<B>, File<B>, impl Future<Output = &mut B>} and [&mut File<B>]
  --> src/lib.rs:5:51
   |
5  |   pub async fn commit<B: Clone>(this: &mut File<B>) {
   |  ___________________________________________________^
6  | |     async {}.await;
7  | |     async {}.await;
8  | |     async {}.await;
...  |
12 | |     *async { &mut this.block }.await = file.block.clone();
13 | | }
   | |_^

My guess is that the B that MIR and typeck disagree on is the one produced by file.block.clone().

Weirdly, if any of the async {}.await lines are removed, the program compiles successfully.

@rustbot

@rustbot label +T-compiler +C-bug

@rustbot

@rustbot label +AsyncAwait-Triaged

@rustbot

As discussed on Zulip I will try to make some report tomorrow regarding this issue!

@rustbot claim

After playing a little bit with this today, as posted also on Zulip I can have a thesis that can make sense, I take a look to the following crates report https://crater-reports.s3.amazonaws.com/pr-97334-1/try%23b755f8b9936d084e0363ce6b393c7e444a37080e/gh/88IO.blogpost-rs/log.txt

where the message crash is the following one

Broken MIR: generator contains type CurrentUser in MIR, but typeck only knows about {ResumeTy, &Handler, serenity::prelude::Context, serenity::model::channel::Message, impl std::future::Future<Output = CurrentUser>, (), bool, String, Arc<serenity::http::Http>, impl std::future::Future<Output = Result<serenity::model::channel::Message, SerenityError>>, Handler, Option<regex::Captures>, regex::Captures, u64, Token, impl std::future::Future<Output = Result<Response<egg_mode::tweet::Tweet>, egg_mode::error::Error>>, impl std::future::Future<Output = Result<Reaction, SerenityError>>, &DraftTweet, DraftTweet, impl std::future::Future<Output = Result<Response<egg_mode::tweet::Tweet>, egg_mode::error::Error>>, Response<egg_mode::tweet::Tweet>, impl std::future::Future<Output = Result<serenity::model::channel::Message, SerenityError>>} and [&Handler, serenity::prelude::Context, serenity::model::channel::Message]

Where the MIR known type does not know the type CurrentUser but knows the type impl std::future::Future<Output = CurrentUser>, I think this was a regression right? the MIR should know also CurrentUser or vice versa the future need to be unwrapped to CurrentUser!

Yeah, so for some more background, this bug is with a new generator capture analysis called Drop Tracking. Drop tracking is more precise than the current analysis, which means that it finds fewer types in the generator type than before (basically, the current analysis has a lot of false positives and drop tracking has less of them). This analysis happens during type checking, and then the code is lowered to MIR. In MIR a similar analysis is done to find the values that need to be captured as part of the generator state. The MIR analysis is also more conservative than in strictly needs to be (it also has false positives), so sometimes we have to make drop tracking less aggressive than it could be to match the MIR analysis. We've had a lot of bugs like that so far, so I would expect that's what's going on here. That said, it could also be that the drop tracking analysis is bad and is missing something that it actually should be finding.

Minimal Exaple to reproduce it

pub struct File<B> {
    block: B,
}

pub async fn commit<B: Clone>(this: &mut File<B>) {
    async {}.await;
    async {}.await;
    async {}.await;
    async {}.await;

    let file = async { &this }.await;
    *async { &mut this.block }.await = file.block.clone();
}

fn main() {
    let mut file = File {
        block: String::new(),
    };
    let _ = async move {
        commit(&mut file).await;
    };
}

@rustbot

@rustbot claim

@eholk

@eholk has this been fixed? Can you still reproduce this ice?

This issue still reproduces with -Zdrop-tracking.

I found another instance of this:

use std::collections::HashMap;

fn main() {
    let _ = real_main();
}

async fn nop() {}

async fn real_main() {
    nop().await;
    nop().await;
    nop().await;
    nop().await;

    let mut map: HashMap<(), ()> = HashMap::new();
    map.insert((), nop().await);
}

Once again we have the four load-bearing nops. I'm guessing this issue is more generally about having four or more awaits followed by some mutable borrow being held across another await.

@rustbot

@rustbot claim

I'm going to take a look at fixing this.

Okay, I made some progress this afternoon so I wanted to update here.

The problem is that we refer to yields by where they come up in a post-order traversal of the HIR tree, but we have some disagreement in how they are counted. For a loop that gets desugared from a .await, cfg_build.rs counts one less expression than the similar calculation in region.rs. This is why the bug only shows up after having four useless awaits, since that's how far we need the counts to drift so we'll have different liveness information.

I've started a branch that will eventually have a fix, but for now I've just added some more debug logging and assertions to detect this drift better.

I looked for places in region.rs where we change expr_and_pat_count to find cases that we might not be matching in cfg_build.rs. I noticed we have some save and restore logic when handling bodies that we don't have cfg_build.rs. Tomorrow I plan to look into this some more to figure out what it's doing and how we should match that logic.

Rollup merge of rust-lang#106699 - eholk:await-chains-drop-tracking, …

Navigation Menu

Repository navigation

Drop tracking ICE involving await in left hand side of assignment (`-Zdrop-tracking`) #102383

4 remaining items

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Drop tracking ICE involving await in left hand side of assignment (-Zdrop-tracking) #102383

Description

Activity

Rageking8 commented on Oct 3, 2022

eholk commented on Oct 10, 2022

vincenzopalazzo commented on Oct 10, 2022

vincenzopalazzo commented on Oct 11, 2022

eholk commented on Oct 11, 2022

vincenzopalazzo commented on Oct 14, 2022

bryangarza commented on Nov 10, 2022

4 remaining items

matthiaskrgr commented on Dec 19, 2022

compiler-errors commented on Dec 19, 2022

eholk commented on Jan 9, 2023

eholk commented on Jan 9, 2023

eholk commented on Jan 10, 2023

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions

Drop tracking ICE involving await in left hand side of assignment (`-Zdrop-tracking`) #102383