Skip to content

Commit bc61541

Browse files
committed
Make us error consistently in issue rust-lang#21232, to fix rust-lang#54986.
Treat attempt to partially intialize local `l` as uses of a `mut` in `let mut l;`, to fix rust-lang#54499.
1 parent 44a2f68 commit bc61541

File tree

2 files changed

+109
-20
lines changed

2 files changed

+109
-20
lines changed

Diff for: src/librustc_mir/borrow_check/error_reporting.rs

+13-12
Original file line numberDiff line numberDiff line change
@@ -51,16 +51,16 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
5151
&mut self,
5252
context: Context,
5353
desired_action: InitializationRequiringAction,
54-
(place, span): (&Place<'tcx>, Span),
54+
(moved_place, used_place, span): (&Place<'tcx>, &Place<'tcx>, Span),
5555
mpi: MovePathIndex,
5656
) {
5757
debug!(
58-
"report_use_of_moved_or_uninitialized: context={:?} desired_action={:?} place={:?} \
59-
span={:?} mpi={:?}",
60-
context, desired_action, place, span, mpi
58+
"report_use_of_moved_or_uninitialized: context={:?} desired_action={:?} \
59+
moved_place={:?} used_place={:?} span={:?} mpi={:?}",
60+
context, desired_action, moved_place, used_place, span, mpi
6161
);
6262

63-
let use_spans = self.move_spans(place, context.loc)
63+
let use_spans = self.move_spans(moved_place, context.loc)
6464
.or_else(|| self.borrow_spans(span, context.loc));
6565
let span = use_spans.args_or_use();
6666

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

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

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

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

92-
let item_msg = match self.describe_place_with_options(place, IncludingDowncast(true)) {
92+
let item_msg = match self.describe_place_with_options(used_place,
93+
IncludingDowncast(true)) {
9394
Some(name) => format!("`{}`", name),
9495
None => "value".to_owned(),
9596
};
9697
let mut err = self.infcx.tcx.cannot_act_on_uninitialized_variable(
9798
span,
9899
desired_action.as_noun(),
99-
&self.describe_place_with_options(place, IncludingDowncast(true))
100+
&self.describe_place_with_options(moved_place, IncludingDowncast(true))
100101
.unwrap_or("_".to_owned()),
101102
Origin::Mir,
102103
);
@@ -111,7 +112,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
111112
} else {
112113
if let Some((reported_place, _)) = self.move_error_reported.get(&move_out_indices) {
113114
if self.prefixes(&reported_place, PrefixSet::All)
114-
.any(|p| p == place)
115+
.any(|p| p == used_place)
115116
{
116117
debug!(
117118
"report_use_of_moved_or_uninitialized place: error suppressed \
@@ -128,7 +129,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
128129
span,
129130
desired_action.as_noun(),
130131
msg,
131-
self.describe_place_with_options(&place, IncludingDowncast(true)),
132+
self.describe_place_with_options(&moved_place, IncludingDowncast(true)),
132133
Origin::Mir,
133134
);
134135

@@ -181,7 +182,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
181182
);
182183
}
183184

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

221222
if let Some((_, mut old_err)) = self.move_error_reported
222-
.insert(move_out_indices, (place.clone(), err))
223+
.insert(move_out_indices, (used_place.clone(), err))
223224
{
224225
// Cancel the old error so it doesn't ICE.
225226
old_err.cancel();

Diff for: src/librustc_mir/borrow_check/mod.rs

+96-8
Original file line numberDiff line numberDiff line change
@@ -853,6 +853,7 @@ enum InitializationRequiringAction {
853853
MatchOn,
854854
Use,
855855
Assignment,
856+
PartialAssignment,
856857
}
857858

858859
struct RootPlace<'d, 'tcx: 'd> {
@@ -868,6 +869,7 @@ impl InitializationRequiringAction {
868869
InitializationRequiringAction::MatchOn => "use", // no good noun
869870
InitializationRequiringAction::Use => "use",
870871
InitializationRequiringAction::Assignment => "assign",
872+
InitializationRequiringAction::PartialAssignment => "assign to part",
871873
}
872874
}
873875

@@ -878,6 +880,7 @@ impl InitializationRequiringAction {
878880
InitializationRequiringAction::MatchOn => "matched on",
879881
InitializationRequiringAction::Use => "used",
880882
InitializationRequiringAction::Assignment => "assigned",
883+
InitializationRequiringAction::PartialAssignment => "partially assigned",
881884
}
882885
}
883886
}
@@ -1498,12 +1501,12 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
14981501

14991502
debug!("check_if_full_path_is_moved place: {:?}", place_span.0);
15001503
match self.move_path_closest_to(place_span.0) {
1501-
Ok(mpi) => {
1504+
Ok((prefix, mpi)) => {
15021505
if maybe_uninits.contains(mpi) {
15031506
self.report_use_of_moved_or_uninitialized(
15041507
context,
15051508
desired_action,
1506-
place_span,
1509+
(prefix, place_span.0, place_span.1),
15071510
mpi,
15081511
);
15091512
return; // don't bother finding other problems.
@@ -1561,7 +1564,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
15611564
self.report_use_of_moved_or_uninitialized(
15621565
context,
15631566
desired_action,
1564-
place_span,
1567+
(place_span.0, place_span.0, place_span.1),
15651568
child_mpi,
15661569
);
15671570
return; // don't bother finding other problems.
@@ -1579,14 +1582,14 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
15791582
/// An Err result includes a tag indicated why the search failed.
15801583
/// Currently this can only occur if the place is built off of a
15811584
/// static variable, as we do not track those in the MoveData.
1582-
fn move_path_closest_to(
1585+
fn move_path_closest_to<'a>(
15831586
&mut self,
1584-
place: &Place<'tcx>,
1585-
) -> Result<MovePathIndex, NoMovePathFound> {
1587+
place: &'a Place<'tcx>,
1588+
) -> Result<(&'a Place<'tcx>, MovePathIndex), NoMovePathFound> where 'cx: 'a {
15861589
let mut last_prefix = place;
15871590
for prefix in self.prefixes(place, PrefixSet::All) {
15881591
if let Some(mpi) = self.move_path_for_place(prefix) {
1589-
return Ok(mpi);
1592+
return Ok((prefix, mpi));
15901593
}
15911594
last_prefix = prefix;
15921595
}
@@ -1667,6 +1670,26 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
16671670
// recur further)
16681671
break;
16691672
}
1673+
1674+
1675+
// Once `let s; s.x = V; read(s.x);`,
1676+
// is allowed, remove this match arm.
1677+
ty::Adt(..) | ty::Tuple(..) => {
1678+
check_parent_of_field(self, context, base, span, flow_state);
1679+
1680+
if let Some(local) = place.base_local() {
1681+
// rust-lang/rust#21232,
1682+
// #54499, #54986: during
1683+
// period where we reject
1684+
// partial initialization, do
1685+
// not complain about
1686+
// unnecessary `mut` on an
1687+
// attempt to do a partial
1688+
// initialization.
1689+
self.used_mut.insert(local);
1690+
}
1691+
}
1692+
16701693
_ => {}
16711694
}
16721695
}
@@ -1677,8 +1700,73 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
16771700
}
16781701
}
16791702
}
1680-
}
16811703

1704+
fn check_parent_of_field<'cx, 'gcx, 'tcx>(this: &mut MirBorrowckCtxt<'cx, 'gcx, 'tcx>,
1705+
context: Context,
1706+
base: &Place<'tcx>,
1707+
span: Span,
1708+
flow_state: &Flows<'cx, 'gcx, 'tcx>)
1709+
{
1710+
// rust-lang/rust#21232: Until Rust allows reads from the
1711+
// initialized parts of partially initialized structs, we
1712+
// will, starting with the 2018 edition, reject attempts
1713+
// to write to structs that are not fully initialized.
1714+
//
1715+
// In other words, *until* we allow this:
1716+
//
1717+
// 1. `let mut s; s.x = Val; read(s.x);`
1718+
//
1719+
// we will for now disallow this:
1720+
//
1721+
// 2. `let mut s; s.x = Val;`
1722+
//
1723+
// and also this:
1724+
//
1725+
// 3. `let mut s = ...; drop(s); s.x=Val;`
1726+
//
1727+
// This does not use check_if_path_or_subpath_is_moved,
1728+
// because we want to *allow* reinitializations of fields:
1729+
// e.g. want to allow
1730+
//
1731+
// `let mut s = ...; drop(s.x); s.x=Val;`
1732+
//
1733+
// This does not use check_if_full_path_is_moved on
1734+
// `base`, because that would report an error about the
1735+
// `base` as a whole, but in this scenario we *really*
1736+
// want to report an error about the actual thing that was
1737+
// moved, which may be some prefix of `base`.
1738+
1739+
// Shallow so that we'll stop at any dereference; we'll
1740+
// report errors about issues with such bases elsewhere.
1741+
let maybe_uninits = &flow_state.uninits;
1742+
1743+
// Find the shortest uninitialized prefix you can reach
1744+
// without going over a Deref.
1745+
let mut shortest_uninit_seen = None;
1746+
for prefix in this.prefixes(base, PrefixSet::Shallow) {
1747+
let mpi = match this.move_path_for_place(prefix) {
1748+
Some(mpi) => mpi, None => continue,
1749+
};
1750+
1751+
if maybe_uninits.contains(mpi) {
1752+
debug!("check_parent_of_field updating shortest_uninit_seen from {:?} to {:?}",
1753+
shortest_uninit_seen, Some((prefix, mpi)));
1754+
shortest_uninit_seen = Some((prefix, mpi));
1755+
} else {
1756+
debug!("check_parent_of_field {:?} is definitely initialized", (prefix, mpi));
1757+
}
1758+
}
1759+
1760+
if let Some((prefix, mpi)) = shortest_uninit_seen {
1761+
this.report_use_of_moved_or_uninitialized(
1762+
context,
1763+
InitializationRequiringAction::PartialAssignment,
1764+
(prefix, base, span),
1765+
mpi,
1766+
);
1767+
}
1768+
}
1769+
}
16821770

16831771
/// Check the permissions for the given place and read or write kind
16841772
///

0 commit comments

Comments
 (0)