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.

Sign up for GitHub

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

Jump to bottom

a few move-checker improvements #36353

Merged
merged 4 commits into from
Sep 16, 2016
Merged

a few move-checker improvements #36353

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
eb19cd6
groundwork refactoring of `gather_moves`
arielb1 Jun 11, 2016
7b25e88
forbid moves out of slices
arielb1 Jul 24, 2016
eeedc14
fix dynamic drop for unions
arielb1 Sep 8, 2016
5c5f752
fix test fallout
Sep 16, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion src/librustc/mir/repr.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1243,7 +1243,7 @@ impl<'a, 'b> GraphSuccessors<'b> for Mir<'a> {
type Iter = IntoIter<BasicBlock>;
}

#[derive(Copy, Clone, PartialEq, Eq, Debug, Hash, Ord, PartialOrd)]
#[derive(Copy, Clone, PartialEq, Eq, Hash, Ord, PartialOrd)]
pub struct Location {
/// the location is within this block
pub block: BasicBlock,
Expand All @@ -1253,3 +1253,8 @@ pub struct Location {
pub statement_index: usize,
}

impl fmt::Debug for Location {
fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result {
write!(fmt, "{:?}[{}]", self.block, self.statement_index)
}
}
3 changes: 0 additions & 3 deletions src/librustc/mir/visit.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -774,9 +774,6 @@ pub enum LvalueContext<'tcx> {
// Being borrowed
Borrow { region: &'tcx Region, kind: BorrowKind },

// Being sliced -- this should be same as being borrowed, probably
Slice { from_start: usize, from_end: usize },

// Used as base for another lvalue, e.g. `x` in `x.y`
Projection,

Expand Down
1 change: 1 addition & 0 deletions src/librustc_borrowck/borrowck/gather_loans/gather_moves.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,7 @@ fn check_and_get_illegal_move_origin<'a, 'tcx>(bccx: &BorrowckCtxt<'a, 'tcx>,
check_and_get_illegal_move_origin(bccx, b)
}
}
ty::TySlice(..) => Some(cmt.clone()),
_ => {
check_and_get_illegal_move_origin(bccx, b)
}
Expand Down
26 changes: 14 additions & 12 deletions src/librustc_borrowck/borrowck/gather_loans/move_error.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ use rustc::ty;
use syntax::ast;
use syntax_pos;
use errors::DiagnosticBuilder;
use rustc::hir;

pub struct MoveErrorCollector<'tcx> {
errors: Vec<MoveError<'tcx>>
Expand Down Expand Up @@ -131,17 +130,20 @@ fn report_cannot_move_out_of<'a, 'tcx>(bccx: &BorrowckCtxt<'a, 'tcx>,
err
}

Categorization::Interior(ref b, mc::InteriorElement(Kind::Index, _)) => {
let expr = bccx.tcx.map.expect_expr(move_from.id);
if let hir::ExprIndex(..) = expr.node {
let mut err = struct_span_err!(bccx, move_from.span, E0508,
"cannot move out of type `{}`, \
a non-copy fixed-size array",
b.ty);
err.span_label(move_from.span, &format!("cannot move out of here"));
err
} else {
span_bug!(move_from.span, "this path should not cause illegal move");
Categorization::Interior(ref b, mc::InteriorElement(ik, _)) => {
match (&b.ty.sty, ik) {
(&ty::TySlice(..), _) |
(_, Kind::Index) => {
let mut err = struct_span_err!(bccx, move_from.span, E0508,
"cannot move out of type `{}`, \
a non-copy array",
b.ty);
err.span_label(move_from.span, &format!("cannot move out of here"));
err
}
(_, Kind::Pattern) => {
span_bug!(move_from.span, "this path should not cause illegal move");
}
}
}

Expand Down
47 changes: 21 additions & 26 deletions src/librustc_borrowck/borrowck/mir/dataflow/impls.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ use super::super::MoveDataParamEnv;
use super::super::DropFlagState;
use super::super::drop_flag_effects_for_function_entry;
use super::super::drop_flag_effects_for_location;
use super::super::on_all_children_bits;
use super::super::on_lookup_result_bits;

use super::{BitDenotation, BlockSets, DataflowOperator};

Expand Down Expand Up @@ -277,10 +277,9 @@ impl<'a, 'tcx> BitDenotation for MaybeInitializedLvals<'a, 'tcx> {
dest_lval: &repr::Lvalue) {
// when a call returns successfully, that means we need to set
// the bits for that dest_lval to 1 (initialized).
let move_path_index = ctxt.move_data.rev_lookup.find(dest_lval);
on_all_children_bits(self.tcx, self.mir, &ctxt.move_data,
move_path_index,
|mpi| { in_out.add(&mpi); });
on_lookup_result_bits(self.tcx, self.mir, &ctxt.move_data,
ctxt.move_data.rev_lookup.find(dest_lval),
|mpi| { in_out.add(&mpi); });
}
}

Expand Down Expand Up @@ -338,11 +337,10 @@ impl<'a, 'tcx> BitDenotation for MaybeUninitializedLvals<'a, 'tcx> {
_dest_bb: repr::BasicBlock,
dest_lval: &repr::Lvalue) {
// when a call returns successfully, that means we need to set
// the bits for that dest_lval to 1 (initialized).
let move_path_index = ctxt.move_data.rev_lookup.find(dest_lval);
on_all_children_bits(self.tcx, self.mir, &ctxt.move_data,
move_path_index,
|mpi| { in_out.remove(&mpi); });
// the bits for that dest_lval to 0 (initialized).
on_lookup_result_bits(self.tcx, self.mir, &ctxt.move_data,
ctxt.move_data.rev_lookup.find(dest_lval),
|mpi| { in_out.remove(&mpi); });
}
}

Expand Down Expand Up @@ -400,10 +398,9 @@ impl<'a, 'tcx> BitDenotation for DefinitelyInitializedLvals<'a, 'tcx> {
dest_lval: &repr::Lvalue) {
// when a call returns successfully, that means we need to set
// the bits for that dest_lval to 1 (initialized).
let move_path_index = ctxt.move_data.rev_lookup.find(dest_lval);
on_all_children_bits(self.tcx, self.mir, &ctxt.move_data,
move_path_index,
|mpi| { in_out.add(&mpi); });
on_lookup_result_bits(self.tcx, self.mir, &ctxt.move_data,
ctxt.move_data.rev_lookup.find(dest_lval),
|mpi| { in_out.add(&mpi); });
}
}

Expand Down Expand Up @@ -448,11 +445,10 @@ impl<'a, 'tcx> BitDenotation for MovingOutStatements<'a, 'tcx> {
// assigning into this `lvalue` kills all
// MoveOuts from it, and *also* all MoveOuts
// for children and associated fragment sets.
let move_path_index = rev_lookup.find(lvalue);
on_all_children_bits(tcx,
on_lookup_result_bits(tcx,
mir,
move_data,
move_path_index,
rev_lookup.find(lvalue),
|mpi| for moi in &path_map[mpi] {
assert!(moi.index() < bits_per_block);
sets.kill_set.add(&moi);
Expand Down Expand Up @@ -489,18 +485,17 @@ impl<'a, 'tcx> BitDenotation for MovingOutStatements<'a, 'tcx> {
_dest_bb: repr::BasicBlock,
dest_lval: &repr::Lvalue) {
let move_data = &ctxt.move_data;
let move_path_index = move_data.rev_lookup.find(dest_lval);
let bits_per_block = self.bits_per_block(ctxt);

let path_map = &move_data.path_map;
on_all_children_bits(self.tcx,
self.mir,
move_data,
move_path_index,
|mpi| for moi in &path_map[mpi] {
assert!(moi.index() < bits_per_block);
in_out.remove(&moi);
});
on_lookup_result_bits(self.tcx,
self.mir,
move_data,
move_data.rev_lookup.find(dest_lval),
|mpi| for moi in &path_map[mpi] {
assert!(moi.index() < bits_per_block);
in_out.remove(&moi);
});
}
}

Expand Down
24 changes: 15 additions & 9 deletions src/librustc_borrowck/borrowck/mir/dataflow/sanity_check.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ use rustc::ty::{self, TyCtxt};
use rustc::mir::repr::{self, Mir};
use rustc_data_structures::indexed_vec::Idx;

use super::super::gather_moves::{MovePathIndex};
use super::super::gather_moves::{MovePathIndex, LookupResult};
use super::super::MoveDataParamEnv;
use super::BitDenotation;
use super::DataflowResults;
Expand Down Expand Up @@ -116,20 +116,26 @@ fn each_block<'a, 'tcx, O>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
repr::BorrowKind::Shared,
ref peeking_at_lval) = *rvalue {
// Okay, our search is over.
let peek_mpi = move_data.rev_lookup.find(peeking_at_lval);
let bit_state = sets.on_entry.contains(&peek_mpi);
debug!("rustc_peek({:?} = &{:?}) bit_state: {}",
lvalue, peeking_at_lval, bit_state);
if !bit_state {
tcx.sess.span_err(span, &format!("rustc_peek: bit not set"));
match move_data.rev_lookup.find(peeking_at_lval) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All these errors look quite internal to me. They either should become a bug! or receive a more meaningful message.

Copy link
Contributor Author

@arielb1 arielb1 Sep 8, 2016 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's an internal debugging feature anyway.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Putting pedantic hat on): technically the span_err are for unit testing; see e.g. https://github.com/rust-lang/rust/blob/34fd68668152530dcd1d00865fa8514461b895d7/src/test/compile-fail/mir-dataflow/uninits-1.rs

(making the message more meaningful might be nice nonetheless, but we would want to balance that against the effort required to maintain the unit tests...)

LookupResult::Exact(peek_mpi) => {
let bit_state = sets.on_entry.contains(&peek_mpi);
debug!("rustc_peek({:?} = &{:?}) bit_state: {}",
lvalue, peeking_at_lval, bit_state);
if !bit_state {
tcx.sess.span_err(span, "rustc_peek: bit not set");
}
}
LookupResult::Parent(..) => {
tcx.sess.span_err(span, "rustc_peek: argument untracked");
}
}
return;
} else {
// Our search should have been over, but the input
// does not match expectations of `rustc_peek` for
// this sanity_check.
let msg = &format!("rustc_peek: argument expression \
must be immediate borrow of form `&expr`");
let msg = "rustc_peek: argument expression \
must be immediate borrow of form `&expr`";
tcx.sess.span_err(span, msg);
}
}
Expand Down
Loading