Skip to content

Commit

Permalink
Fix diagnostic issue when using FakeReads in closures
Browse files Browse the repository at this point in the history
  • Loading branch information
@roxelo
roxelo committed Mar 30, 2021
1 parent cb473c2 commit c3cf93a
Show file tree
Hide file tree
Showing 31 changed files with 120 additions and 81 deletions.
16 changes: 13 additions & 3 deletions compiler/rustc_middle/src/mir/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1470,7 +1470,7 @@ pub struct Statement<'tcx> {

// `Statement` is used a lot. Make sure it doesn't unintentionally get bigger.
#[cfg(all(target_arch = "x86_64", target_pointer_width = "64"))]
static_assert_size!(Statement<'_>, 32);
static_assert_size!(Statement<'_>, 40);

impl Statement<'_> {
/// Changes a statement to a nop. This is both faster than deleting instructions and avoids
Expand Down Expand Up @@ -1593,7 +1593,12 @@ pub enum FakeReadCause {

/// `let x: !; match x {}` doesn't generate any read of x so we need to
/// generate a read of x to check that it is initialized and safe.
ForMatchedPlace,
///
/// If a closure pattern matches a Place starting with an Upvar, then we introduce a
/// FakeRead for that Place outside the closure, in such a case this option would be
/// Some(closure_def_id).
/// Otherwise, the value of the optional DefId will be None.
ForMatchedPlace(Option<DefId>),

/// A fake read of the RefWithinGuard version of a bind-by-value variable
/// in a match guard to ensure that it's value hasn't change by the time
Expand All @@ -1612,7 +1617,12 @@ pub enum FakeReadCause {
/// but in some cases it can affect the borrow checker, as in #53695.
/// Therefore, we insert a "fake read" here to ensure that we get
/// appropriate errors.
ForLet,
///
/// If a closure pattern matches a Place starting with an Upvar, then we introduce a
/// FakeRead for that Place outside the closure, in such a case this option would be
/// Some(closure_def_id).
/// Otherwise, the value of the optional DefId will be None.
ForLet(Option<DefId>),

/// If we have an index expression like
///
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_mir/src/borrow_check/diagnostics/explain_borrow.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -515,7 +515,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
let block = &self.body.basic_blocks()[location.block];

let kind = if let Some(&Statement {
kind: StatementKind::FakeRead(FakeReadCause::ForLet, _),
kind: StatementKind::FakeRead(FakeReadCause::ForLet(_), _),
..
}) = block.statements.get(location.statement_index)
{
Expand Down
22 changes: 20 additions & 2 deletions compiler/rustc_mir/src/borrow_check/diagnostics/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@ use rustc_hir::def_id::DefId;
use rustc_hir::lang_items::LangItemGroup;
use rustc_hir::GeneratorKind;
use rustc_middle::mir::{
AggregateKind, Constant, Field, Local, LocalInfo, LocalKind, Location, Operand, Place,
PlaceRef, ProjectionElem, Rvalue, Statement, StatementKind, Terminator, TerminatorKind,
AggregateKind, Constant, FakeReadCause, Field, Local, LocalInfo, LocalKind, Location, Operand,
Place, PlaceRef, ProjectionElem, Rvalue, Statement, StatementKind, Terminator, TerminatorKind,
};
use rustc_middle::ty::print::Print;
use rustc_middle::ty::{self, DefIdTree, Instance, Ty, TyCtxt};
Expand Down Expand Up @@ -794,6 +794,24 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
}
}

// StatementKind::FakeRead only contains a def_id if they are introduced as a result
// of pattern matching within a closure.
if let StatementKind::FakeRead(cause, box ref place) = stmt.kind {
match cause {
FakeReadCause::ForMatchedPlace(Some(closure_def_id))
| FakeReadCause::ForLet(Some(closure_def_id)) => {
debug!("move_spans: def_id={:?} place={:?}", closure_def_id, place);
let places = &[Operand::Move(*place)];
if let Some((args_span, generator_kind, var_span)) =
self.closure_span(closure_def_id, moved_place, places)
{
return ClosureUse { generator_kind, args_span, var_span };
}
}
_ => {}
}
}

let normal_ret =
if moved_place.projection.iter().any(|p| matches!(p, ProjectionElem::Downcast(..))) {
PatUse(stmt.source_info.span)
Expand Down
32 changes: 14 additions & 18 deletions compiler/rustc_mir_build/src/build/expr/as_rvalue.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -179,24 +179,20 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
// match x { _ => () } // fake read of `x`
// };
// ```
// FIXME(RFC2229): Remove feature gate once diagnostics are improved
if this.tcx.features().capture_disjoint_fields {
for (thir_place, cause, hir_id) in fake_reads.into_iter() {
let place_builder =
unpack!(block = this.as_place_builder(block, thir_place));

if let Ok(place_builder_resolved) =
place_builder.try_upvars_resolved(this.tcx, this.typeck_results)
{
let mir_place =
place_builder_resolved.into_place(this.tcx, this.typeck_results);
this.cfg.push_fake_read(
block,
this.source_info(this.tcx.hir().span(*hir_id)),
*cause,
mir_place,
);
}
for (thir_place, cause, hir_id) in fake_reads.into_iter() {
let place_builder = unpack!(block = this.as_place_builder(block, thir_place));

if let Ok(place_builder_resolved) =
place_builder.try_upvars_resolved(this.tcx, this.typeck_results)
{
let mir_place =
place_builder_resolved.into_place(this.tcx, this.typeck_results);
this.cfg.push_fake_read(
block,
this.source_info(this.tcx.hir().span(*hir_id)),
*cause,
mir_place,
);
}
}

Expand Down
6 changes: 3 additions & 3 deletions compiler/rustc_mir_build/src/build/matches/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
// uninhabited value. If we get never patterns, those will check that
// the place is initialized, and so this read would only be used to
// check safety.
let cause_matched_place = FakeReadCause::ForMatchedPlace;
let cause_matched_place = FakeReadCause::ForMatchedPlace(None);
let source_info = self.source_info(scrutinee_span);

if let Ok(scrutinee_builder) =
Expand Down Expand Up @@ -400,7 +400,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {

// Inject a fake read, see comments on `FakeReadCause::ForLet`.
let source_info = self.source_info(irrefutable_pat.span);
self.cfg.push_fake_read(block, source_info, FakeReadCause::ForLet, place);
self.cfg.push_fake_read(block, source_info, FakeReadCause::ForLet(None), place);

self.schedule_drop_for_binding(var, irrefutable_pat.span, OutsideGuard);
block.unit()
Expand Down Expand Up @@ -435,7 +435,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {

// Inject a fake read, see comments on `FakeReadCause::ForLet`.
let pattern_source_info = self.source_info(irrefutable_pat.span);
let cause_let = FakeReadCause::ForLet;
let cause_let = FakeReadCause::ForLet(None);
self.cfg.push_fake_read(block, pattern_source_info, cause_let, place);

let ty_source_info = self.source_info(user_ty_span);
Expand Down
21 changes: 18 additions & 3 deletions compiler/rustc_typeck/src/expr_use_visitor.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -279,9 +279,14 @@ impl<'a, 'tcx> ExprUseVisitor<'a, 'tcx> {
if needs_to_be_read {
self.borrow_expr(&discr, ty::ImmBorrow);
} else {
let closure_def_id = match discr_place.place.base {
PlaceBase::Upvar(upvar_id) => Some(upvar_id.closure_expr_id.to_def_id()),
_ => None,
};

self.delegate.fake_read(
discr_place.place.clone(),
FakeReadCause::ForMatchedPlace,
FakeReadCause::ForMatchedPlace(closure_def_id),
discr_place.hir_id,
);

Expand Down Expand Up @@ -577,9 +582,14 @@ impl<'a, 'tcx> ExprUseVisitor<'a, 'tcx> {
}

fn walk_arm(&mut self, discr_place: &PlaceWithHirId<'tcx>, arm: &hir::Arm<'_>) {
let closure_def_id = match discr_place.place.base {
PlaceBase::Upvar(upvar_id) => Some(upvar_id.closure_expr_id.to_def_id()),
_ => None,
};

self.delegate.fake_read(
discr_place.place.clone(),
FakeReadCause::ForMatchedPlace,
FakeReadCause::ForMatchedPlace(closure_def_id),
discr_place.hir_id,
);
self.walk_pat(discr_place, &arm.pat);
Expand All @@ -594,9 +604,14 @@ impl<'a, 'tcx> ExprUseVisitor<'a, 'tcx> {
/// Walks a pat that occurs in isolation (i.e., top-level of fn argument or
/// let binding, and *not* a match arm or nested pat.)
fn walk_irrefutable_pat(&mut self, discr_place: &PlaceWithHirId<'tcx>, pat: &hir::Pat<'_>) {
let closure_def_id = match discr_place.place.base {
PlaceBase::Upvar(upvar_id) => Some(upvar_id.closure_expr_id.to_def_id()),
_ => None,
};

self.delegate.fake_read(
discr_place.place.clone(),
FakeReadCause::ForLet,
FakeReadCause::ForLet(closure_def_id),
discr_place.hir_id,
);
self.walk_pat(discr_place, pat);
Expand Down
28 changes: 14 additions & 14 deletions src/test/mir-opt/address_of.address_of_reborrow.SimplifyCfg-initial.after.mir
View file
Original file line number Diff line number Diff line change
Expand Up @@ -130,12 +130,12 @@ fn address_of_reborrow() -> () {
StorageLive(_2); // scope 0 at $DIR/address-of.rs:4:14: 4:21
_2 = [const 0_i32; 10]; // scope 0 at $DIR/address-of.rs:4:14: 4:21
_1 = &_2; // scope 0 at $DIR/address-of.rs:4:13: 4:21
FakeRead(ForLet, _1); // scope 0 at $DIR/address-of.rs:4:9: 4:10
FakeRead(ForLet(None), _1); // scope 0 at $DIR/address-of.rs:4:9: 4:10
StorageLive(_3); // scope 1 at $DIR/address-of.rs:5:9: 5:14
StorageLive(_4); // scope 1 at $DIR/address-of.rs:5:22: 5:29
_4 = [const 0_i32; 10]; // scope 1 at $DIR/address-of.rs:5:22: 5:29
_3 = &mut _4; // scope 1 at $DIR/address-of.rs:5:17: 5:29
FakeRead(ForLet, _3); // scope 1 at $DIR/address-of.rs:5:9: 5:14
FakeRead(ForLet(None), _3); // scope 1 at $DIR/address-of.rs:5:9: 5:14
StorageLive(_5); // scope 2 at $DIR/address-of.rs:7:5: 7:18
StorageLive(_6); // scope 2 at $DIR/address-of.rs:7:5: 7:18
_6 = &raw const (*_1); // scope 2 at $DIR/address-of.rs:7:5: 7:6
Expand Down Expand Up @@ -170,25 +170,25 @@ fn address_of_reborrow() -> () {
StorageDead(_13); // scope 2 at $DIR/address-of.rs:11:20: 11:21
StorageLive(_15); // scope 2 at $DIR/address-of.rs:13:9: 13:10
_15 = &raw const (*_1); // scope 2 at $DIR/address-of.rs:13:23: 13:24
FakeRead(ForLet, _15); // scope 2 at $DIR/address-of.rs:13:9: 13:10
FakeRead(ForLet(None), _15); // scope 2 at $DIR/address-of.rs:13:9: 13:10
AscribeUserType(_15, o, UserTypeProjection { base: UserType(3), projs: [] }); // scope 2 at $DIR/address-of.rs:13:12: 13:20
StorageLive(_16); // scope 3 at $DIR/address-of.rs:14:9: 14:10
_16 = &raw const (*_1); // scope 3 at $DIR/address-of.rs:14:31: 14:32
FakeRead(ForLet, _16); // scope 3 at $DIR/address-of.rs:14:9: 14:10
FakeRead(ForLet(None), _16); // scope 3 at $DIR/address-of.rs:14:9: 14:10
AscribeUserType(_16, o, UserTypeProjection { base: UserType(5), projs: [] }); // scope 3 at $DIR/address-of.rs:14:12: 14:28
StorageLive(_17); // scope 4 at $DIR/address-of.rs:15:9: 15:10
StorageLive(_18); // scope 4 at $DIR/address-of.rs:15:30: 15:31
_18 = &raw const (*_1); // scope 4 at $DIR/address-of.rs:15:30: 15:31
_17 = move _18 as *const dyn std::marker::Send (Pointer(Unsize)); // scope 4 at $DIR/address-of.rs:15:30: 15:31
StorageDead(_18); // scope 4 at $DIR/address-of.rs:15:30: 15:31
FakeRead(ForLet, _17); // scope 4 at $DIR/address-of.rs:15:9: 15:10
FakeRead(ForLet(None), _17); // scope 4 at $DIR/address-of.rs:15:9: 15:10
AscribeUserType(_17, o, UserTypeProjection { base: UserType(7), projs: [] }); // scope 4 at $DIR/address-of.rs:15:12: 15:27
StorageLive(_19); // scope 5 at $DIR/address-of.rs:16:9: 16:10
StorageLive(_20); // scope 5 at $DIR/address-of.rs:16:27: 16:28
_20 = &raw const (*_1); // scope 5 at $DIR/address-of.rs:16:27: 16:28
_19 = move _20 as *const [i32] (Pointer(Unsize)); // scope 5 at $DIR/address-of.rs:16:27: 16:28
StorageDead(_20); // scope 5 at $DIR/address-of.rs:16:27: 16:28
FakeRead(ForLet, _19); // scope 5 at $DIR/address-of.rs:16:9: 16:10
FakeRead(ForLet(None), _19); // scope 5 at $DIR/address-of.rs:16:9: 16:10
AscribeUserType(_19, o, UserTypeProjection { base: UserType(9), projs: [] }); // scope 5 at $DIR/address-of.rs:16:12: 16:24
StorageLive(_21); // scope 6 at $DIR/address-of.rs:18:5: 18:18
StorageLive(_22); // scope 6 at $DIR/address-of.rs:18:5: 18:18
Expand Down Expand Up @@ -218,25 +218,25 @@ fn address_of_reborrow() -> () {
StorageDead(_27); // scope 6 at $DIR/address-of.rs:21:22: 21:23
StorageLive(_29); // scope 6 at $DIR/address-of.rs:23:9: 23:10
_29 = &raw const (*_3); // scope 6 at $DIR/address-of.rs:23:23: 23:24
FakeRead(ForLet, _29); // scope 6 at $DIR/address-of.rs:23:9: 23:10
FakeRead(ForLet(None), _29); // scope 6 at $DIR/address-of.rs:23:9: 23:10
AscribeUserType(_29, o, UserTypeProjection { base: UserType(13), projs: [] }); // scope 6 at $DIR/address-of.rs:23:12: 23:20
StorageLive(_30); // scope 7 at $DIR/address-of.rs:24:9: 24:10
_30 = &raw const (*_3); // scope 7 at $DIR/address-of.rs:24:31: 24:32
FakeRead(ForLet, _30); // scope 7 at $DIR/address-of.rs:24:9: 24:10
FakeRead(ForLet(None), _30); // scope 7 at $DIR/address-of.rs:24:9: 24:10
AscribeUserType(_30, o, UserTypeProjection { base: UserType(15), projs: [] }); // scope 7 at $DIR/address-of.rs:24:12: 24:28
StorageLive(_31); // scope 8 at $DIR/address-of.rs:25:9: 25:10
StorageLive(_32); // scope 8 at $DIR/address-of.rs:25:30: 25:31
_32 = &raw const (*_3); // scope 8 at $DIR/address-of.rs:25:30: 25:31
_31 = move _32 as *const dyn std::marker::Send (Pointer(Unsize)); // scope 8 at $DIR/address-of.rs:25:30: 25:31
StorageDead(_32); // scope 8 at $DIR/address-of.rs:25:30: 25:31
FakeRead(ForLet, _31); // scope 8 at $DIR/address-of.rs:25:9: 25:10
FakeRead(ForLet(None), _31); // scope 8 at $DIR/address-of.rs:25:9: 25:10
AscribeUserType(_31, o, UserTypeProjection { base: UserType(17), projs: [] }); // scope 8 at $DIR/address-of.rs:25:12: 25:27
StorageLive(_33); // scope 9 at $DIR/address-of.rs:26:9: 26:10
StorageLive(_34); // scope 9 at $DIR/address-of.rs:26:27: 26:28
_34 = &raw const (*_3); // scope 9 at $DIR/address-of.rs:26:27: 26:28
_33 = move _34 as *const [i32] (Pointer(Unsize)); // scope 9 at $DIR/address-of.rs:26:27: 26:28
StorageDead(_34); // scope 9 at $DIR/address-of.rs:26:27: 26:28
FakeRead(ForLet, _33); // scope 9 at $DIR/address-of.rs:26:9: 26:10
FakeRead(ForLet(None), _33); // scope 9 at $DIR/address-of.rs:26:9: 26:10
AscribeUserType(_33, o, UserTypeProjection { base: UserType(19), projs: [] }); // scope 9 at $DIR/address-of.rs:26:12: 26:24
StorageLive(_35); // scope 10 at $DIR/address-of.rs:28:5: 28:16
StorageLive(_36); // scope 10 at $DIR/address-of.rs:28:5: 28:16
Expand Down Expand Up @@ -266,25 +266,25 @@ fn address_of_reborrow() -> () {
StorageDead(_41); // scope 10 at $DIR/address-of.rs:31:20: 31:21
StorageLive(_43); // scope 10 at $DIR/address-of.rs:33:9: 33:10
_43 = &raw mut (*_3); // scope 10 at $DIR/address-of.rs:33:21: 33:22
FakeRead(ForLet, _43); // scope 10 at $DIR/address-of.rs:33:9: 33:10
FakeRead(ForLet(None), _43); // scope 10 at $DIR/address-of.rs:33:9: 33:10
AscribeUserType(_43, o, UserTypeProjection { base: UserType(23), projs: [] }); // scope 10 at $DIR/address-of.rs:33:12: 33:18
StorageLive(_44); // scope 11 at $DIR/address-of.rs:34:9: 34:10
_44 = &raw mut (*_3); // scope 11 at $DIR/address-of.rs:34:29: 34:30
FakeRead(ForLet, _44); // scope 11 at $DIR/address-of.rs:34:9: 34:10
FakeRead(ForLet(None), _44); // scope 11 at $DIR/address-of.rs:34:9: 34:10
AscribeUserType(_44, o, UserTypeProjection { base: UserType(25), projs: [] }); // scope 11 at $DIR/address-of.rs:34:12: 34:26
StorageLive(_45); // scope 12 at $DIR/address-of.rs:35:9: 35:10
StorageLive(_46); // scope 12 at $DIR/address-of.rs:35:28: 35:29
_46 = &raw mut (*_3); // scope 12 at $DIR/address-of.rs:35:28: 35:29
_45 = move _46 as *mut dyn std::marker::Send (Pointer(Unsize)); // scope 12 at $DIR/address-of.rs:35:28: 35:29
StorageDead(_46); // scope 12 at $DIR/address-of.rs:35:28: 35:29
FakeRead(ForLet, _45); // scope 12 at $DIR/address-of.rs:35:9: 35:10
FakeRead(ForLet(None), _45); // scope 12 at $DIR/address-of.rs:35:9: 35:10
AscribeUserType(_45, o, UserTypeProjection { base: UserType(27), projs: [] }); // scope 12 at $DIR/address-of.rs:35:12: 35:25
StorageLive(_47); // scope 13 at $DIR/address-of.rs:36:9: 36:10
StorageLive(_48); // scope 13 at $DIR/address-of.rs:36:25: 36:26
_48 = &raw mut (*_3); // scope 13 at $DIR/address-of.rs:36:25: 36:26
_47 = move _48 as *mut [i32] (Pointer(Unsize)); // scope 13 at $DIR/address-of.rs:36:25: 36:26
StorageDead(_48); // scope 13 at $DIR/address-of.rs:36:25: 36:26
FakeRead(ForLet, _47); // scope 13 at $DIR/address-of.rs:36:9: 36:10
FakeRead(ForLet(None), _47); // scope 13 at $DIR/address-of.rs:36:9: 36:10
AscribeUserType(_47, o, UserTypeProjection { base: UserType(29), projs: [] }); // scope 13 at $DIR/address-of.rs:36:12: 36:22
_0 = const (); // scope 0 at $DIR/address-of.rs:3:26: 37:2
StorageDead(_47); // scope 13 at $DIR/address-of.rs:37:1: 37:2
Expand Down
Loading

0 comments on commit c3cf93a

Please sign in to comment.