Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Broken MIR: generator contains type ... after copy & destination propagation #76375

Closed
tmiasko opened this issue Sep 5, 2020 · 4 comments · Fixed by #78133 or #79027
Closed

Broken MIR: generator contains type ... after copy & destination propagation #76375

tmiasko opened this issue Sep 5, 2020 · 4 comments · Fixed by #78133 or #79027
Labels
A-coroutines Area: Coroutines A-mir-opt Area: MIR optimizations C-bug Category: This is a bug. E-needs-test Call for participation: An issue has been fixed and does not reproduce, but no test has been added. glacier ICE tracked in rust-lang/glacier. I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️ requires-nightly This issue requires a nightly compiler in some way. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@tmiasko
Copy link
Contributor

tmiasko commented Sep 5, 2020

x.rs:

#![crate_type = "lib"]

#[inline(always)]
pub fn f(s: bool) -> String {
    let a = "Hello world!".to_string();
    let b = a;
    let c = b;
    if s {
        c
    } else {
        String::new()
    }
}

y.rs

#![crate_type = "lib"]

pub async fn g() {
    x::f(true);
    h().await;
}

pub async fn h() {}
$ rustc --edition=2018 -Zmir-opt-level=2 x.rs -Zunsound-mir-opts x.rs
$ rustc --edition=2018 -Zmir-opt-level=2 y.rs --extern x -L.
error: internal compiler error: src/librustc_mir/transform/generator.rs:752:13: Broken MIR: generator contains type std::string::String in MIR, but typeck only knows about {std::future::ResumeTy, impl std::future::Future, ()}

The copy propagation removes storage markers during optimization. Afterwards state transforms considers them to be always live across yield. Due to pass ordering, requires inliner to expose the issue.

@tmiasko tmiasko added C-bug Category: This is a bug. I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️ T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Sep 5, 2020
@jonas-schievink jonas-schievink added A-coroutines Area: Coroutines A-mir-opt Area: MIR optimizations requires-nightly This issue requires a nightly compiler in some way. labels Sep 5, 2020
@tmiasko tmiasko changed the title Broken MIR: generator contains type ... after copy propagation Broken MIR: generator contains type ... after copy & destination propagation Sep 21, 2020
@rust-lang-glacier-bot rust-lang-glacier-bot added the glacier ICE tracked in rust-lang/glacier. label Sep 24, 2020
@JohnTitor
Copy link
Member

The ICE is no longer reproduced with the latest nightly on glacier, marking as E-needs-test.

@JohnTitor JohnTitor added the E-needs-test Call for participation: An issue has been fixed and does not reproduce, but no test has been added. label Sep 29, 2020
@tmiasko
Copy link
Contributor Author

tmiasko commented Sep 29, 2020

For copy propagation this is still an issue, but the test case now requires -Zunsound-mir-opts.
For destination propagation the test case was fixed after changes that stop propagation if either local was ever borrowed. Hard to say if this fixed the issue more generally.

@JohnTitor
Copy link
Member

@tmiasko Thanks for the heads-up!

Submitted a PR to readd this: rust-lang/glacier#479

@JohnTitor JohnTitor removed the E-needs-test Call for participation: An issue has been fixed and does not reproduce, but no test has been added. label Sep 30, 2020
@JohnTitor
Copy link
Member

Triage: It's now compiled fine (again) with the latest nightly (I'd say it's finally fixed by #77306).

@JohnTitor JohnTitor added the E-needs-test Call for participation: An issue has been fixed and does not reproduce, but no test has been added. label Oct 19, 2020
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this issue Oct 20, 2020
Add some MIR-related regression tests

Closes rust-lang#68841
Closes rust-lang#75053
Closes rust-lang#76375
Closes rust-lang#77911

I think they're fixed by rust-lang#77306.
@bors bors closed this as completed in a8a424f Oct 20, 2020
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue Nov 17, 2020
…r=oli-obk

Limit storage duration of inlined always live locals

Closes rust-lang#76375.
m-ou-se added a commit to m-ou-se/rust that referenced this issue Nov 17, 2020
…r=oli-obk

Limit storage duration of inlined always live locals

Closes rust-lang#76375.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-coroutines Area: Coroutines A-mir-opt Area: MIR optimizations C-bug Category: This is a bug. E-needs-test Call for participation: An issue has been fixed and does not reproduce, but no test has been added. glacier ICE tracked in rust-lang/glacier. I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️ requires-nightly This issue requires a nightly compiler in some way. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
4 participants