From d409dbf98343fe69f8bd61e81b733c91c376217a Mon Sep 17 00:00:00 2001 From: Matthew Jasper Date: Mon, 27 Nov 2017 08:06:36 +0000 Subject: [PATCH 1/3] Add initialization info to `MoveData` * Used for new dataflow to track if a variable has every been initialized * Used for other dataflows that need to be updated for initializations --- src/librustc_mir/borrow_check.rs | 68 ++++-- .../dataflow/drop_flag_effects.rs | 75 +++--- src/librustc_mir/dataflow/impls/mod.rs | 222 +++++++++++++----- src/librustc_mir/dataflow/mod.rs | 1 + .../dataflow/move_paths/builder.rs | 61 ++++- src/librustc_mir/dataflow/move_paths/mod.rs | 37 +++ .../borrowck/borrowck-storage-dead.rs | 7 + src/test/compile-fail/issue-45199.rs | 41 ++++ 8 files changed, 389 insertions(+), 123 deletions(-) create mode 100644 src/test/compile-fail/issue-45199.rs diff --git a/src/librustc_mir/borrow_check.rs b/src/librustc_mir/borrow_check.rs index 289636be321..57735807edb 100644 --- a/src/librustc_mir/borrow_check.rs +++ b/src/librustc_mir/borrow_check.rs @@ -31,7 +31,7 @@ use dataflow::{do_dataflow}; use dataflow::{MoveDataParamEnv}; use dataflow::{BitDenotation, BlockSets, DataflowResults, DataflowResultsConsumer}; use dataflow::{MaybeInitializedLvals, MaybeUninitializedLvals}; -use dataflow::{MovingOutStatements}; +use dataflow::{MovingOutStatements, EverInitializedLvals}; use dataflow::{Borrows, BorrowData, BorrowIndex}; use dataflow::move_paths::{MoveError, IllegalMoveOriginKind}; use dataflow::move_paths::{HasMoveData, MoveData, MovePathIndex, LookupResult, MoveOutIndex}; @@ -130,6 +130,9 @@ fn do_mir_borrowck<'a, 'gcx, 'tcx>(infcx: &InferCtxt<'a, 'gcx, 'tcx>, let flow_move_outs = do_dataflow(tcx, mir, id, &attributes, &dead_unwinds, MovingOutStatements::new(tcx, mir, &mdpe), |bd, i| &bd.move_data().moves[i]); + let flow_ever_inits = do_dataflow(tcx, mir, id, &attributes, &dead_unwinds, + EverInitializedLvals::new(tcx, mir, &mdpe), + |bd, i| &bd.move_data().inits[i]); let mut mbcx = MirBorrowckCtxt { tcx: tcx, @@ -143,7 +146,8 @@ fn do_mir_borrowck<'a, 'gcx, 'tcx>(infcx: &InferCtxt<'a, 'gcx, 'tcx>, let mut state = InProgress::new(flow_borrows, flow_inits, flow_uninits, - flow_move_outs); + flow_move_outs, + flow_ever_inits); mbcx.analyze_results(&mut state); // entry point for DataflowResultsConsumer } @@ -167,6 +171,7 @@ pub struct InProgress<'b, 'gcx: 'tcx, 'tcx: 'b> { inits: FlowInProgress>, uninits: FlowInProgress>, move_outs: FlowInProgress>, + ever_inits: FlowInProgress>, } struct FlowInProgress where BD: BitDenotation { @@ -190,7 +195,8 @@ impl<'cx, 'gcx, 'tcx> DataflowResultsConsumer<'cx, 'tcx> for MirBorrowckCtxt<'cx flow_state.each_flow(|b| b.reset_to_entry_of(bb), |i| i.reset_to_entry_of(bb), |u| u.reset_to_entry_of(bb), - |m| m.reset_to_entry_of(bb)); + |m| m.reset_to_entry_of(bb), + |e| e.reset_to_entry_of(bb)); } fn reconstruct_statement_effect(&mut self, @@ -199,7 +205,8 @@ impl<'cx, 'gcx, 'tcx> DataflowResultsConsumer<'cx, 'tcx> for MirBorrowckCtxt<'cx flow_state.each_flow(|b| b.reconstruct_statement_effect(location), |i| i.reconstruct_statement_effect(location), |u| u.reconstruct_statement_effect(location), - |m| m.reconstruct_statement_effect(location)); + |m| m.reconstruct_statement_effect(location), + |e| e.reconstruct_statement_effect(location)); } fn apply_local_effect(&mut self, @@ -208,7 +215,8 @@ impl<'cx, 'gcx, 'tcx> DataflowResultsConsumer<'cx, 'tcx> for MirBorrowckCtxt<'cx flow_state.each_flow(|b| b.apply_local_effect(), |i| i.apply_local_effect(), |u| u.apply_local_effect(), - |m| m.apply_local_effect()); + |m| m.apply_local_effect(), + |e| e.apply_local_effect()); } fn reconstruct_terminator_effect(&mut self, @@ -217,7 +225,8 @@ impl<'cx, 'gcx, 'tcx> DataflowResultsConsumer<'cx, 'tcx> for MirBorrowckCtxt<'cx flow_state.each_flow(|b| b.reconstruct_terminator_effect(location), |i| i.reconstruct_terminator_effect(location), |u| u.reconstruct_terminator_effect(location), - |m| m.reconstruct_terminator_effect(location)); + |m| m.reconstruct_terminator_effect(location), + |e| e.reconstruct_terminator_effect(location)); } fn visit_block_entry(&mut self, @@ -750,22 +759,13 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { } if let Some(mpi) = self.move_path_for_lvalue(lvalue) { - if flow_state.inits.curr_state.contains(&mpi) { - // may already be assigned before reaching this statement; - // report error. - // FIXME: Not ideal, it only finds the assignment that lexically comes first - let assigned_lvalue = &move_data.move_paths[mpi].lvalue; - let assignment_stmt = self.mir.basic_blocks().iter().filter_map(|bb| { - bb.statements.iter().find(|stmt| { - if let StatementKind::Assign(ref lv, _) = stmt.kind { - *lv == *assigned_lvalue - } else { - false - } - }) - }).next().unwrap(); - self.report_illegal_reassignment( - context, (lvalue, span), assignment_stmt.source_info.span); + for ii in &move_data.init_path_map[mpi] { + if flow_state.ever_inits.curr_state.contains(ii) { + let first_assign_span = self.move_data.inits[*ii].span; + self.report_illegal_reassignment( + context, (lvalue, span), first_assign_span); + break; + } } } } @@ -1852,30 +1852,35 @@ impl<'b, 'gcx, 'tcx> InProgress<'b, 'gcx, 'tcx> { pub(super) fn new(borrows: DataflowResults>, inits: DataflowResults>, uninits: DataflowResults>, - move_out: DataflowResults>) + move_out: DataflowResults>, + ever_inits: DataflowResults>) -> Self { InProgress { borrows: FlowInProgress::new(borrows), inits: FlowInProgress::new(inits), uninits: FlowInProgress::new(uninits), - move_outs: FlowInProgress::new(move_out) + move_outs: FlowInProgress::new(move_out), + ever_inits: FlowInProgress::new(ever_inits) } } - fn each_flow(&mut self, + fn each_flow(&mut self, mut xform_borrows: XB, mut xform_inits: XI, mut xform_uninits: XU, - mut xform_move_outs: XM) where + mut xform_move_outs: XM, + mut xform_ever_inits: XE) where XB: FnMut(&mut FlowInProgress>), XI: FnMut(&mut FlowInProgress>), XU: FnMut(&mut FlowInProgress>), XM: FnMut(&mut FlowInProgress>), + XE: FnMut(&mut FlowInProgress>), { xform_borrows(&mut self.borrows); xform_inits(&mut self.inits); xform_uninits(&mut self.uninits); xform_move_outs(&mut self.move_outs); + xform_ever_inits(&mut self.ever_inits); } fn summary(&self) -> String { @@ -1932,6 +1937,17 @@ impl<'b, 'gcx, 'tcx> InProgress<'b, 'gcx, 'tcx> { &self.move_outs.base_results.operator().move_data().moves[mpi_move_out]; s.push_str(&format!("{:?}", move_out)); }); + s.push_str("] "); + + s.push_str("ever_init: ["); + let mut saw_one = false; + self.ever_inits.each_state_bit(|mpi_ever_init| { + if saw_one { s.push_str(", "); }; + saw_one = true; + let ever_init = + &self.ever_inits.base_results.operator().move_data().inits[mpi_ever_init]; + s.push_str(&format!("{:?}", ever_init)); + }); s.push_str("]"); return s; diff --git a/src/librustc_mir/dataflow/drop_flag_effects.rs b/src/librustc_mir/dataflow/drop_flag_effects.rs index c1320d9daa8..92539321300 100644 --- a/src/librustc_mir/dataflow/drop_flag_effects.rs +++ b/src/librustc_mir/dataflow/drop_flag_effects.rs @@ -14,7 +14,7 @@ use util::elaborate_drops::DropFlagState; use super::{MoveDataParamEnv}; use super::indexes::MovePathIndex; -use super::move_paths::{MoveData, LookupResult}; +use super::move_paths::{MoveData, LookupResult, InitKind}; pub fn move_path_children_matching<'tcx, F>(move_data: &MoveData<'tcx>, path: MovePathIndex, @@ -197,47 +197,40 @@ pub(crate) fn drop_flag_effects_for_location<'a, 'gcx, 'tcx, F>( |mpi| callback(mpi, DropFlagState::Absent)) } - let block = &mir[loc.block]; - match block.statements.get(loc.statement_index) { - Some(stmt) => match stmt.kind { - mir::StatementKind::SetDiscriminant{ .. } => { - span_bug!(stmt.source_info.span, "SetDiscrimant should not exist during borrowck"); - } - mir::StatementKind::Assign(ref lvalue, ref rvalue) => { - match rvalue.initialization_state() { - mir::tcx::RvalueInitializationState::Shallow => { - debug!("drop_flag_effects: box assignment {:?}", stmt); - if let LookupResult::Exact(mpi) = move_data.rev_lookup.find(lvalue) { - callback(mpi, DropFlagState::Present); - } - } - mir::tcx::RvalueInitializationState::Deep => { - debug!("drop_flag_effects: assignment {:?}", stmt); - on_lookup_result_bits(tcx, mir, move_data, - move_data.rev_lookup.find(lvalue), - |mpi| callback(mpi, DropFlagState::Present)) - } - } - } - mir::StatementKind::StorageLive(_) | - mir::StatementKind::StorageDead(_) | - mir::StatementKind::InlineAsm { .. } | - mir::StatementKind::EndRegion(_) | - mir::StatementKind::Validate(..) | - mir::StatementKind::Nop => {} - }, - None => { - debug!("drop_flag_effects: replace {:?}", block.terminator()); - match block.terminator().kind { - mir::TerminatorKind::DropAndReplace { ref location, .. } => { - on_lookup_result_bits(tcx, mir, move_data, - move_data.rev_lookup.find(location), - |mpi| callback(mpi, DropFlagState::Present)) - } - _ => { - // other terminators do not contain move-ins - } + debug!("drop_flag_effects: assignment for location({:?})", loc); + + for_location_inits( + tcx, + mir, + move_data, + loc, + |mpi| callback(mpi, DropFlagState::Present) + ); +} + +pub(crate) fn for_location_inits<'a, 'gcx, 'tcx, F>( + tcx: TyCtxt<'a, 'gcx, 'tcx>, + mir: &Mir<'tcx>, + move_data: &MoveData<'tcx>, + loc: Location, + mut callback: F) + where F: FnMut(MovePathIndex) +{ + for ii in &move_data.init_loc_map[loc] { + let init = move_data.inits[*ii]; + match init.kind { + InitKind::Deep => { + let path = init.path; + + on_all_children_bits(tcx, mir, move_data, + path, + &mut callback) + }, + InitKind::Shallow => { + let mpi = init.path; + callback(mpi); } + InitKind::NonPanicPathOnly => (), } } } diff --git a/src/librustc_mir/dataflow/impls/mod.rs b/src/librustc_mir/dataflow/impls/mod.rs index 147f3d796b9..f9f03023cc9 100644 --- a/src/librustc_mir/dataflow/impls/mod.rs +++ b/src/librustc_mir/dataflow/impls/mod.rs @@ -22,13 +22,13 @@ use rustc_data_structures::indexed_vec::Idx; use super::MoveDataParamEnv; use util::elaborate_drops::DropFlagState; -use super::move_paths::{HasMoveData, MoveData, MoveOutIndex, MovePathIndex}; -use super::move_paths::LookupResult; +use super::move_paths::{HasMoveData, MoveData, MoveOutIndex, MovePathIndex, InitIndex}; +use super::move_paths::{LookupResult, InitKind}; use super::{BitDenotation, BlockSets, DataflowOperator}; use super::drop_flag_effects_for_function_entry; use super::drop_flag_effects_for_location; -use super::on_lookup_result_bits; +use super::{on_lookup_result_bits, for_location_inits}; mod storage_liveness; @@ -242,6 +242,56 @@ impl<'a, 'gcx, 'tcx> HasMoveData<'tcx> for MovingOutStatements<'a, 'gcx, 'tcx> { fn move_data(&self) -> &MoveData<'tcx> { &self.mdpe.move_data } } +/// `EverInitializedLvals` tracks all l-values that might have ever been +/// initialized upon reaching a particular point in the control flow +/// for a function, without an intervening `Storage Dead`. +/// +/// This dataflow is used to determine if an immutable local variable may +/// be assigned to. +/// +/// For example, in code like the following, we have corresponding +/// dataflow information shown in the right-hand comments. +/// +/// ```rust +/// struct S; +/// fn foo(pred: bool) { // ever-init: +/// // { } +/// let a = S; let b = S; let c; let d; // {a, b } +/// +/// if pred { +/// drop(a); // {a, b, } +/// b = S; // {a, b, } +/// +/// } else { +/// drop(b); // {a, b, } +/// d = S; // {a, b, d } +/// +/// } // {a, b, d } +/// +/// c = S; // {a, b, c, d } +/// } +/// ``` +pub struct EverInitializedLvals<'a, 'gcx: 'tcx, 'tcx: 'a> { + tcx: TyCtxt<'a, 'gcx, 'tcx>, + mir: &'a Mir<'tcx>, + mdpe: &'a MoveDataParamEnv<'gcx, 'tcx>, +} + +impl<'a, 'gcx: 'tcx, 'tcx: 'a> EverInitializedLvals<'a, 'gcx, 'tcx> { + pub fn new(tcx: TyCtxt<'a, 'gcx, 'tcx>, + mir: &'a Mir<'tcx>, + mdpe: &'a MoveDataParamEnv<'gcx, 'tcx>) + -> Self + { + EverInitializedLvals { tcx: tcx, mir: mir, mdpe: mdpe } + } +} + +impl<'a, 'gcx, 'tcx> HasMoveData<'tcx> for EverInitializedLvals<'a, 'gcx, 'tcx> { + fn move_data(&self) -> &MoveData<'tcx> { &self.mdpe.move_data } +} + + impl<'a, 'gcx, 'tcx> MaybeInitializedLvals<'a, 'gcx, 'tcx> { fn update_bits(sets: &mut BlockSets, path: MovePathIndex, state: DropFlagState) @@ -454,7 +504,7 @@ impl<'a, 'gcx, 'tcx> BitDenotation for MovingOutStatements<'a, 'gcx, 'tcx> { let stmt = &mir[location.block].statements[location.statement_index]; let loc_map = &move_data.loc_map; let path_map = &move_data.path_map; - let rev_lookup = &move_data.rev_lookup; + let bits_per_block = self.bits_per_block(); match stmt.kind { // this analysis only tries to find moves explicitly @@ -474,52 +524,23 @@ impl<'a, 'gcx, 'tcx> BitDenotation for MovingOutStatements<'a, 'gcx, 'tcx> { } } - let bits_per_block = self.bits_per_block(); - match stmt.kind { - mir::StatementKind::SetDiscriminant { .. } => { - span_bug!(stmt.source_info.span, "SetDiscriminant should not exist in borrowck"); + for_location_inits(tcx, mir, move_data, location, + |mpi| for moi in &path_map[mpi] { + assert!(moi.index() < bits_per_block); + sets.kill_set.add(&moi); } - mir::StatementKind::Assign(ref lvalue, ref rvalue) => { - // assigning into this `lvalue` kills all - // MoveOuts from it, and *also* all MoveOuts - // for children and associated fragment sets. - match rvalue.initialization_state() { - mir::tcx::RvalueInitializationState::Shallow => { - if let LookupResult::Exact(mpi) = rev_lookup.find(lvalue) { - for moi in &path_map[mpi] { - assert!(moi.index() < bits_per_block); - sets.kill_set.add(&moi); - } - } - } - mir::tcx::RvalueInitializationState::Deep => { - on_lookup_result_bits(tcx, - mir, - move_data, - rev_lookup.find(lvalue), - |mpi| for moi in &path_map[mpi] { - assert!(moi.index() < bits_per_block); - sets.kill_set.add(&moi); - }); - } - } - } - mir::StatementKind::StorageLive(_) | - mir::StatementKind::StorageDead(_) | - mir::StatementKind::InlineAsm { .. } | - mir::StatementKind::EndRegion(_) | - mir::StatementKind::Validate(..) | - mir::StatementKind::Nop => {} - } + ); } fn terminator_effect(&self, sets: &mut BlockSets, location: Location) { - let (mir, move_data) = (self.mir, self.move_data()); + let (tcx, mir, move_data) = (self.tcx, self.mir, self.move_data()); let term = mir[location.block].terminator(); let loc_map = &move_data.loc_map; + let path_map = &move_data.path_map; + debug!("terminator {:?} at loc {:?} moves out of move_indexes {:?}", term, location, &loc_map[location]); let bits_per_block = self.bits_per_block(); @@ -527,19 +548,13 @@ impl<'a, 'gcx, 'tcx> BitDenotation for MovingOutStatements<'a, 'gcx, 'tcx> { assert!(move_index.index() < bits_per_block); zero_to_one(sets.gen_set.words_mut(), *move_index); } - match term.kind { - mir::TerminatorKind::DropAndReplace { ref location, .. } => { - on_lookup_result_bits(self.tcx, - mir, - move_data, - move_data.rev_lookup.find(location), - |mpi| for moi in &move_data.path_map[mpi] { - assert!(moi.index() < bits_per_block); - sets.kill_set.add(&moi); - }); + + for_location_inits(tcx, mir, move_data, location, + |mpi| for moi in &path_map[mpi] { + assert!(moi.index() < bits_per_block); + sets.kill_set.add(&moi); } - _ => {} - } + ); } fn propagate_call_return(&self, @@ -562,12 +577,97 @@ impl<'a, 'gcx, 'tcx> BitDenotation for MovingOutStatements<'a, 'gcx, 'tcx> { } } +impl<'a, 'gcx, 'tcx> BitDenotation for EverInitializedLvals<'a, 'gcx, 'tcx> { + type Idx = InitIndex; + fn name() -> &'static str { "ever_init" } + fn bits_per_block(&self) -> usize { + self.move_data().inits.len() + } + + fn start_block_effect(&self, sets: &mut BlockSets) { + let bits_per_block = self.bits_per_block(); + for init_index in (0..self.mir.arg_count).map(InitIndex::new) { + assert!(init_index.index() < bits_per_block); + sets.gen_set.add(&init_index); + } + } + fn statement_effect(&self, + sets: &mut BlockSets, + location: Location) { + let (_, mir, move_data) = (self.tcx, self.mir, self.move_data()); + let stmt = &mir[location.block].statements[location.statement_index]; + let init_path_map = &move_data.init_path_map; + let init_loc_map = &move_data.init_loc_map; + let rev_lookup = &move_data.rev_lookup; + let bits_per_block = self.bits_per_block(); + + debug!("statement {:?} at loc {:?} initializes move_indexes {:?}", + stmt, location, &init_loc_map[location]); + for init_index in &init_loc_map[location] { + assert!(init_index.index() < bits_per_block); + sets.gen_set.add(init_index); + } + + match stmt.kind { + mir::StatementKind::StorageDead(local) => { + // End inits for StorageDead, so that an immutable variable can + // be reinitialized on the next iteration of the loop. + if let LookupResult::Exact(mpi) = rev_lookup.find(&mir::Lvalue::Local(local)) { + debug!("stmt {:?} at loc {:?} clears the ever initialized status of {:?}", + stmt, location, &init_path_map[mpi]); + for ii in &init_path_map[mpi] { + assert!(ii.index() < bits_per_block); + sets.kill_set.add(&ii); + } + } + } + _ => {} + } + } + + fn terminator_effect(&self, + sets: &mut BlockSets, + location: Location) + { + let (mir, move_data) = (self.mir, self.move_data()); + let term = mir[location.block].terminator(); + let init_loc_map = &move_data.init_loc_map; + debug!("terminator {:?} at loc {:?} initializes move_indexes {:?}", + term, location, &init_loc_map[location]); + let bits_per_block = self.bits_per_block(); + for init_index in &init_loc_map[location] { + if move_data.inits[*init_index].kind != InitKind::NonPanicPathOnly { + assert!(init_index.index() < bits_per_block); + sets.gen_set.add(init_index); + } + } + } + + fn propagate_call_return(&self, + in_out: &mut IdxSet, + call_bb: mir::BasicBlock, + _dest_bb: mir::BasicBlock, + _dest_lval: &mir::Lvalue) { + let move_data = self.move_data(); + let bits_per_block = self.bits_per_block(); + let init_loc_map = &move_data.init_loc_map; + + let call_loc = Location { + block: call_bb, + statement_index: self.mir[call_bb].statements.len(), + }; + for init_index in &init_loc_map[call_loc] { + assert!(init_index.index() < bits_per_block); + in_out.add(init_index); + } + } +} + fn zero_to_one(bitvec: &mut [usize], move_index: MoveOutIndex) { let retval = bitvec.set_bit(move_index.index()); assert!(retval); } - impl<'a, 'gcx, 'tcx> BitwiseOperator for MaybeInitializedLvals<'a, 'gcx, 'tcx> { #[inline] fn join(&self, pred1: usize, pred2: usize) -> usize { @@ -596,6 +696,13 @@ impl<'a, 'gcx, 'tcx> BitwiseOperator for MovingOutStatements<'a, 'gcx, 'tcx> { } } +impl<'a, 'gcx, 'tcx> BitwiseOperator for EverInitializedLvals<'a, 'gcx, 'tcx> { + #[inline] + fn join(&self, pred1: usize, pred2: usize) -> usize { + pred1 | pred2 // inits from both preds are in scope + } +} + // The way that dataflow fixed point iteration works, you want to // start at bottom and work your way to a fixed point. Control-flow // merges will apply the `join` operator to each block entry's current @@ -633,3 +740,10 @@ impl<'a, 'gcx, 'tcx> DataflowOperator for MovingOutStatements<'a, 'gcx, 'tcx> { false // bottom = no loans in scope by default } } + +impl<'a, 'gcx, 'tcx> DataflowOperator for EverInitializedLvals<'a, 'gcx, 'tcx> { + #[inline] + fn bottom_value() -> bool { + false // bottom = no initialized variables by default + } +} diff --git a/src/librustc_mir/dataflow/mod.rs b/src/librustc_mir/dataflow/mod.rs index bca9324d5b0..711b8091ec8 100644 --- a/src/librustc_mir/dataflow/mod.rs +++ b/src/librustc_mir/dataflow/mod.rs @@ -27,6 +27,7 @@ use std::usize; pub use self::impls::{MaybeStorageLive}; pub use self::impls::{MaybeInitializedLvals, MaybeUninitializedLvals}; pub use self::impls::{DefinitelyInitializedLvals, MovingOutStatements}; +pub use self::impls::EverInitializedLvals; pub use self::impls::borrows::{Borrows, BorrowData, BorrowIndex}; pub(crate) use self::drop_flag_effects::*; diff --git a/src/librustc_mir/dataflow/move_paths/builder.rs b/src/librustc_mir/dataflow/move_paths/builder.rs index a0212de605e..af582b8cf66 100644 --- a/src/librustc_mir/dataflow/move_paths/builder.rs +++ b/src/librustc_mir/dataflow/move_paths/builder.rs @@ -22,7 +22,7 @@ use std::mem; use super::abs_domain::Lift; use super::{LocationMap, MoveData, MovePath, MovePathLookup, MovePathIndex, MoveOut, MoveOutIndex}; -use super::{MoveError}; +use super::{MoveError, InitIndex, Init, LookupResult, InitKind}; use super::IllegalMoveOriginKind::*; struct MoveDataBuilder<'a, 'gcx: 'tcx, 'tcx: 'a> { @@ -40,6 +40,7 @@ impl<'a, 'gcx, 'tcx> MoveDataBuilder<'a, 'gcx, 'tcx> { -> Self { let mut move_paths = IndexVec::new(); let mut path_map = IndexVec::new(); + let mut init_path_map = IndexVec::new(); MoveDataBuilder { mir, @@ -51,18 +52,28 @@ impl<'a, 'gcx, 'tcx> MoveDataBuilder<'a, 'gcx, 'tcx> { loc_map: LocationMap::new(mir), rev_lookup: MovePathLookup { locals: mir.local_decls.indices().map(Lvalue::Local).map(|v| { - Self::new_move_path(&mut move_paths, &mut path_map, None, v) + Self::new_move_path( + &mut move_paths, + &mut path_map, + &mut init_path_map, + None, + v, + ) }).collect(), projections: FxHashMap(), }, move_paths, path_map, + inits: IndexVec::new(), + init_loc_map: LocationMap::new(mir), + init_path_map, } } } fn new_move_path(move_paths: &mut IndexVec>, path_map: &mut IndexVec>, + init_path_map: &mut IndexVec>, parent: Option, lvalue: Lvalue<'tcx>) -> MovePathIndex @@ -82,6 +93,10 @@ impl<'a, 'gcx, 'tcx> MoveDataBuilder<'a, 'gcx, 'tcx> { let path_map_ent = path_map.push(vec![]); assert_eq!(path_map_ent, move_path); + + let init_path_map_ent = init_path_map.push(vec![]); + assert_eq!(init_path_map_ent, move_path); + move_path } } @@ -165,6 +180,7 @@ impl<'b, 'a, 'gcx, 'tcx> Gatherer<'b, 'a, 'gcx, 'tcx> { let path = MoveDataBuilder::new_move_path( &mut self.builder.data.move_paths, &mut self.builder.data.path_map, + &mut self.builder.data.init_path_map, Some(base), lval.clone() ); @@ -204,6 +220,8 @@ pub(super) fn gather_moves<'a, 'gcx, 'tcx>(mir: &Mir<'tcx>, (MoveData<'tcx>, Vec>)> { let mut builder = MoveDataBuilder::new(mir, tcx, param_env); + builder.gather_args(); + for (bb, block) in mir.basic_blocks().iter_enumerated() { for (i, stmt) in block.statements.iter().enumerate() { let source = Location { block: bb, statement_index: i }; @@ -221,6 +239,22 @@ pub(super) fn gather_moves<'a, 'gcx, 'tcx>(mir: &Mir<'tcx>, } impl<'a, 'gcx, 'tcx> MoveDataBuilder<'a, 'gcx, 'tcx> { + fn gather_args(&mut self) { + for arg in self.mir.args_iter() { + let path = self.data.rev_lookup.locals[arg]; + let span = self.mir.local_decls[arg].source_info.span; + + let init = self.data.inits.push(Init { + path, span, kind: InitKind::Deep + }); + + debug!("gather_args: adding init {:?} of {:?} for argument {:?}", + init, path, arg); + + self.data.init_path_map[path].push(init); + } + } + fn gather_statement(&mut self, loc: Location, stmt: &Statement<'tcx>) { debug!("gather_statement({:?}, {:?})", loc, stmt); (Gatherer { builder: self, loc }).gather_statement(stmt); @@ -247,6 +281,9 @@ impl<'b, 'a, 'gcx, 'tcx> Gatherer<'b, 'a, 'gcx, 'tcx> { // move-path for the interior so it will be separate from // the exterior. self.create_move_path(&lval.clone().deref()); + self.gather_init(lval, InitKind::Shallow); + } else { + self.gather_init(lval, InitKind::Deep); } self.gather_rvalue(rval); } @@ -329,6 +366,7 @@ impl<'b, 'a, 'gcx, 'tcx> Gatherer<'b, 'a, 'gcx, 'tcx> { TerminatorKind::DropAndReplace { ref location, ref value, .. } => { self.create_move_path(location); self.gather_operand(value); + self.gather_init(location, InitKind::Deep); } TerminatorKind::Call { ref func, ref args, ref destination, cleanup: _ } => { self.gather_operand(func); @@ -337,6 +375,7 @@ impl<'b, 'a, 'gcx, 'tcx> Gatherer<'b, 'a, 'gcx, 'tcx> { } if let Some((ref destination, _bb)) = *destination { self.create_move_path(destination); + self.gather_init(destination, InitKind::NonPanicPathOnly); } } } @@ -378,4 +417,22 @@ impl<'b, 'a, 'gcx, 'tcx> Gatherer<'b, 'a, 'gcx, 'tcx> { self.builder.data.path_map[path].push(move_out); self.builder.data.loc_map[self.loc].push(move_out); } + + fn gather_init(&mut self, lval: &Lvalue<'tcx>, kind: InitKind) { + debug!("gather_init({:?}, {:?})", self.loc, lval); + + if let LookupResult::Exact(path) = self.builder.data.rev_lookup.find(lval) { + let init = self.builder.data.inits.push(Init { + span: self.builder.mir.source_info(self.loc).span, + path, + kind, + }); + + debug!("gather_init({:?}, {:?}): adding init {:?} of {:?}", + self.loc, lval, init, path); + + self.builder.data.init_path_map[path].push(init); + self.builder.data.init_loc_map[self.loc].push(init); + } + } } diff --git a/src/librustc_mir/dataflow/move_paths/mod.rs b/src/librustc_mir/dataflow/move_paths/mod.rs index 73218c98150..4703dd8a2af 100644 --- a/src/librustc_mir/dataflow/move_paths/mod.rs +++ b/src/librustc_mir/dataflow/move_paths/mod.rs @@ -60,12 +60,16 @@ pub(crate) mod indexes { /// Index into MoveData.moves. new_index!(MoveOutIndex, "mo"); + /// Index into MoveData.inits. + new_index!(InitIndex, "in"); + /// Index into Borrows.locations new_index!(BorrowIndex, "bw"); } pub use self::indexes::MovePathIndex; pub use self::indexes::MoveOutIndex; +pub use self::indexes::InitIndex; impl MoveOutIndex { pub fn move_path_index(&self, move_data: &MoveData) -> MovePathIndex { @@ -126,6 +130,11 @@ pub struct MoveData<'tcx> { pub loc_map: LocationMap>, pub path_map: IndexVec>, pub rev_lookup: MovePathLookup<'tcx>, + pub inits: IndexVec, + /// Each Location `l` is mapped to the Inits that are effects + /// of executing the code at `l`. + pub init_loc_map: LocationMap>, + pub init_path_map: IndexVec>, } pub trait HasMoveData<'tcx> { @@ -182,6 +191,34 @@ impl fmt::Debug for MoveOut { } } +/// `Init` represents a point in a program that initializes some L-value; +#[derive(Copy, Clone)] +pub struct Init { + /// path being initialized + pub path: MovePathIndex, + /// span of initialization + pub span: Span, + /// Extra information about this initialization + pub kind: InitKind, +} + +/// Additional information about the initialization. +#[derive(Copy, Clone, Debug, PartialEq, Eq)] +pub enum InitKind { + /// Deep init, even on panic + Deep, + /// Only does a shallow init + Shallow, + /// This doesn't initialize the variabe on panic (and a panic is possible). + NonPanicPathOnly, +} + +impl fmt::Debug for Init { + fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result { + write!(fmt, "{:?}@{:?} ({:?})", self.path, self.span, self.kind) + } +} + /// Tables mapping from an l-value to its MovePathIndex. #[derive(Debug)] pub struct MovePathLookup<'tcx> { diff --git a/src/test/compile-fail/borrowck/borrowck-storage-dead.rs b/src/test/compile-fail/borrowck/borrowck-storage-dead.rs index 9971b1d0915..bc01088696d 100644 --- a/src/test/compile-fail/borrowck/borrowck-storage-dead.rs +++ b/src/test/compile-fail/borrowck/borrowck-storage-dead.rs @@ -16,6 +16,12 @@ fn ok() { } } +fn also_ok() { + loop { + let _x = String::new(); + } +} + fn fail() { loop { let x: i32; @@ -26,5 +32,6 @@ fn fail() { fn main() { ok(); + also_ok(); fail(); } diff --git a/src/test/compile-fail/issue-45199.rs b/src/test/compile-fail/issue-45199.rs new file mode 100644 index 00000000000..af8f7dce608 --- /dev/null +++ b/src/test/compile-fail/issue-45199.rs @@ -0,0 +1,41 @@ +// 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. + +// revisions: ast mir +//[mir]compile-flags: -Zborrowck=mir + +fn test_drop_replace() { + let b: Box; + b = Box::new(1); //[ast]~ NOTE first assignment + //[mir]~^ NOTE first assignment + b = Box::new(2); //[ast]~ ERROR cannot assign twice to immutable variable + //[mir]~^ ERROR cannot assign twice to immutable variable `b` + //[ast]~| NOTE cannot assign twice to immutable + //[mir]~| NOTE cannot assign twice to immutable +} + +fn test_call() { + let b = Box::new(1); //[ast]~ NOTE first assignment + //[mir]~^ NOTE first assignment + b = Box::new(2); //[ast]~ ERROR cannot assign twice to immutable variable + //[mir]~^ ERROR cannot assign twice to immutable variable `b` + //[ast]~| NOTE cannot assign twice to immutable + //[mir]~| NOTE cannot assign twice to immutable +} + +fn test_args(b: Box) { //[ast]~ NOTE first assignment + //[mir]~^ NOTE first assignment + b = Box::new(2); //[ast]~ ERROR cannot assign twice to immutable variable + //[mir]~^ ERROR cannot assign twice to immutable variable `b` + //[ast]~| NOTE cannot assign twice to immutable + //[mir]~| NOTE cannot assign twice to immutable +} + +fn main() {} From 6853b34f0ed922cae68d7e521b21ebd10d063e72 Mon Sep 17 00:00:00 2001 From: Matthew Jasper Date: Mon, 27 Nov 2017 08:07:49 +0000 Subject: [PATCH 2/3] Don't show first assignment in loops Matches current ast-borrowck behavior. --- src/librustc_mir/borrow_check.rs | 14 ++-- .../liveness-assign-imm-local-notes.rs | 54 ++++++++++++++++ .../liveness-assign-imm-local-notes.stderr | 64 +++++++++++++++++++ 3 files changed, 126 insertions(+), 6 deletions(-) create mode 100644 src/test/ui/lifetime-errors/liveness-assign-imm-local-notes.rs create mode 100644 src/test/ui/lifetime-errors/liveness-assign-imm-local-notes.stderr diff --git a/src/librustc_mir/borrow_check.rs b/src/librustc_mir/borrow_check.rs index 57735807edb..76968a871c9 100644 --- a/src/librustc_mir/borrow_check.rs +++ b/src/librustc_mir/borrow_check.rs @@ -1586,13 +1586,15 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { _context: Context, (lvalue, span): (&Lvalue<'tcx>, Span), assigned_span: Span) { - self.tcx.cannot_reassign_immutable(span, + let mut err = self.tcx.cannot_reassign_immutable(span, &self.describe_lvalue(lvalue), - Origin::Mir) - .span_label(span, "cannot assign twice to immutable variable") - .span_label(assigned_span, format!("first assignment to `{}`", - self.describe_lvalue(lvalue))) - .emit(); + Origin::Mir); + err.span_label(span, "cannot assign twice to immutable variable"); + if span != assigned_span { + err.span_label(assigned_span, format!("first assignment to `{}`", + self.describe_lvalue(lvalue))); + } + err.emit(); } fn report_assignment_to_static(&mut self, diff --git a/src/test/ui/lifetime-errors/liveness-assign-imm-local-notes.rs b/src/test/ui/lifetime-errors/liveness-assign-imm-local-notes.rs new file mode 100644 index 00000000000..d4ef87cdd76 --- /dev/null +++ b/src/test/ui/lifetime-errors/liveness-assign-imm-local-notes.rs @@ -0,0 +1,54 @@ +// 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. + +// FIXME: Change to UI Test +// Check notes are placed on an assignment that can actually preceed the current assigmnent +// Don't emmit a first assignment for assignment in a loop. + +// compile-flags: -Zborrowck=compare + +fn test() { + let x; + if true { + x = 1; + } else { + x = 2; + x = 3; //~ ERROR (Ast) [E0384] + //~^ ERROR (Mir) [E0384] + } +} + +fn test_in_loop() { + loop { + let x; + if true { + x = 1; + } else { + x = 2; + x = 3; //~ ERROR (Ast) [E0384] + //~^ ERROR (Mir) [E0384] + } + } +} + +fn test_using_loop() { + let x; + loop { + if true { + x = 1; //~ ERROR (Ast) [E0384] + //~^ ERROR (Mir) [E0384] + } else { + x = 2; //~ ERROR (Ast) [E0384] + //~^ ERROR (Mir) [E0384] + } + } +} + +fn main() {} diff --git a/src/test/ui/lifetime-errors/liveness-assign-imm-local-notes.stderr b/src/test/ui/lifetime-errors/liveness-assign-imm-local-notes.stderr new file mode 100644 index 00000000000..b8f738e445e --- /dev/null +++ b/src/test/ui/lifetime-errors/liveness-assign-imm-local-notes.stderr @@ -0,0 +1,64 @@ +error[E0384]: cannot assign twice to immutable variable `x` (Ast) + --> $DIR/liveness-assign-imm-local-notes.rs:23:9 + | +22 | x = 2; + | ----- first assignment to `x` +23 | x = 3; //~ ERROR (Ast) [E0384] + | ^^^^^ cannot assign twice to immutable variable + +error[E0384]: cannot assign twice to immutable variable `x` (Ast) + --> $DIR/liveness-assign-imm-local-notes.rs:35:13 + | +34 | x = 2; + | ----- first assignment to `x` +35 | x = 3; //~ ERROR (Ast) [E0384] + | ^^^^^ cannot assign twice to immutable variable + +error[E0384]: cannot assign twice to immutable variable `x` (Ast) + --> $DIR/liveness-assign-imm-local-notes.rs:45:13 + | +45 | x = 1; //~ ERROR (Ast) [E0384] + | ^^^^^ cannot assign twice to immutable variable + +error[E0384]: cannot assign twice to immutable variable `x` (Ast) + --> $DIR/liveness-assign-imm-local-notes.rs:48:13 + | +45 | x = 1; //~ ERROR (Ast) [E0384] + | ----- first assignment to `x` +... +48 | x = 2; //~ ERROR (Ast) [E0384] + | ^^^^^ cannot assign twice to immutable variable + +error[E0384]: cannot assign twice to immutable variable `x` (Mir) + --> $DIR/liveness-assign-imm-local-notes.rs:23:9 + | +22 | x = 2; + | ----- first assignment to `x` +23 | x = 3; //~ ERROR (Ast) [E0384] + | ^^^^^ cannot assign twice to immutable variable + +error[E0384]: cannot assign twice to immutable variable `x` (Mir) + --> $DIR/liveness-assign-imm-local-notes.rs:35:13 + | +34 | x = 2; + | ----- first assignment to `x` +35 | x = 3; //~ ERROR (Ast) [E0384] + | ^^^^^ cannot assign twice to immutable variable + +error[E0384]: cannot assign twice to immutable variable `x` (Mir) + --> $DIR/liveness-assign-imm-local-notes.rs:45:13 + | +45 | x = 1; //~ ERROR (Ast) [E0384] + | ^^^^^ cannot assign twice to immutable variable + +error[E0384]: cannot assign twice to immutable variable `x` (Mir) + --> $DIR/liveness-assign-imm-local-notes.rs:48:13 + | +45 | x = 1; //~ ERROR (Ast) [E0384] + | ----- first assignment to `x` +... +48 | x = 2; //~ ERROR (Ast) [E0384] + | ^^^^^ cannot assign twice to immutable variable + +error: aborting due to 8 previous errors + From d64a31882f6cdfcfbea82a257bb5cc64e6f7a7fc Mon Sep 17 00:00:00 2001 From: Matthew Jasper Date: Mon, 27 Nov 2017 08:07:58 +0000 Subject: [PATCH 3/3] Update tests involving E0384 for mir-borrowck --- .../compile-fail/assign-imm-local-twice.rs | 12 ++++++--- .../liveness-assign-imm-local-in-loop.rs | 9 +++++-- .../liveness-assign-imm-local-in-op-eq.rs | 12 ++++++--- .../liveness-assign-imm-local-with-drop.rs | 26 +++++++++++++++++++ .../liveness-assign-imm-local-with-init.rs | 12 ++++++--- 5 files changed, 60 insertions(+), 11 deletions(-) create mode 100644 src/test/compile-fail/liveness-assign-imm-local-with-drop.rs diff --git a/src/test/compile-fail/assign-imm-local-twice.rs b/src/test/compile-fail/assign-imm-local-twice.rs index 5b3b7d44bd2..d5e412c3745 100644 --- a/src/test/compile-fail/assign-imm-local-twice.rs +++ b/src/test/compile-fail/assign-imm-local-twice.rs @@ -8,12 +8,18 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. +// revisions: ast mir +//[mir]compile-flags: -Zborrowck=mir + fn test() { let v: isize; - v = 1; //~ NOTE first assignment + v = 1; //[ast]~ NOTE first assignment + //[mir]~^ NOTE first assignment println!("v={}", v); - v = 2; //~ ERROR cannot assign twice to immutable variable - //~| NOTE cannot assign twice to immutable + v = 2; //[ast]~ ERROR cannot assign twice to immutable variable + //[mir]~^ ERROR cannot assign twice to immutable variable `v` + //[ast]~| NOTE cannot assign twice to immutable + //[mir]~| NOTE cannot assign twice to immutable println!("v={}", v); } diff --git a/src/test/compile-fail/liveness-assign-imm-local-in-loop.rs b/src/test/compile-fail/liveness-assign-imm-local-in-loop.rs index fa8f264eb5a..f28906ddb95 100644 --- a/src/test/compile-fail/liveness-assign-imm-local-in-loop.rs +++ b/src/test/compile-fail/liveness-assign-imm-local-in-loop.rs @@ -8,11 +8,16 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. +// revisions: ast mir +//[mir]compile-flags: -Zborrowck=mir + fn test() { let v: isize; loop { - v = 1; //~ ERROR cannot assign twice to immutable variable - //~^ NOTE cannot assign twice to immutable variable + v = 1; //[ast]~ ERROR cannot assign twice to immutable variable + //[mir]~^ ERROR cannot assign twice to immutable variable `v` + //[ast]~| NOTE cannot assign twice to immutable variable + //[mir]~| NOTE cannot assign twice to immutable variable v.clone(); // just to prevent liveness warnings } } diff --git a/src/test/compile-fail/liveness-assign-imm-local-in-op-eq.rs b/src/test/compile-fail/liveness-assign-imm-local-in-op-eq.rs index bfdd4347de7..594cc076121 100644 --- a/src/test/compile-fail/liveness-assign-imm-local-in-op-eq.rs +++ b/src/test/compile-fail/liveness-assign-imm-local-in-op-eq.rs @@ -8,11 +8,17 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. +// revisions: ast mir +//[mir]compile-flags: -Zborrowck=mir + fn test() { let v: isize; - v = 2; //~ NOTE first assignment - v += 1; //~ ERROR cannot assign twice to immutable variable - //~| NOTE cannot assign twice to immutable + v = 2; //[ast]~ NOTE first assignment + //[mir]~^ NOTE first assignment + v += 1; //[ast]~ ERROR cannot assign twice to immutable variable + //[mir]~^ ERROR cannot assign twice to immutable variable `v` + //[ast]~| NOTE cannot assign twice to immutable + //[mir]~| NOTE cannot assign twice to immutable v.clone(); } diff --git a/src/test/compile-fail/liveness-assign-imm-local-with-drop.rs b/src/test/compile-fail/liveness-assign-imm-local-with-drop.rs new file mode 100644 index 00000000000..b4fb33ca15e --- /dev/null +++ b/src/test/compile-fail/liveness-assign-imm-local-with-drop.rs @@ -0,0 +1,26 @@ +// 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. + +// revisions: ast mir +//[mir]compile-flags: -Zborrowck=mir + +fn test() { + let b = Box::new(1); //[ast]~ NOTE first assignment + //[mir]~^ NOTE first assignment + drop(b); + b = Box::new(2); //[ast]~ ERROR cannot assign twice to immutable variable + //[mir]~^ ERROR cannot assign twice to immutable variable `b` + //[ast]~| NOTE cannot assign twice to immutable + //[mir]~| NOTE cannot assign twice to immutable + drop(b); +} + +fn main() { +} diff --git a/src/test/compile-fail/liveness-assign-imm-local-with-init.rs b/src/test/compile-fail/liveness-assign-imm-local-with-init.rs index f35c1c69acd..7204b5d5f2e 100644 --- a/src/test/compile-fail/liveness-assign-imm-local-with-init.rs +++ b/src/test/compile-fail/liveness-assign-imm-local-with-init.rs @@ -8,11 +8,17 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. +// revisions: ast mir +//[mir]compile-flags: -Zborrowck=mir + fn test() { - let v: isize = 1; //~ NOTE first assignment + let v: isize = 1; //[ast]~ NOTE first assignment + //[mir]~^ NOTE first assignment v.clone(); - v = 2; //~ ERROR cannot assign twice to immutable variable - //~| NOTE cannot assign twice to immutable + v = 2; //[ast]~ ERROR cannot assign twice to immutable variable + //[mir]~^ ERROR cannot assign twice to immutable variable `v` + //[ast]~| NOTE cannot assign twice to immutable + //[mir]~| NOTE cannot assign twice to immutable v.clone(); }