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

ICE on if let Some() = ... expression in loop with variable shadowing and missing let keyword #77218

Closed
zdenek-crha opened this issue Sep 26, 2020 · 10 comments · Fixed by #77283
Assignees
Labels
A-typesystem Area: The type system C-bug Category: This is a bug. glacier ICE tracked in rust-lang/glacier. I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️ P-high High priority regression-from-stable-to-beta Performance or correctness regression from stable to beta. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@zdenek-crha
Copy link

zdenek-crha commented Sep 26, 2020

Code

use std::collections::BTreeMap;

// Type aliases to replace complex data types used in the original code.
pub type Data = usize;
pub type Key = String;
pub type DataRef<'c> = &'c Data;

pub struct Storage(BTreeMap<Key, Data>);

impl Storage {
    pub fn list<'a>(&'a self, key: &Key) -> Vec<&Data> {
        unimplemented!()
    }
}

pub struct Cache<'c> {
    storage: Storage,
    cached_data_refs: BTreeMap<Key, DataRef<'c>>,
}

impl<'c> Cache<'c> {

    pub fn data_ref_list<'a>(&'a mut self, key: &Key) {
        for reference in self.storage.list(key) {
            self.cached_data_refs.insert(key.clone(), reference);

            // Following `if` statement is the reason for compiler crash. To trigger the crash it
            // is required to:
            //
            // * have `let` keyword missing from `if let Some(...)` expression
            // * have binding name in the `Some(...)` portion of the `if let Some()` shadow the
            //   name from the outer loop
            //
            if /*let*/ Some(reference) = self.cached_data_refs.get(key) {
                unimplemented!()
            }
        }
    }
}

fn main() {}

Meta

rustc --version --verbose:

rustc 1.48.0-nightly (043f6d747 2020-09-25)
binary: rustc
commit-hash: 043f6d747c15068f0053a0542e9b0f17ae7f4de4
commit-date: 2020-09-25
host: x86_64-unknown-linux-gnu
release: 1.48.0-nightly
LLVM version: 11.0

Error output

$ rustc bugtest.rs
error: internal compiler error: compiler/rustc_typeck/src/check/fn_ctxt.rs:459:25: while adjusting Expr { hir_id: HirI
d { owner: DefId(0:20 ~ bugtest[317d]::{{impl}}[1]::data_ref_list[0]), local_id: 70 }, kind: Path(Resolved(None, Path
{ span: bugtest.rs:34:42: 34:46 (#0), res: Local(HirId { owner: DefId(0:20 ~ bugtest[317d]::{{impl}}[1]::data_ref_list
[0]), local_id: 1 }), segments: [PathSegment { ident: self#0, hir_id: Some(HirId { owner: DefId(0:20 ~ bugtest[317d]::
{{impl}}[1]::data_ref_list[0]), local_id: 69 }), res: Some(Local(HirId { owner: DefId(0:20 ~ bugtest[317d]::{{impl}}[1
]::data_ref_list[0]), local_id: 1 })), args: None, infer_args: true }] })), attrs: ThinVec(None), span: bugtest.rs:34:
42: 34:46 (#0) }, can't compose [Deref(None) -> Cache<'c>] and [Deref(None) -> Cache<'c>]

thread 'rustc' panicked at 'Box<Any>', compiler/rustc_errors/src/lib.rs:945:9

note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
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-compil
er&template=ice.md
note: rustc 1.48.0-nightly (043f6d747 2020-09-25) running on x86_64-unknown-linux-gnu

error: aborting due to previous error
Backtrace

stack backtrace:
   0: std::panicking::begin_panic
   1: rustc_errors::HandlerInner::bug
   2: rustc_errors::Handler::bug
   3: rustc_middle::util::bug::opt_span_bug_fmt::{{closure}}
   4: rustc_middle::ty::context::tls::with_opt::{{closure}}
   5: rustc_middle::ty::context::tls::with_opt
   6: rustc_middle::util::bug::opt_span_bug_fmt
   7: rustc_middle::util::bug::bug_fmt
   8: rustc_typeck::check::fn_ctxt::FnCtxt::apply_adjustments
   9: rustc_typeck::check::expr::<impl rustc_typeck::check::fn_ctxt::FnCtxt>::check_field
  10: rustc_typeck::check::expr::<impl rustc_typeck::check::fn_ctxt::FnCtxt>::check_expr_kind
  11: rustc_typeck::check::expr::<impl rustc_typeck::check::fn_ctxt::FnCtxt>::check_expr_with_expectation
  12: rustc_typeck::check::expr::<impl rustc_typeck::check::fn_ctxt::FnCtxt>::check_expr_kind
  13: rustc_typeck::check::expr::<impl rustc_typeck::check::fn_ctxt::FnCtxt>::check_expr_with_expectation
  14: rustc_typeck::check::expr::<impl rustc_typeck::check::fn_ctxt::FnCtxt>::check_expr_coercable_to_type
  15: rustc_typeck::check::expr::<impl rustc_typeck::check::fn_ctxt::FnCtxt>::check_expr_kind
  16: rustc_typeck::check::expr::<impl rustc_typeck::check::fn_ctxt::FnCtxt>::check_expr_with_expectation
  17: rustc_typeck::check::expr::<impl rustc_typeck::check::fn_ctxt::FnCtxt>::check_expr_kind
  18: rustc_typeck::check::expr::<impl rustc_typeck::check::fn_ctxt::FnCtxt>::check_expr_with_expectation
  19: rustc_typeck::check::_match::<impl rustc_typeck::check::fn_ctxt::FnCtxt>::check_match
  20: rustc_typeck::check::expr::<impl rustc_typeck::check::fn_ctxt::FnCtxt>::check_expr_kind
  21: rustc_typeck::check::expr::<impl rustc_typeck::check::fn_ctxt::FnCtxt>::check_expr_with_expectation
  22: rustc_typeck::check::fn_ctxt::FnCtxt::check_block_with_expected
  23: rustc_typeck::check::expr::<impl rustc_typeck::check::fn_ctxt::FnCtxt>::check_expr_kind
  24: rustc_typeck::check::expr::<impl rustc_typeck::check::fn_ctxt::FnCtxt>::check_expr_with_expectation
  25: rustc_typeck::check::fn_ctxt::FnCtxt::check_stmt
  26: rustc_typeck::check::fn_ctxt::FnCtxt::check_block_with_expected
  27: rustc_typeck::check::expr::<impl rustc_typeck::check::fn_ctxt::FnCtxt>::check_expr_kind
  28: rustc_typeck::check::expr::<impl rustc_typeck::check::fn_ctxt::FnCtxt>::check_expr_with_expectation
  29: rustc_typeck::check::_match::<impl rustc_typeck::check::fn_ctxt::FnCtxt>::check_match
  30: rustc_typeck::check::expr::<impl rustc_typeck::check::fn_ctxt::FnCtxt>::check_expr_kind
  31: rustc_typeck::check::expr::<impl rustc_typeck::check::fn_ctxt::FnCtxt>::check_expr_with_expectation
  32: rustc_typeck::check::expr::<impl rustc_typeck::check::fn_ctxt::FnCtxt>::check_expr_kind
  33: rustc_typeck::check::expr::<impl rustc_typeck::check::fn_ctxt::FnCtxt>::check_expr_with_expectation
  34: rustc_typeck::check::fn_ctxt::FnCtxt::check_block_with_expected
  35: rustc_typeck::check::expr::<impl rustc_typeck::check::fn_ctxt::FnCtxt>::check_expr_kind
  36: rustc_typeck::check::expr::<impl rustc_typeck::check::fn_ctxt::FnCtxt>::check_expr_with_expectation
  37: rustc_typeck::check::expr::<impl rustc_typeck::check::fn_ctxt::FnCtxt>::check_return_expr
  38: rustc_typeck::check::check::check_fn
  39: rustc_infer::infer::InferCtxtBuilder::enter
  40: rustc_typeck::check::typeck
  41: rustc_middle::ty::query::<impl rustc_query_system::query::config::QueryAccessors<rustc_middle::ty::context::TyCt
xt> for rustc_middle::ty::query::queries::typeck>::compute
  42: rustc_query_system::dep_graph::graph::DepGraph<K>::with_task_impl
  43: rustc_data_structures::stack::ensure_sufficient_stack
  44: rustc_query_system::query::plumbing::get_query_impl
  45: rustc_query_system::query::plumbing::ensure_query_impl
  46: rustc_typeck::check::typeck_item_bodies
  47: rustc_middle::ty::query::<impl rustc_query_system::query::config::QueryAccessors<rustc_middle::ty::context::TyCt
xt> for rustc_middle::ty::query::queries::typeck_item_bodies>::compute
  48: rustc_query_system::dep_graph::graph::DepGraph<K>::with_task_impl
  49: rustc_data_structures::stack::ensure_sufficient_stack
  50: rustc_query_system::query::plumbing::get_query_impl
  51: rustc_typeck::check_crate
  52: rustc_interface::passes::analysis
  53: rustc_middle::ty::query::<impl rustc_query_system::query::config::QueryAccessors<rustc_middle::ty::context::TyCt
xt> for rustc_middle::ty::query::queries::analysis>::compute
  54: rustc_query_system::dep_graph::graph::DepGraph<K>::with_task_impl
  55: rustc_data_structures::stack::ensure_sufficient_stack
  56: rustc_query_system::query::plumbing::get_query_impl
  57: rustc_interface::passes::QueryContext::enter
  58: rustc_interface::queries::<impl rustc_interface::interface::Compiler>::enter
  59: rustc_span::with_source_map
  60: rustc_interface::interface::create_compiler_and_run
  61: scoped_tls::ScopedKey<T>::set

@zdenek-crha zdenek-crha 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 26, 2020
@jonas-schievink jonas-schievink added A-typesystem Area: The type system I-prioritize Issue: Indicates that prioritization has been requested for this issue. labels Sep 26, 2020
@Stupremee Stupremee added the regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. label Sep 26, 2020
@jyn514 jyn514 added the E-needs-bisection Call for participation: This issue needs bisection: https://github.com/rust-lang/cargo-bisect-rustc label Sep 26, 2020
@Stupremee
Copy link
Member

Regression in d824b2351449714dc685d90e298c9d630ad6c437

Regressed in #75931

@Stupremee Stupremee removed the E-needs-bisection Call for participation: This issue needs bisection: https://github.com/rust-lang/cargo-bisect-rustc label Sep 26, 2020
@camelid
Copy link
Member

camelid commented Sep 26, 2020

Cc @estebank

@camelid
Copy link
Member

camelid commented Sep 26, 2020

Related: this code should probably have a suggestion for if let (playground):

fn foo() {
    let list = vec![Some(1), None, Some(3)];
    
    for reference in list {
        if /*let*/ Some(reference) = reference {
            unimplemented!()
        }
    }
}

Currently it produces:

error[E0308]: mismatched types
 --> src/lib.rs:5:38
  |
5 |         if /*let*/ Some(reference) = reference {
  |                                      ^^^^^^^^^
  |                                      |
  |                                      expected enum `Option`, found integer
  |                                      help: try using a variant of the expected enum: `Some(reference)`
  |
  = note: expected enum `Option<Option<{integer}>>`
             found enum `Option<{integer}>`

error[E0308]: mismatched types
 --> src/lib.rs:5:20
  |
5 |         if /*let*/ Some(reference) = reference {
  |                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected `bool`, found `()`

error: aborting due to 2 previous errors

@camelid
Copy link
Member

camelid commented Sep 26, 2020

Minimized (playground):

// Type aliases to replace complex data types used in the original code.
pub type Data = usize;
pub type Key = usize;
pub type DataRef<'c> = &'c Data;

pub struct Cache<'c> {
    cached_data_refs: Vec<DataRef<'c>>,
}

impl Cache<'_> {
    pub fn data_ref_list(&mut self, key: &Key) {
        for reference in vec![1, 2, 3] {
            // Following `if` statement is the reason for compiler crash. To trigger the crash it
            // is required to:
            //
            // * have `let` keyword missing from `if let Some(...)` expression
            // * have binding name in the `Some(...)` portion of the `if let Some()` shadow the
            //   name from the outer loop
            //
            if /*let*/ Some(reference) = self.cached_data_refs.get(*key) {
                unimplemented!()
            }
        }
    }
}

I think this might be caused by the lifetimes.

@camelid
Copy link
Member

camelid commented Sep 26, 2020

Never mind! This causes it too, with no lifetimes (playground):

pub struct Cache {
    data: Vec<i32>,
}

impl Cache {
    pub fn list_data(&mut self, key: &usize) {
        for reference in vec![1, 2, 3] {
            if /*let*/ Some(reference) = self.data.get(*key) {
                unimplemented!()
            }
        }
    }
}

EDIT: removed the type aliases

EDIT 2: changed Vec<usize> to Vec<i32> to make it clearer that they don't need to be the same type

EDIT 3: shortened names, removed comment

@camelid
Copy link
Member

camelid commented Sep 26, 2020

Assigning P-high and removing I-prioritize as discussed in the prioritization working group.

@camelid camelid added P-high High priority and removed I-prioritize Issue: Indicates that prioritization has been requested for this issue. labels Sep 26, 2020
@camelid
Copy link
Member

camelid commented Sep 26, 2020

Further minimized to this (playground):

pub struct Cache {
    data: Vec<i32>,
}

pub fn list_data(cache: &Cache, key: usize) {
    for reference in vec![1, 2, 3] {
        if /*let*/ Some(reference) = cache.data.get(key) {
            unimplemented!()
        }
    }
}

Having cache be a &Cache or &mut Cache produces the ICE, but making it an owned Cache does not produce the ICE.

@SNCPlay42
Copy link
Contributor

#75931 causes the operands to be typechecked twice:

let lhs_ty = self.check_expr(&lhs);
let rhs_ty = self.check_expr(&rhs);

// Do this to cause extra errors about the assignment.
let lhs_ty = self.check_expr_with_needs(&lhs, Needs::MutPlace);
let _ = self.check_expr_coercable_to_type(&rhs, lhs_ty, Some(lhs));

I think this has side effects that are causing problems by being applied twice. Commenting out the second check_expr(_with_needs/coercable_to_type) makes the ICE go away, but I'm not sure what the best approach for a proper fix is. The effect on the UI suite is rather low:

27      error[E0308]: mismatched types
-         --> $DIR/if-let-typo.rs:6:12
-          |
-       LL |     if 3 = foo {}
-          |            ^^^ expected integer, found enum `Option`
-          |
-          = note: expected type `{integer}`
-                     found enum `Option<{integer}>`
-
-       error[E0308]: mismatched types

(It still emits this error:)

error[E0308]: mismatched types
  --> $DIR/if-let-typo.rs:6:8
   |
LL |     if 3 = foo {} //~ ERROR mismatched types
   |        ^^^^^^^ expected `bool`, found `()`

@estebank estebank self-assigned this Sep 27, 2020
@estebank
Copy link
Contributor

@SNCPlay42 I tried a couple of things but it seems to me that you hit the nail in the head and we just need to remove the else branch.

I think this might be caused by the lifetimes.

@camelid this is caused because one side is seen to be Option<_> while the other is seen to be Option<&_> or &Option<_> (depending on the case). This is a problem because the left hand side expression should in reality be a pattern instead, but there's no utility to build a pattern from an expression: patterns are built during parsing (due to differences like, ref being a valid thing there). Because of that it'll be very hard to make the suggestion happen for those cases, much harder than for the ones we already handle.

@chrysn
Copy link
Contributor

chrysn commented Oct 1, 2020

I think I just found a smaller example (granted, it's not valid code any more, but how is one supposed to know that in case of ICE? ;-) ):

fn main() {
    let value = [7u8];
    while Some(0) = value.get(0) {
    }
}

(If that's not the same error I'm happy to open a new ICE issue, but to me it looks the same, with all the while adjusting Expr { hir_id: ..., the panic at Box<Any> and the conditional destructuring in a loop).

@estebank estebank added regression-from-stable-to-beta Performance or correctness regression from stable to beta. and removed regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. labels Oct 20, 2020
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue Oct 26, 2020
…crum

Tweak `if let` suggestion to be more liberal with suggestion and to not ICE

Fix rust-lang#77218. Fix rust-lang#77238.
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue Oct 26, 2020
…crum

Tweak `if let` suggestion to be more liberal with suggestion and to not ICE

Fix rust-lang#77218. Fix rust-lang#77238.
bors added a commit to rust-lang-ci/rust that referenced this issue Oct 26, 2020
Tweak `if let` suggestion to be more liberal with suggestion and to not ICE

Fix rust-lang#77218. Fix rust-lang#77238.
@bors bors closed this as completed in cabf6d0 Oct 26, 2020
Mark-Simulacrum pushed a commit to Mark-Simulacrum/rust that referenced this issue Nov 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-typesystem Area: The type system C-bug Category: This is a bug. glacier ICE tracked in rust-lang/glacier. I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️ P-high High priority regression-from-stable-to-beta Performance or correctness regression from stable to beta. 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.

9 participants