Skip to content

Mir Borrowck: Parity with Ast for E0384 (Cannot assign twice to immutable) #46022

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

Merged
merged 3 commits into from
Nov 27, 2017
Merged

Mir Borrowck: Parity with Ast for E0384 (Cannot assign twice to immutable) #46022

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d409dbf
Add initialization info to `MoveData`
matthewjasper Nov 27, 2017
6853b34
Don't show first assignment in loops
matthewjasper Nov 27, 2017
d64a318
Update tests involving E0384 for mir-borrowck
matthewjasper Nov 27, 2017
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
82 changes: 50 additions & 32 deletions src/librustc_mir/borrow_check.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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,
Expand All @@ -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
}
Expand All @@ -167,6 +171,7 @@ pub struct InProgress<'b, 'gcx: 'tcx, 'tcx: 'b> {
inits: FlowInProgress<MaybeInitializedLvals<'b, 'gcx, 'tcx>>,
uninits: FlowInProgress<MaybeUninitializedLvals<'b, 'gcx, 'tcx>>,
move_outs: FlowInProgress<MovingOutStatements<'b, 'gcx, 'tcx>>,
ever_inits: FlowInProgress<EverInitializedLvals<'b, 'gcx, 'tcx>>,
}

struct FlowInProgress<BD> where BD: BitDenotation {
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand Down Expand Up @@ -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;
}
}
}
}
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -1852,30 +1854,35 @@ impl<'b, 'gcx, 'tcx> InProgress<'b, 'gcx, 'tcx> {
pub(super) fn new(borrows: DataflowResults<Borrows<'b, 'gcx, 'tcx>>,
inits: DataflowResults<MaybeInitializedLvals<'b, 'gcx, 'tcx>>,
uninits: DataflowResults<MaybeUninitializedLvals<'b, 'gcx, 'tcx>>,
move_out: DataflowResults<MovingOutStatements<'b, 'gcx, 'tcx>>)
move_out: DataflowResults<MovingOutStatements<'b, 'gcx, 'tcx>>,
ever_inits: DataflowResults<EverInitializedLvals<'b, 'gcx, 'tcx>>)
-> 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<XB, XI, XU, XM>(&mut self,
fn each_flow<XB, XI, XU, XM, XE>(&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<Borrows<'b, 'gcx, 'tcx>>),
XI: FnMut(&mut FlowInProgress<MaybeInitializedLvals<'b, 'gcx, 'tcx>>),
XU: FnMut(&mut FlowInProgress<MaybeUninitializedLvals<'b, 'gcx, 'tcx>>),
XM: FnMut(&mut FlowInProgress<MovingOutStatements<'b, 'gcx, 'tcx>>),
XE: FnMut(&mut FlowInProgress<EverInitializedLvals<'b, 'gcx, 'tcx>>),
{
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 {
Expand Down Expand Up @@ -1932,6 +1939,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;
Expand Down
75 changes: 34 additions & 41 deletions src/librustc_mir/dataflow/drop_flag_effects.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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 => (),
}
}
}
Loading