From 830d65c1ffa2f19ada218d8a95b62042411b68a7 Mon Sep 17 00:00:00 2001 From: Mikhail Modin Date: Fri, 10 Nov 2017 14:11:25 +0300 Subject: [PATCH 1/3] add `StorageDead` handling --- .../dataflow/drop_flag_effects.rs | 7 ++++- src/librustc_mir/dataflow/impls/mod.rs | 21 ++++++++----- .../dataflow/move_paths/builder.rs | 16 +++++----- .../borrowck/borrowck-storage-dead.rs | 30 +++++++++++++++++++ 4 files changed, 59 insertions(+), 15 deletions(-) create mode 100644 src/test/compile-fail/borrowck/borrowck-storage-dead.rs diff --git a/src/librustc_mir/dataflow/drop_flag_effects.rs b/src/librustc_mir/dataflow/drop_flag_effects.rs index e35bd34c40bd0..7ef9cc6fc3f49 100644 --- a/src/librustc_mir/dataflow/drop_flag_effects.rs +++ b/src/librustc_mir/dataflow/drop_flag_effects.rs @@ -231,8 +231,13 @@ pub(crate) fn drop_flag_effects_for_location<'a, 'gcx, 'tcx, F>( } } } + mir::StatementKind::StorageDead(local) => { + on_lookup_result_bits(tcx, mir, move_data, + move_data.rev_lookup.find(&mir::Lvalue::Local(local)), + |mpi| callback(mpi, DropFlagState::Absent)) + + } mir::StatementKind::StorageLive(_) | - mir::StatementKind::StorageDead(_) | mir::StatementKind::InlineAsm { .. } | mir::StatementKind::EndRegion(_) | mir::StatementKind::Validate(..) | diff --git a/src/librustc_mir/dataflow/impls/mod.rs b/src/librustc_mir/dataflow/impls/mod.rs index dad96dc3a6ffe..524f8ffed83f3 100644 --- a/src/librustc_mir/dataflow/impls/mod.rs +++ b/src/librustc_mir/dataflow/impls/mod.rs @@ -456,14 +456,21 @@ impl<'a, 'gcx, 'tcx> BitDenotation for MovingOutStatements<'a, 'gcx, 'tcx> { let path_map = &move_data.path_map; let rev_lookup = &move_data.rev_lookup; - debug!("stmt {:?} at loc {:?} moves out of move_indexes {:?}", - stmt, location, &loc_map[location]); - for move_index in &loc_map[location] { - // Every path deinitialized by a *particular move* - // has corresponding bit, "gen'ed" (i.e. set) - // here, in dataflow vector - zero_to_one(sets.gen_set.words_mut(), *move_index); + match stmt.kind { + // skip move out for StorageDead + mir::StatementKind::StorageDead(_) => {} + _ => { + debug!("stmt {:?} at loc {:?} moves out of move_indexes {:?}", + stmt, location, &loc_map[location]); + for move_index in &loc_map[location] { + // Every path deinitialized by a *particular move* + // has corresponding bit, "gen'ed" (i.e. set) + // here, in dataflow vector + zero_to_one(sets.gen_set.words_mut(), *move_index); + } + } } + let bits_per_block = self.bits_per_block(); match stmt.kind { mir::StatementKind::SetDiscriminant { .. } => { diff --git a/src/librustc_mir/dataflow/move_paths/builder.rs b/src/librustc_mir/dataflow/move_paths/builder.rs index f333dd4d2a178..a0212de605eee 100644 --- a/src/librustc_mir/dataflow/move_paths/builder.rs +++ b/src/librustc_mir/dataflow/move_paths/builder.rs @@ -250,8 +250,10 @@ impl<'b, 'a, 'gcx, 'tcx> Gatherer<'b, 'a, 'gcx, 'tcx> { } self.gather_rvalue(rval); } - StatementKind::StorageLive(_) | - StatementKind::StorageDead(_) => {} + StatementKind::StorageLive(_) => {} + StatementKind::StorageDead(local) => { + self.gather_move(&Lvalue::Local(local), true); + } StatementKind::SetDiscriminant{ .. } => { span_bug!(stmt.source_info.span, "SetDiscriminant should not exist during borrowck"); @@ -309,7 +311,7 @@ impl<'b, 'a, 'gcx, 'tcx> Gatherer<'b, 'a, 'gcx, 'tcx> { TerminatorKind::Unreachable => { } TerminatorKind::Return => { - self.gather_move(&Lvalue::Local(RETURN_POINTER)); + self.gather_move(&Lvalue::Local(RETURN_POINTER), false); } TerminatorKind::Assert { .. } | @@ -322,7 +324,7 @@ impl<'b, 'a, 'gcx, 'tcx> Gatherer<'b, 'a, 'gcx, 'tcx> { } TerminatorKind::Drop { ref location, target: _, unwind: _ } => { - self.gather_move(location); + self.gather_move(location, false); } TerminatorKind::DropAndReplace { ref location, ref value, .. } => { self.create_move_path(location); @@ -344,19 +346,19 @@ impl<'b, 'a, 'gcx, 'tcx> Gatherer<'b, 'a, 'gcx, 'tcx> { match *operand { Operand::Constant(..) => {} // not-a-move Operand::Consume(ref lval) => { // a move - self.gather_move(lval); + self.gather_move(lval, false); } } } - fn gather_move(&mut self, lval: &Lvalue<'tcx>) { + fn gather_move(&mut self, lval: &Lvalue<'tcx>, force: bool) { debug!("gather_move({:?}, {:?})", self.loc, lval); let tcx = self.builder.tcx; let gcx = tcx.global_tcx(); let lv_ty = lval.ty(self.builder.mir, tcx).to_ty(tcx); let erased_ty = gcx.lift(&tcx.erase_regions(&lv_ty)).unwrap(); - if !erased_ty.moves_by_default(gcx, self.builder.param_env, DUMMY_SP) { + if !force && !erased_ty.moves_by_default(gcx, self.builder.param_env, DUMMY_SP) { debug!("gather_move({:?}, {:?}) - {:?} is Copy. skipping", self.loc, lval, lv_ty); return } diff --git a/src/test/compile-fail/borrowck/borrowck-storage-dead.rs b/src/test/compile-fail/borrowck/borrowck-storage-dead.rs new file mode 100644 index 0000000000000..5ef502acd8113 --- /dev/null +++ b/src/test/compile-fail/borrowck/borrowck-storage-dead.rs @@ -0,0 +1,30 @@ +// 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 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// compile-flags: -Z emit-end-regions -Z borrowck-mir + +fn ok() { + loop { + let _x = 1; + } +} + +fn fail() { + loop { + let x: i32; + let _ = x + 1; //~ERROR (Ast) [E0381] + //~^ ERROR (Mir) [E0381] + } +} + +fn main() { + ok(); + fail(); +} From 34be1516aa0f308d68369fe756b10f3c87ef2e96 Mon Sep 17 00:00:00 2001 From: Mikhail Modin Date: Mon, 13 Nov 2017 15:46:22 +0300 Subject: [PATCH 2/3] fix comment, remove redundant code --- .../dataflow/drop_flag_effects.rs | 19 +------------------ src/librustc_mir/dataflow/impls/mod.rs | 5 ++++- 2 files changed, 5 insertions(+), 19 deletions(-) diff --git a/src/librustc_mir/dataflow/drop_flag_effects.rs b/src/librustc_mir/dataflow/drop_flag_effects.rs index 7ef9cc6fc3f49..c1320d9daa8a4 100644 --- a/src/librustc_mir/dataflow/drop_flag_effects.rs +++ b/src/librustc_mir/dataflow/drop_flag_effects.rs @@ -8,8 +8,6 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -use syntax_pos::DUMMY_SP; - use rustc::mir::{self, Mir, Location}; use rustc::ty::{self, TyCtxt}; use util::elaborate_drops::DropFlagState; @@ -187,7 +185,6 @@ pub(crate) fn drop_flag_effects_for_location<'a, 'gcx, 'tcx, F>( where F: FnMut(MovePathIndex, DropFlagState) { let move_data = &ctxt.move_data; - let param_env = ctxt.param_env; debug!("drop_flag_effects_for_location({:?})", loc); // first, move out of the RHS @@ -195,15 +192,6 @@ pub(crate) fn drop_flag_effects_for_location<'a, 'gcx, 'tcx, F>( let path = mi.move_path_index(move_data); debug!("moving out of path {:?}", move_data.move_paths[path]); - // don't move out of non-Copy things - let lvalue = &move_data.move_paths[path].lvalue; - let ty = lvalue.ty(mir, tcx).to_ty(tcx); - let gcx = tcx.global_tcx(); - let erased_ty = gcx.lift(&tcx.erase_regions(&ty)).unwrap(); - if !erased_ty.moves_by_default(gcx, param_env, DUMMY_SP) { - continue; - } - on_all_children_bits(tcx, mir, move_data, path, |mpi| callback(mpi, DropFlagState::Absent)) @@ -231,13 +219,8 @@ pub(crate) fn drop_flag_effects_for_location<'a, 'gcx, 'tcx, F>( } } } - mir::StatementKind::StorageDead(local) => { - on_lookup_result_bits(tcx, mir, move_data, - move_data.rev_lookup.find(&mir::Lvalue::Local(local)), - |mpi| callback(mpi, DropFlagState::Absent)) - - } mir::StatementKind::StorageLive(_) | + mir::StatementKind::StorageDead(_) | mir::StatementKind::InlineAsm { .. } | mir::StatementKind::EndRegion(_) | mir::StatementKind::Validate(..) | diff --git a/src/librustc_mir/dataflow/impls/mod.rs b/src/librustc_mir/dataflow/impls/mod.rs index 524f8ffed83f3..147f3d796b91c 100644 --- a/src/librustc_mir/dataflow/impls/mod.rs +++ b/src/librustc_mir/dataflow/impls/mod.rs @@ -457,7 +457,10 @@ impl<'a, 'gcx, 'tcx> BitDenotation for MovingOutStatements<'a, 'gcx, 'tcx> { let rev_lookup = &move_data.rev_lookup; match stmt.kind { - // skip move out for StorageDead + // this analysis only tries to find moves explicitly + // written by the user, so we ignore the move-outs + // created by `StorageDead` and at the beginning + // of a function. mir::StatementKind::StorageDead(_) => {} _ => { debug!("stmt {:?} at loc {:?} moves out of move_indexes {:?}", From 9e35fd262f8ca57d755259448df4e7b601cb236a Mon Sep 17 00:00:00 2001 From: Mikhail Modin Date: Wed, 15 Nov 2017 12:30:30 +0300 Subject: [PATCH 3/3] fix test --- src/test/compile-fail/issue-25579.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/test/compile-fail/issue-25579.rs b/src/test/compile-fail/issue-25579.rs index 7da80d2852e73..4437e20fc42f7 100644 --- a/src/test/compile-fail/issue-25579.rs +++ b/src/test/compile-fail/issue-25579.rs @@ -18,11 +18,10 @@ enum Sexpression { fn causes_ice(mut l: &mut Sexpression) { loop { match l { - &mut Sexpression::Num(ref mut n) => {}, //[mir]~ ERROR (Mir) [E0384] + &mut Sexpression::Num(ref mut n) => {}, &mut Sexpression::Cons(ref mut expr) => { //[ast]~ ERROR [E0499] //[mir]~^ ERROR (Ast) [E0499] //[mir]~| ERROR (Mir) [E0506] - //[mir]~| ERROR (Mir) [E0384] //[mir]~| ERROR (Mir) [E0499] l = &mut **expr; //[ast]~ ERROR [E0506] //[mir]~^ ERROR (Ast) [E0506]