Use of unimplemented!() causing ICE with NLL

[dylanede]

Here is a fairly small test case (playground):

#![feature(nll)]
fn main() {
    use std::collections::HashMap;
    let mut m: HashMap<(), ()> = HashMap::new();
    m.insert((), unimplemented!());
}

The error message is

thread 'main' panicked at 'assertion failed: `(left == right)`
  left: `Some((_4, bw0))`,
 right: `None`: never found an activation for this borrow!', librustc_mir/borrow_check/borrow_set.rs:121:9

[DutchGhost]

Its not only with unimplemented!():

using continue, or return instead of break results in the same error message.
https://play.rust-lang.org/?gist=49af6d88f86f4e517e1b0f2a9c671993&version=nightly&mode=debug

#![feature(nll)]

fn main() {
    let mut v = Vec::new();
    
    loop { v.push(break) }
}

 Compiling playground v0.0.1 (file:///playground)
warning: unreachable expression
 --> src/main.rs:6:12
  |
6 |     loop { v.push(break) }
  |            ^^^^^^^^^^^^^
  |
  = note: #[warn(unreachable_code)] on by default

thread 'main' panicked at 'assertion failed: `(left == right)`
  left: `Some((_3, bw0))`,
 right: `None`: never found an activation for this borrow!', librustc_mir/borrow_check/borrow_set.rs:121:9
note: Run with `RUST_BACKTRACE=1` for a backtrace.

error: internal compiler error: unexpected panic

Backtrace:

thread 'main' panicked at 'assertion failed: `(left == right)`
  left: `Some((_3, bw0))`,
 right: `None`: never found an activation for this borrow!', librustc_mir\borrow_check\borrow_set.rs:121:9
stack backtrace:
   0: <std::sync::mpsc::RecvTimeoutError as core::fmt::Debug>::fmt
   1: <std::sys::windows::dynamic_lib::DynamicLibrary as core::ops::drop::Drop>::drop
   2: std::panicking::take_hook
   3: std::panicking::take_hook
   4: rustc::ty::structural_impls::<impl rustc::ty::context::Lift<'tcx> for rustc::middle::const_val::ErrKind<'a>>::lift_to_tcx
   5: std::panicking::rust_panic_with_hook
   6: std::panicking::begin_panic_fmt
   7: <rustc_mir::borrow_check::borrow_set::BorrowData<'tcx> as core::fmt::Display>::fmt
   8: rustc_mir::borrow_check::provide
   9: <rustc_mir::dataflow::move_paths::abs_domain::AbstractOperand as core::fmt::Debug>::fmt
  10: <rustc_mir::borrow_check::borrow_set::BorrowData<'tcx> as core::fmt::Debug>::fmt
  11: rustc_mir::borrow_check::provide
  12: rustc::ty::maps::on_disk_cache::__ty_decoder_impl::<impl serialize::serialize::Decoder for rustc::ty::maps::on_disk_cache::CacheDecoder<'a, 'tcx, 'x>>::read_str
  13: rustc::ty::context::tls::track_diagnostic
  14: rustc::ty::context::tls::track_diagnostic
  15: rustc::dep_graph::graph::DepGraph::assert_ignored
  16: rustc::ty::context::tls::track_diagnostic
  17: rustc::ty::maps::plumbing::<impl rustc::ty::context::TyCtxt<'a, 'gcx, 'tcx>>::try_mark_green_and_read
  18: rustc::ty::maps::plumbing::<impl rustc::ty::context::TyCtxt<'a, 'gcx, 'tcx>>::try_mark_green_and_read
  19: rustc::ty::maps::<impl rustc::ty::context::TyCtxt<'a, 'tcx, 'lcx>>::mir_borrowck
  20: <rustc_driver::derive_registrar::Finder as rustc::hir::itemlikevisit::ItemLikeVisitor<'v>>::visit_item
  21: <humantime::duration::Error as std::error::Error>::cause
  22: <rustc_driver::pretty::NoAnn<'hir> as rustc_driver::pretty::HirPrinterSupport<'hir>>::sess
  23: <unknown>
  24: rustc_driver::driver::compile_input
  25: rustc_driver::run_compiler
  26: rustc_driver::target_features::add_configuration
  27: rustc_driver::target_features::add_configuration
  28: _rust_maybe_catch_panic
  29: rustc_driver::set_sigpipe_handler
  30: rustc_driver::main
  31: <unknown>
  32: std::panicking::update_panic_count
  33: _rust_maybe_catch_panic
  34: std::rt::lang_start_internal
  35: <unknown>
  36: <unknown>
  37: BaseThreadInitThunk
  38: RtlUserThreadStart
query stack during panic:
#0 [mir_borrowck] processing `main`
end of query stack

[nikomatsakis]

The assertion failure is here:

rust/src/librustc_mir/borrow_check/borrow_set.rs

Lines 119 to 130 in 860d169

	// Double check: We should have found an activation for every pending
	// activation.
	assert_eq!(
	visitor
	.pending_activations
	.iter()
	.find(|&(_local, &borrow_index)| visitor.idx_vec[borrow_index]
	.activation_location
	.is_none()),
	None,
	"never found an activation for this borrow!",
	);

I think that this assertion is just wrong. We add the assertion because -- for 2-phase borrows -- we always generate an activation. But I did not consider the case where the activation is in dead code (as in this case).

Unfortunately, we probably can't just delete the assertion. The challenge is that the activation_location would then remain None, which means that later on when we read the borrow, we will interpret it as "not 2-phase":

rust/src/librustc_mir/borrow_check/path_utils.rs

Lines 84 to 91 in 860d169

	let activation_location = match borrow_data.activation_location {
	// If this is not a 2-phase borrow, it is always active.
	None => return true,
	// And if the unique 2-phase use is not an activation, then it is *never* active.
	Some((TwoPhaseUse::SharedUse, _)) => return false,
	// Otherwise, we derive info from the activation point `v`:
	Some((TwoPhaseUse::MutActivate, v)) => v,
	};

What we probably want to do is to change that field from an Option to some kind of 3-valued enum. Something like:

enum TwoPhaseActivation {
    NotTwoPhase, // what is now `None`
    NotActivated,
    ActivatedAt(Location), // roughly what is now `Some`
}

We could then use the NotActivated setting instead of the existing TwoPhaseUse::SharedUse enum (the TwoPhaseUse enum can probably just go away), so that we basically just return false and say it is never annotated:

rust/src/librustc_mir/borrow_check/path_utils.rs

Lines 87 to 88 in 860d169

	// And if the unique 2-phase use is not an activation, then it is *never* active.
	Some((TwoPhaseUse::SharedUse, _)) => return false,