Skip to content

Commit

Permalink
Rollup merge of rust-lang#52058 - davidtwco:issue-51345, r=nikomatsakis
Browse files Browse the repository at this point in the history
Use of unimplemented!() causing ICE with NLL

Fixes rust-lang#51345.

r? @nikomatsakis
  • Loading branch information
Mark-Simulacrum committed Jul 7, 2018
2 parents 9b3ec57 + f90eada commit b71b921
Show file tree
Hide file tree
Showing 4 changed files with 70 additions and 41 deletions.
66 changes: 30 additions & 36 deletions src/librustc_mir/borrow_check/borrow_set.rs
Expand Up @@ -53,25 +53,22 @@ impl<'tcx> Index<BorrowIndex> for BorrowSet<'tcx> {
}
}

/// Every two-phase borrow has *exactly one* use (or else it is not a
/// proper two-phase borrow under our current definition). However, not
/// all uses are actually ones that activate the reservation.. In
/// particular, a shared borrow of a `&mut` does not activate the
/// reservation.
/// Location where a two phase borrow is activated, if a borrow
/// is in fact a two phase borrow.
#[derive(Copy, Clone, PartialEq, Eq, Debug)]
crate enum TwoPhaseUse {
MutActivate,
SharedUse,
crate enum TwoPhaseActivation {
NotTwoPhase,
NotActivated,
ActivatedAt(Location),
}

#[derive(Debug)]
crate struct BorrowData<'tcx> {
/// Location where the borrow reservation starts.
/// In many cases, this will be equal to the activation location but not always.
crate reserve_location: Location,
/// Location where the borrow is activated. None if this is not a
/// 2-phase borrow.
crate activation_location: Option<(TwoPhaseUse, Location)>,
/// Location where the borrow is activated.
crate activation_location: TwoPhaseActivation,
/// What kind of borrow this is
crate kind: mir::BorrowKind,
/// The region for which this borrow is live
Expand Down Expand Up @@ -116,19 +113,6 @@ impl<'tcx> BorrowSet<'tcx> {
visitor.visit_basic_block_data(block, block_data);
}

// 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!",
);

BorrowSet {
borrows: visitor.idx_vec,
location_map: visitor.location_map,
Expand Down Expand Up @@ -183,7 +167,7 @@ impl<'a, 'gcx, 'tcx> Visitor<'tcx> for GatherBorrows<'a, 'gcx, 'tcx> {
kind,
region,
reserve_location: location,
activation_location: None,
activation_location: TwoPhaseActivation::NotTwoPhase,
borrowed_place: borrowed_place.clone(),
assigned_place: assigned_place.clone(),
};
Expand Down Expand Up @@ -232,38 +216,43 @@ impl<'a, 'gcx, 'tcx> Visitor<'tcx> for GatherBorrows<'a, 'gcx, 'tcx> {
return;
}

if let Some(other_activation) = borrow_data.activation_location {
if let TwoPhaseActivation::ActivatedAt(other_location) =
borrow_data.activation_location {
span_bug!(
self.mir.source_info(location).span,
"found two uses for 2-phase borrow temporary {:?}: \
{:?} and {:?}",
temp,
location,
other_activation,
other_location,
);
}

// Otherwise, this is the unique later use
// that we expect.

let two_phase_use;

match context {
borrow_data.activation_location = match context {
// The use of TMP in a shared borrow does not
// count as an actual activation.
PlaceContext::Borrow { kind: mir::BorrowKind::Shared, .. } => {
two_phase_use = TwoPhaseUse::SharedUse;
TwoPhaseActivation::NotActivated
}
_ => {
two_phase_use = TwoPhaseUse::MutActivate;
// Double check: This borrow is indeed a two-phase borrow (that is,
// we are 'transitioning' from `NotActivated` to `ActivatedAt`) and
// we've not found any other activations (checked above).
assert_eq!(
borrow_data.activation_location,
TwoPhaseActivation::NotActivated,
"never found an activation for this borrow!",
);

self.activation_map
.entry(location)
.or_insert(Vec::new())
.push(borrow_index);
TwoPhaseActivation::ActivatedAt(location)
}
}

borrow_data.activation_location = Some((two_phase_use, location));
};
}

None => {}
Expand Down Expand Up @@ -342,6 +331,11 @@ impl<'a, 'gcx, 'tcx> GatherBorrows<'a, 'gcx, 'tcx> {
);
};

// Consider the borrow not activated to start. When we find an activation, we'll update
// this field.
let borrow_data = &mut self.idx_vec[borrow_index];
borrow_data.activation_location = TwoPhaseActivation::NotActivated;

// Insert `temp` into the list of pending activations. From
// now on, we'll be on the lookout for a use of it. Note that
// we are guaranteed that this use will come after the
Expand Down
10 changes: 5 additions & 5 deletions src/librustc_mir/borrow_check/path_utils.rs
Expand Up @@ -8,7 +8,7 @@
// option. This file may not be copied, modified, or distributed
// except according to those terms.

use borrow_check::borrow_set::{BorrowSet, BorrowData, TwoPhaseUse};
use borrow_check::borrow_set::{BorrowSet, BorrowData, TwoPhaseActivation};
use borrow_check::places_conflict;
use borrow_check::Context;
use borrow_check::ShallowOrDeep;
Expand Down Expand Up @@ -83,11 +83,11 @@ pub(super) fn is_active<'tcx>(

let activation_location = match borrow_data.activation_location {
// If this is not a 2-phase borrow, it is always active.
None => return true,
TwoPhaseActivation::NotTwoPhase => 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,
TwoPhaseActivation::NotActivated => return false,
// Otherwise, we derive info from the activation point `loc`:
TwoPhaseActivation::ActivatedAt(loc) => loc,
};

// Otherwise, it is active for every location *except* in between
Expand Down
18 changes: 18 additions & 0 deletions src/test/run-fail/issue-51345.rs
@@ -0,0 +1,18 @@
// Copyright 2012 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

// error-pattern: thread 'main' panicked at 'explicit panic'

#![feature(nll)]

fn main() {
let mut vec = vec![];
vec.push((vec.len(), panic!()));
}
17 changes: 17 additions & 0 deletions src/test/run-pass/issue-51345.rs
@@ -0,0 +1,17 @@
// Copyright 2012 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

#![feature(nll)]

fn main() {
let mut v = Vec::new();

loop { v.push(break) }
}

0 comments on commit b71b921

Please sign in to comment.