Skip to content

Commit

Permalink
Rollup merge of rust-lang#53932 - matthewjasper:remove-base-path, r=n…
Browse files Browse the repository at this point in the history
…ikomatsakis

[NLL] Remove base_place

This function was supposed to make `Box` less special. But

* I think that the consensus is that MIR borrowck is going to fully special case `Box`
* It wasn't implemented correctly, it's looking at the type of the wrong `Place`, resulting in weird behaviour:

```rust
#![feature(nll)]
type A = Box<i32>; // If this is changed to another type then this will compile.

pub fn foo(x: Box<(String, A)>) {
    let a = x.0; // This will compile if these lines are swapped
    let b = x.1;
}
```

r? @nikomatsakis
  • Loading branch information
kennytm authored Sep 8, 2018
2 parents 4b2be62 + faf80ad commit 0158694
Show file tree
Hide file tree
Showing 3 changed files with 18 additions and 70 deletions.
50 changes: 4 additions & 46 deletions src/librustc_mir/borrow_check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1605,10 +1605,6 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
place_span: (&Place<'tcx>, Span),
flow_state: &Flows<'cx, 'gcx, 'tcx>,
) {
// FIXME: analogous code in check_loans first maps `place` to
// its base_path ... but is that what we want here?
let place = self.base_path(place_span.0);

let maybe_uninits = &flow_state.uninits;

// Bad scenarios:
Expand Down Expand Up @@ -1646,8 +1642,8 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
//
// This code covers scenarios 1, 2, and 3.

debug!("check_if_full_path_is_moved place: {:?}", place);
match self.move_path_closest_to(place) {
debug!("check_if_full_path_is_moved place: {:?}", place_span.0);
match self.move_path_closest_to(place_span.0) {
Ok(mpi) => {
if maybe_uninits.contains(&mpi) {
self.report_use_of_moved_or_uninitialized(
Expand Down Expand Up @@ -1677,10 +1673,6 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
place_span: (&Place<'tcx>, Span),
flow_state: &Flows<'cx, 'gcx, 'tcx>,
) {
// FIXME: analogous code in check_loans first maps `place` to
// its base_path ... but is that what we want here?
let place = self.base_path(place_span.0);

let maybe_uninits = &flow_state.uninits;

// Bad scenarios:
Expand Down Expand Up @@ -1709,8 +1701,8 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
//
// This code covers scenario 1.

debug!("check_if_path_or_subpath_is_moved place: {:?}", place);
if let Some(mpi) = self.move_path_for_place(place) {
debug!("check_if_path_or_subpath_is_moved place: {:?}", place_span.0);
if let Some(mpi) = self.move_path_for_place(place_span.0) {
if let Some(child_mpi) = maybe_uninits.has_any_child_of(mpi) {
self.report_use_of_moved_or_uninitialized(
context,
Expand Down Expand Up @@ -1813,11 +1805,6 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
let tcx = self.tcx;
match base.ty(self.mir, tcx).to_ty(tcx).sty {
ty::Adt(def, _) if def.has_dtor(tcx) => {

// FIXME: analogous code in
// check_loans.rs first maps
// `base` to its base_path.

self.check_if_path_or_subpath_is_moved(
context, InitializationRequiringAction::Assignment,
(base, span), flow_state);
Expand Down Expand Up @@ -2190,35 +2177,6 @@ enum Overlap {
Disjoint,
}

impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
// FIXME (#16118): function intended to allow the borrow checker
// to be less precise in its handling of Box while still allowing
// moves out of a Box. They should be removed when/if we stop
// treating Box specially (e.g. when/if DerefMove is added...)

fn base_path<'d>(&self, place: &'d Place<'tcx>) -> &'d Place<'tcx> {
//! Returns the base of the leftmost (deepest) dereference of an
//! Box in `place`. If there is no dereference of an Box
//! in `place`, then it just returns `place` itself.
let mut cursor = place;
let mut deepest = place;
loop {
let proj = match *cursor {
Place::Promoted(_) |
Place::Local(..) | Place::Static(..) => return deepest,
Place::Projection(ref proj) => proj,
};
if proj.elem == ProjectionElem::Deref
&& place.ty(self.mir, self.tcx).to_ty(self.tcx).is_box()
{
deepest = &proj.base;
}
cursor = &proj.base;
}
}
}

#[derive(Copy, Clone, PartialEq, Eq, Debug)]
struct Context {
kind: ContextKind,
Expand Down
35 changes: 12 additions & 23 deletions src/test/ui/borrowck/borrowck-box-insensitivity.nll.stderr
Original file line number Diff line number Diff line change
@@ -1,25 +1,14 @@
error[E0382]: use of moved value: `a.y`
--> $DIR/borrowck-box-insensitivity.rs:46:14
|
LL | let _x = a.x;
| --- value moved here
LL | //~^ value moved here
LL | let _y = a.y; //~ ERROR use of moved
| ^^^ value used here after move
|
= note: move occurs because `a.x` has type `std::boxed::Box<isize>`, which does not implement the `Copy` trait

error[E0382]: use of moved value: `a.y`
--> $DIR/borrowck-box-insensitivity.rs:108:14
|
LL | let _x = a.x.x;
| ----- value moved here
LL | //~^ value moved here
LL | let _y = a.y; //~ ERROR use of collaterally moved
| ^^^ value used here after move
|
= note: move occurs because `a.x.x` has type `std::boxed::Box<isize>`, which does not implement the `Copy` trait
error: compilation successful
--> $DIR/borrowck-box-insensitivity.rs:160:1
|
LL | / fn main() {
LL | | copy_after_move();
LL | | move_after_move();
LL | | borrow_after_move();
... |
LL | | mut_borrow_after_borrow_nested();
LL | | }
| |_^

error: aborting due to 2 previous errors
error: aborting due to previous error

For more information about this error, try `rustc --explain E0382`.
3 changes: 2 additions & 1 deletion src/test/ui/borrowck/borrowck-box-insensitivity.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
// option. This file may not be copied, modified, or distributed
// except according to those terms.

#![feature(box_syntax)]
#![feature(box_syntax, rustc_attrs)]

struct A {
x: Box<isize>,
Expand Down Expand Up @@ -156,6 +156,7 @@ fn mut_borrow_after_borrow_nested() {
//~^ mutable borrow occurs here
}

#[rustc_error]
fn main() {
copy_after_move();
move_after_move();
Expand Down

0 comments on commit 0158694

Please sign in to comment.