Skip to content

Commit

Permalink
Make us error consistently in issue rust-lang#21232, to fix rust-lang…
Browse files Browse the repository at this point in the history 
…#54986.

Treat attempt to partially intialize local `l` as uses of a `mut` in `let mut l;`, to fix rust-lang#54499.
  • Loading branch information
@pnkfelix
pnkfelix committed Oct 16, 2018
1 parent 44a2f68 commit bc61541
Show file tree
Hide file tree
Showing 2 changed files with 109 additions and 20 deletions.
25 changes: 13 additions & 12 deletions src/librustc_mir/borrow_check/error_reporting.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,16 +51,16 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
&mut self,
context: Context,
desired_action: InitializationRequiringAction,
(place, span): (&Place<'tcx>, Span),
(moved_place, used_place, span): (&Place<'tcx>, &Place<'tcx>, Span),
mpi: MovePathIndex,
) {
debug!(
"report_use_of_moved_or_uninitialized: context={:?} desired_action={:?} place={:?} \
span={:?} mpi={:?}",
context, desired_action, place, span, mpi
"report_use_of_moved_or_uninitialized: context={:?} desired_action={:?} \
moved_place={:?} used_place={:?} span={:?} mpi={:?}",
context, desired_action, moved_place, used_place, span, mpi
);

let use_spans = self.move_spans(place, context.loc)
let use_spans = self.move_spans(moved_place, context.loc)
.or_else(|| self.borrow_spans(span, context.loc));
let span = use_spans.args_or_use();

Expand All @@ -75,7 +75,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
.collect();

if move_out_indices.is_empty() {
let root_place = self.prefixes(&place, PrefixSet::All).last().unwrap();
let root_place = self.prefixes(&used_place, PrefixSet::All).last().unwrap();

if self.uninitialized_error_reported
.contains(&root_place.clone())
Expand All @@ -89,14 +89,15 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {

self.uninitialized_error_reported.insert(root_place.clone());

let item_msg = match self.describe_place_with_options(place, IncludingDowncast(true)) {
let item_msg = match self.describe_place_with_options(used_place,
IncludingDowncast(true)) {
Some(name) => format!("`{}`", name),
None => "value".to_owned(),
};
let mut err = self.infcx.tcx.cannot_act_on_uninitialized_variable(
span,
desired_action.as_noun(),
&self.describe_place_with_options(place, IncludingDowncast(true))
&self.describe_place_with_options(moved_place, IncludingDowncast(true))
.unwrap_or("_".to_owned()),
Origin::Mir,
);
Expand All @@ -111,7 +112,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
} else {
if let Some((reported_place, _)) = self.move_error_reported.get(&move_out_indices) {
if self.prefixes(&reported_place, PrefixSet::All)
.any(|p| p == place)
.any(|p| p == used_place)
{
debug!(
"report_use_of_moved_or_uninitialized place: error suppressed \
Expand All @@ -128,7 +129,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
span,
desired_action.as_noun(),
msg,
self.describe_place_with_options(&place, IncludingDowncast(true)),
self.describe_place_with_options(&moved_place, IncludingDowncast(true)),
Origin::Mir,
);

Expand Down Expand Up @@ -181,7 +182,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
);
}

if let Some(ty) = self.retrieve_type_for_place(place) {
if let Some(ty) = self.retrieve_type_for_place(used_place) {
let needs_note = match ty.sty {
ty::Closure(id, _) => {
let tables = self.infcx.tcx.typeck_tables_of(id);
Expand Down Expand Up @@ -219,7 +220,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
}

if let Some((_, mut old_err)) = self.move_error_reported
.insert(move_out_indices, (place.clone(), err))
.insert(move_out_indices, (used_place.clone(), err))
{
// Cancel the old error so it doesn't ICE.
old_err.cancel();
Expand Down
104 changes: 96 additions & 8 deletions src/librustc_mir/borrow_check/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -853,6 +853,7 @@ enum InitializationRequiringAction {
MatchOn,
Use,
Assignment,
PartialAssignment,
}

struct RootPlace<'d, 'tcx: 'd> {
Expand All @@ -868,6 +869,7 @@ impl InitializationRequiringAction {
InitializationRequiringAction::MatchOn => "use", // no good noun
InitializationRequiringAction::Use => "use",
InitializationRequiringAction::Assignment => "assign",
InitializationRequiringAction::PartialAssignment => "assign to part",
}
}

Expand All @@ -878,6 +880,7 @@ impl InitializationRequiringAction {
InitializationRequiringAction::MatchOn => "matched on",
InitializationRequiringAction::Use => "used",
InitializationRequiringAction::Assignment => "assigned",
InitializationRequiringAction::PartialAssignment => "partially assigned",
}
}
}
Expand Down Expand Up @@ -1498,12 +1501,12 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {

debug!("check_if_full_path_is_moved place: {:?}", place_span.0);
match self.move_path_closest_to(place_span.0) {
Ok(mpi) => {
Ok((prefix, mpi)) => {
if maybe_uninits.contains(mpi) {
self.report_use_of_moved_or_uninitialized(
context,
desired_action,
place_span,
(prefix, place_span.0, place_span.1),
mpi,
);
return; // don't bother finding other problems.
Expand Down Expand Up @@ -1561,7 +1564,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
self.report_use_of_moved_or_uninitialized(
context,
desired_action,
place_span,
(place_span.0, place_span.0, place_span.1),
child_mpi,
);
return; // don't bother finding other problems.
Expand All @@ -1579,14 +1582,14 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
/// An Err result includes a tag indicated why the search failed.
/// Currently this can only occur if the place is built off of a
/// static variable, as we do not track those in the MoveData.
fn move_path_closest_to(
fn move_path_closest_to<'a>(
&mut self,
place: &Place<'tcx>,
) -> Result<MovePathIndex, NoMovePathFound> {
place: &'a Place<'tcx>,
) -> Result<(&'a Place<'tcx>, MovePathIndex), NoMovePathFound> where 'cx: 'a {
let mut last_prefix = place;
for prefix in self.prefixes(place, PrefixSet::All) {
if let Some(mpi) = self.move_path_for_place(prefix) {
return Ok(mpi);
return Ok((prefix, mpi));
}
last_prefix = prefix;
}
Expand Down Expand Up @@ -1667,6 +1670,26 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
// recur further)
break;
}


// Once `let s; s.x = V; read(s.x);`,
// is allowed, remove this match arm.
ty::Adt(..) | ty::Tuple(..) => {
check_parent_of_field(self, context, base, span, flow_state);

if let Some(local) = place.base_local() {
// rust-lang/rust#21232,
// #54499, #54986: during
// period where we reject
// partial initialization, do
// not complain about
// unnecessary `mut` on an
// attempt to do a partial
// initialization.
self.used_mut.insert(local);
}
}

_ => {}
}
}
Expand All @@ -1677,8 +1700,73 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
}
}
}
}

fn check_parent_of_field<'cx, 'gcx, 'tcx>(this: &mut MirBorrowckCtxt<'cx, 'gcx, 'tcx>,
context: Context,
base: &Place<'tcx>,
span: Span,
flow_state: &Flows<'cx, 'gcx, 'tcx>)
{
// rust-lang/rust#21232: Until Rust allows reads from the
// initialized parts of partially initialized structs, we
// will, starting with the 2018 edition, reject attempts
// to write to structs that are not fully initialized.
//
// In other words, *until* we allow this:
//
// 1. `let mut s; s.x = Val; read(s.x);`
//
// we will for now disallow this:
//
// 2. `let mut s; s.x = Val;`
//
// and also this:
//
// 3. `let mut s = ...; drop(s); s.x=Val;`
//
// This does not use check_if_path_or_subpath_is_moved,
// because we want to *allow* reinitializations of fields:
// e.g. want to allow
//
// `let mut s = ...; drop(s.x); s.x=Val;`
//
// This does not use check_if_full_path_is_moved on
// `base`, because that would report an error about the
// `base` as a whole, but in this scenario we *really*
// want to report an error about the actual thing that was
// moved, which may be some prefix of `base`.

// Shallow so that we'll stop at any dereference; we'll
// report errors about issues with such bases elsewhere.
let maybe_uninits = &flow_state.uninits;

// Find the shortest uninitialized prefix you can reach
// without going over a Deref.
let mut shortest_uninit_seen = None;
for prefix in this.prefixes(base, PrefixSet::Shallow) {
let mpi = match this.move_path_for_place(prefix) {
Some(mpi) => mpi, None => continue,
};

if maybe_uninits.contains(mpi) {
debug!("check_parent_of_field updating shortest_uninit_seen from {:?} to {:?}",
shortest_uninit_seen, Some((prefix, mpi)));
shortest_uninit_seen = Some((prefix, mpi));
} else {
debug!("check_parent_of_field {:?} is definitely initialized", (prefix, mpi));
}
}

if let Some((prefix, mpi)) = shortest_uninit_seen {
this.report_use_of_moved_or_uninitialized(
context,
InitializationRequiringAction::PartialAssignment,
(prefix, base, span),
mpi,
);
}
}
}

/// Check the permissions for the given place and read or write kind
///
Expand Down

0 comments on commit bc61541

Please sign in to comment.