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 with if let chain #99938

Closed
JustForFun88 opened this issue Jul 30, 2022 · 10 comments · Fixed by #104771
Closed

Broken MIR with if let chain #99938

JustForFun88 opened this issue Jul 30, 2022 · 10 comments · Fixed by #104771
Labels
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. 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.

Comments

@JustForFun88
Copy link

Recently I played with the newly stabilized if let chain (tracker for issue #53667, stabilized in Rust 1.64 #94927) and found that in some cases it gives internal compiler error: broken MIR . Below is a minimal example. It compiles fine with cargo build or cargo run, but does not compile with cargo build --release. It also compiles if you replace if let chain with nested if let.

Code

struct TupleIter<T, I: Iterator<Item = T>> {
    inner: I,
}

impl<T, I: Iterator<Item = T>> Iterator for TupleIter<T, I> {
    type Item = (T, T, T);

    fn next(&mut self) -> Option<Self::Item> {
        let inner = &mut self.inner;

        // Error with if_let_chain
        if let Some(first) = inner.next()
            && let Some(second) = inner.next()
            && let Some(third) = inner.next()
        {
            Some((first, second, third))
        } else {
            None
        }

        // Ok we use nested if_let
        // if let Some(first) = inner.next() {
        //     if let Some(second) = inner.next() {
        //         if let Some(third) = inner.next() {
        //             Some((first, second, third))
        //         } else {
        //             None
        //         }
        //     } else {
        //         None
        //     }
        // } else {
        //     None
        // }
    }
}

fn main() {
    let vec: Vec<u8> = Vec::new();
    let mut tup_iter = TupleIter {
        inner: vec.into_iter(),
    };
    tup_iter.next();
}

Meta

rustc --version --verbose:

rustc 1.64.0-nightly (4d6d601c8 2022-07-26)
binary: rustc
commit-hash: 4d6d601c8a83284d6b23c253a3e2a060fd197316
commit-date: 2022-07-26
host: x86_64-pc-windows-msvc
release: 1.64.0-nightly
LLVM version: 14.0.6

Error output

cargo build --release
error: internal compiler error: no errors encountered even though `delay_span_bug` issued                                 

error: internal compiler error: broken MIR in Item(WithOptConstParam { did: DefId(0:11 ~ if_let_chain[af92]::{impl#0}::next), const_param_did: None }) (end of phase transition to Optimized) at bb22[0]:
                                use of local _14, which has no storage here
  --> src\main.rs:17:9
   |
17 |         } else {
   |         ^
   |
   = note: delayed at compiler\rustc_const_eval\src\transform\validate.rs:129:36

thread 'rustc' panicked at 'Box<dyn Any>', compiler\rustc_errors\src\lib.rs:1426:13
stack backtrace:
   0:     0x7ff92c1e9c5f - <std::sys_common::backtrace::_print::DisplayBacktrace as core::fmt::Display>::fmt::h22e7b784ac716ba6
   1:     0x7ff92c2255ba - core::fmt::write::h710985228ff64dc4
   2:     0x7ff92c1dcada - <std::io::IoSliceMut as core::fmt::Debug>::fmt::h7a965e27de439674
   3:     0x7ff92c1ed65b - std::panicking::default_hook::h59e772df96aabd4c
   4:     0x7ff92c1ed287 - std::panicking::default_hook::h59e772df96aabd4c
   5:     0x7ff915f26b81 - <rustc_session[9ee3cdee11a5ff9c]::options::WasiExecModel as rustc_session[9ee3cdee11a5ff9c]::config::dep_tracking::DepTrackingHash>::hash
   6:     0x7ff92c1ee03c - std::panicking::rust_panic_with_hook::h38636381814ab6ea
   7:     0x7ff918147cf3 - <rustc_errors[76a7f7b80221f39a]::annotate_snippet_emitter_writer::AnnotateSnippetEmitterWriter>::ui_testing
   8:     0x7ff918147a99 - <rustc_errors[76a7f7b80221f39a]::annotate_snippet_emitter_writer::AnnotateSnippetEmitterWriter>::ui_testing
   9:     0x7ff91813d329 - <rustc_errors[76a7f7b80221f39a]::emitter::FileWithAnnotatedLines as core[9a2c4b0c2bb38f26]::fmt::Debug>::fmt
  10:     0x7ff9181316b9 - <rustc_feature[b39b46621b392478]::builtin_attrs::AttributeType as core[9a2c4b0c2bb38f26]::fmt::Debug>::fmt
  11:     0x7ff9147a943d - <rustc_errors[76a7f7b80221f39a]::HandlerInner as core[9a2c4b0c2bb38f26]::ops::drop::Drop>::drop  12:     0x7ff913919b9b - <rustc_middle[d6f72711bda4db03]::ty::SymbolName as core[9a2c4b0c2bb38f26]::fmt::Display>::fmt  
  13:     0x7ff91391cabf - <rustc_middle[d6f72711bda4db03]::ty::SymbolName as core[9a2c4b0c2bb38f26]::fmt::Display>::fmt  
  14:     0x7ff91390aced - <unknown>
  15:     0x7ff9138f73e7 - <unknown>
  16:     0x7ff91392e937 - rustc_driver[197bfed8d57a407b]::args::arg_expand_all
  17:     0x7ff913917cf9 - <unknown>
  18:     0x7ff9139187fd - <rustc_middle[d6f72711bda4db03]::ty::SymbolName as core[9a2c4b0c2bb38f26]::fmt::Display>::fmt  
  19:     0x7ff92c1ff15c - std::sys::windows::thread::Thread::new::hec2508d69eebdd0a
  20:     0x7ff993187034 - BaseThreadInitThunk
  21:     0x7ff9934c2651 - RtlUserThreadStart

note: the compiler unexpectedly panicked. this is a bug.

note: we would appreciate a bug report: https://github.com/rust-lang/rust/issues/new?labels=C-bug%2C+I-ICE%2C+T-compiler&template=ice.md

note: rustc 1.64.0-nightly (4d6d601c8 2022-07-26) running on x86_64-pc-windows-msvc

note: compiler flags: --crate-type bin -C opt-level=3 -C embed-bitcode=no

note: some of the compiler flags provided by cargo are hidden

query stack during panic:
end of query stack
error: could not compile `if_let_chain`
@JustForFun88 JustForFun88 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 Jul 30, 2022
@Nilstrieb
Copy link
Member

Thank you for you report, but this is probably a duplicate of #99852

@Lee-Janggun
Copy link
Contributor

Can confirm this no longer ICEs on most recent nightly:

https://play.rust-lang.org/?version=nightly&mode=release&edition=2021&gist=ca82d8f6583d900c67af8e1784a882f1

@Rageking8
Copy link
Contributor

@rustbot label +E-needs-test

@rustbot rustbot 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 15, 2022
@est31
Copy link
Member

est31 commented Oct 15, 2022

Yeah likely this was fixed by #102394 . I wonder if it's worth adding a test for this, or it can just be closed as a dupe of #99852.

@flip1995
Copy link
Member

We just hit this during the Clippy sync in #104688 (comment)

The offending code was this:

if cfg!(debug_assertions)
&& let Ok(current_exe) = env::current_exe()
&& let Some(current_exe) = current_exe.to_str()
{
file_depinfo.insert(Symbol::intern(current_exe));
}

Pulling the cfg! if line out of the chain fixed the issue, see 5e6a9a4, so this works:

if cfg!(debug_assertions) {
if let Ok(current_exe) = env::current_exe()
&& let Some(current_exe) = current_exe.to_str()
{
file_depinfo.insert(Symbol::intern(current_exe));
}
}

@est31
Copy link
Member

est31 commented Nov 22, 2022

@flip1995 your issue is different, can you file a bug for it? This issue does not reproduce any more on latest nightly, it should be closed (or a test added for it before closure, but #102394 had tests already.

@flip1995
Copy link
Member

Hmm, I can do this, but I think that is actually the same issue. While the above reproducer might be fixed, the fix doesn't seem to generally apply. The above code produced the exact same ICE/error message as posted in this issue.

@est31
Copy link
Member

est31 commented Nov 22, 2022

@flip1995 I think this might be some MIR optimization gone wrong, maybe caused by wrong codegen, but your issue requires optimizations to be turned on, it can't just be reproduced by -Zvalidate-mir.

The original ICE was also reproducible by -Zvalidate-mir (as well as with -O, as well as with both params), I've just verified using nightly-2022-07-30. I would thus recommend you file a new issue, but your choice.

Some additional info to your issue, this is a reproducer:

#![feature(let_chains)]

pub fn foo() {
    if false
        && let Ok(ce) = std::env::current_exe()
        && let Some(ce) = ce.to_str()
    {
        let _ = ce.len();
    }
}

Interestingly, the issue is a regression, as it didn't ICE on nightly-2022-07-30.

Edit: I very much appreciate the notion of not wanting to file multiple issues, after all this very issue is a dupe, but the clippy bug is separate.

@Nilstrieb
Copy link
Member

This isn't because of MIR opts, it only happens under --release because storage markers, which are needed to get this ICE, are only emitted in optimized builds.

@est31
Copy link
Member

est31 commented Nov 22, 2022

@Nilstrieb is this storage markers emitting only in release a new feature? Because I can reproduce the ICE with the original reproducer from this issue's author, with nightly-2022-07-30 and -Zvalidate-mir and no -O flag whatsoever.

Dylan-DPC added a commit to Dylan-DPC/rust that referenced this issue Nov 24, 2022
…, r=davidtwco

Add regression test for issue rust-lang#99938

That issue was a dupe of rust-lang#99852, and it got fixed since, but it's always better to have multiple regression tests rather than one.

closes rust-lang#99938
bors added a commit to rust-lang-ci/rust that referenced this issue Nov 24, 2022
…iaskrgr

Rollup of 9 pull requests

Successful merges:

 - rust-lang#103908 (Suggest `.clone()` or `ref binding` on E0382)
 - rust-lang#104517 (Throw error on failure in loading llvm-plugin)
 - rust-lang#104594 (Properly handle `Pin<&mut dyn* Trait>` receiver in codegen)
 - rust-lang#104742 (Make `deref_into_dyn_supertrait` lint the impl and not the usage)
 - rust-lang#104753 (Pass `InferCtxt` to `DropRangeVisitor` so we can resolve vars)
 - rust-lang#104771 (Add regression test for issue rust-lang#99938)
 - rust-lang#104772 (Small accessibility improvements)
 - rust-lang#104775 (Use ObligationCtxt::normalize)
 - rust-lang#104778 (:arrow_up: rust-analyzer)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors closed this as completed in 84ff4ab Nov 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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. 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.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants