Skip to content

Commit 4591a24

Browse files
borsbors
committed
Auto merge of #54188 - lqd:fallout-53695, r=nikomatsakis
NLL: disallow creation of immediately unusable variables Fix #53695 Original description follows ---- This WIP PR is for discussing the impact of fixing #53695 by injecting a fake read in let patterns. (Travis will fail, at least the `mir-opt` suite is failing in its current state)
2 parents af50e38 + 3bdba74 commit 4591a24

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

52 files changed

+185
-102
lines changed

Diff for: ‎src/librustc/ich/impls_mir.rs

+4-1
Original file line numberDiff line numberDiff line change
@@ -238,7 +238,8 @@ for mir::StatementKind<'gcx> {
238238
place.hash_stable(hcx, hasher);
239239
rvalue.hash_stable(hcx, hasher);
240240
}
241-
mir::StatementKind::ReadForMatch(ref place) => {
241+
mir::StatementKind::FakeRead(ref cause, ref place) => {
242+
cause.hash_stable(hcx, hasher);
242243
place.hash_stable(hcx, hasher);
243244
}
244245
mir::StatementKind::SetDiscriminant { ref place, variant_index } => {
@@ -271,6 +272,8 @@ for mir::StatementKind<'gcx> {
271272
}
272273
}
273274

275+
impl_stable_hash_for!(enum mir::FakeReadCause { ForMatch, ForLet });
276+
274277
impl<'a, 'gcx, T> HashStable<StableHashingContext<'a>>
275278
for mir::ValidationOperand<'gcx, T>
276279
where T: HashStable<StableHashingContext<'a>>

Diff for: ‎src/librustc/mir/mod.rs

+32-4
Original file line numberDiff line numberDiff line change
@@ -1613,8 +1613,10 @@ pub enum StatementKind<'tcx> {
16131613
Assign(Place<'tcx>, Rvalue<'tcx>),
16141614

16151615
/// This represents all the reading that a pattern match may do
1616-
/// (e.g. inspecting constants and discriminant values).
1617-
ReadForMatch(Place<'tcx>),
1616+
/// (e.g. inspecting constants and discriminant values), and the
1617+
/// kind of pattern it comes from. This is in order to adapt potential
1618+
/// error messages to these specific patterns.
1619+
FakeRead(FakeReadCause, Place<'tcx>),
16181620

16191621
/// Write the discriminant for a variant to the enum Place.
16201622
SetDiscriminant {
@@ -1662,6 +1664,31 @@ pub enum StatementKind<'tcx> {
16621664
Nop,
16631665
}
16641666

1667+
/// The `FakeReadCause` describes the type of pattern why a `FakeRead` statement exists.
1668+
#[derive(Copy, Clone, RustcEncodable, RustcDecodable, Debug)]
1669+
pub enum FakeReadCause {
1670+
/// Inject a fake read of the borrowed input at the start of each arm's
1671+
/// pattern testing code.
1672+
///
1673+
/// This should ensure that you cannot change the variant for an enum
1674+
/// while you are in the midst of matching on it.
1675+
ForMatch,
1676+
1677+
/// Officially, the semantics of
1678+
///
1679+
/// `let pattern = <expr>;`
1680+
///
1681+
/// is that `<expr>` is evaluated into a temporary and then this temporary is
1682+
/// into the pattern.
1683+
///
1684+
/// However, if we see the simple pattern `let var = <expr>`, we optimize this to
1685+
/// evaluate `<expr>` directly into the variable `var`. This is mostly unobservable,
1686+
/// but in some cases it can affect the borrow checker, as in #53695.
1687+
/// Therefore, we insert a "fake read" here to ensure that we get
1688+
/// appropriate errors.
1689+
ForLet,
1690+
}
1691+
16651692
/// The `ValidationOp` describes what happens with each of the operands of a
16661693
/// `Validate` statement.
16671694
#[derive(Copy, Clone, RustcEncodable, RustcDecodable, PartialEq, Eq)]
@@ -1718,7 +1745,7 @@ impl<'tcx> Debug for Statement<'tcx> {
17181745
use self::StatementKind::*;
17191746
match self.kind {
17201747
Assign(ref place, ref rv) => write!(fmt, "{:?} = {:?}", place, rv),
1721-
ReadForMatch(ref place) => write!(fmt, "ReadForMatch({:?})", place),
1748+
FakeRead(ref cause, ref place) => write!(fmt, "FakeRead({:?}, {:?})", cause, place),
17221749
// (reuse lifetime rendering policy from ppaux.)
17231750
EndRegion(ref ce) => write!(fmt, "EndRegion({})", ty::ReScope(*ce)),
17241751
Validate(ref op, ref places) => write!(fmt, "Validate({:?}, {:?})", op, places),
@@ -2585,6 +2612,7 @@ CloneTypeFoldableAndLiftImpls! {
25852612
Mutability,
25862613
SourceInfo,
25872614
UpvarDecl,
2615+
FakeReadCause,
25882616
ValidationOp,
25892617
SourceScope,
25902618
SourceScopeData,
@@ -2651,7 +2679,7 @@ BraceStructTypeFoldableImpl! {
26512679
EnumTypeFoldableImpl! {
26522680
impl<'tcx> TypeFoldable<'tcx> for StatementKind<'tcx> {
26532681
(StatementKind::Assign)(a, b),
2654-
(StatementKind::ReadForMatch)(place),
2682+
(StatementKind::FakeRead)(cause, place),
26552683
(StatementKind::SetDiscriminant) { place, variant_index },
26562684
(StatementKind::StorageLive)(a),
26572685
(StatementKind::StorageDead)(a),

Diff for: ‎src/librustc/mir/visit.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -354,7 +354,7 @@ macro_rules! make_mir_visitor {
354354
ref $($mutability)* rvalue) => {
355355
self.visit_assign(block, place, rvalue, location);
356356
}
357-
StatementKind::ReadForMatch(ref $($mutability)* place) => {
357+
StatementKind::FakeRead(_, ref $($mutability)* place) => {
358358
self.visit_place(place,
359359
PlaceContext::Inspect,
360360
location);

Diff for: ‎src/librustc/session/config.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -1330,7 +1330,7 @@ options! {DebuggingOptions, DebuggingSetter, basic_debugging_options,
13301330
disable_nll_user_type_assert: bool = (false, parse_bool, [UNTRACKED],
13311331
"disable user provided type assertion in NLL"),
13321332
nll_dont_emit_read_for_match: bool = (false, parse_bool, [UNTRACKED],
1333-
"in match codegen, do not include ReadForMatch statements (used by mir-borrowck)"),
1333+
"in match codegen, do not include FakeRead statements (used by mir-borrowck)"),
13341334
dont_buffer_diagnostics: bool = (false, parse_bool, [UNTRACKED],
13351335
"emit diagnostics rather than buffering (breaks NLL error downgrading, sorting)."),
13361336
polonius: bool = (false, parse_bool, [UNTRACKED],

Diff for: ‎src/librustc/ty/context.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -1505,7 +1505,7 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> {
15051505
self.emit_read_for_match()
15061506
}
15071507

1508-
/// If true, make MIR codegen for `match` emit ReadForMatch
1508+
/// If true, make MIR codegen for `match` emit FakeRead
15091509
/// statements (which simulate the maximal effect of executing the
15101510
/// patterns in a match arm).
15111511
pub fn emit_read_for_match(&self) -> bool {

Diff for: ‎src/librustc_codegen_llvm/mir/statement.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@ impl FunctionCx<'a, 'll, 'tcx> {
8989
asm::codegen_inline_asm(&bx, asm, outputs, input_vals);
9090
bx
9191
}
92-
mir::StatementKind::ReadForMatch(_) |
92+
mir::StatementKind::FakeRead(..) |
9393
mir::StatementKind::EndRegion(_) |
9494
mir::StatementKind::Validate(..) |
9595
mir::StatementKind::AscribeUserType(..) |

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

+16-1
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ use borrow_check::WriteKind;
1212
use rustc::middle::region::ScopeTree;
1313
use rustc::mir::VarBindingForm;
1414
use rustc::mir::{BindingForm, BorrowKind, ClearCrossCrate, Field, Local};
15-
use rustc::mir::{LocalDecl, LocalKind, Location, Operand, Place};
15+
use rustc::mir::{FakeReadCause, LocalDecl, LocalKind, Location, Operand, Place};
1616
use rustc::mir::{ProjectionElem, Rvalue, Statement, StatementKind};
1717
use rustc::ty;
1818
use rustc_data_structures::fx::FxHashSet;
@@ -1020,6 +1020,21 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
10201020
false
10211021
}
10221022
}
1023+
1024+
/// Returns the `FakeReadCause` at this location if it is a `FakeRead` statement.
1025+
pub(super) fn retrieve_fake_read_cause_for_location(
1026+
&self,
1027+
location: &Location,
1028+
) -> Option<FakeReadCause> {
1029+
let stmt = self.mir.basic_blocks()[location.block]
1030+
.statements
1031+
.get(location.statement_index)?;
1032+
if let StatementKind::FakeRead(cause, _) = stmt.kind {
1033+
Some(cause)
1034+
} else {
1035+
None
1036+
}
1037+
}
10231038
}
10241039

10251040
// The span(s) associated to a use of a place.

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

+3-3
Original file line numberDiff line numberDiff line change
@@ -501,9 +501,9 @@ impl<'cx, 'gcx, 'tcx> DataflowResultsConsumer<'cx, 'tcx> for MirBorrowckCtxt<'cx
501501
flow_state,
502502
);
503503
}
504-
StatementKind::ReadForMatch(ref place) => {
504+
StatementKind::FakeRead(_, ref place) => {
505505
self.access_place(
506-
ContextKind::ReadForMatch.new(location),
506+
ContextKind::FakeRead.new(location),
507507
(place, span),
508508
(Deep, Read(ReadKind::Borrow(BorrowKind::Shared))),
509509
LocalMutationIsAllowed::No,
@@ -2229,7 +2229,7 @@ enum ContextKind {
22292229
CallDest,
22302230
Assert,
22312231
Yield,
2232-
ReadForMatch,
2232+
FakeRead,
22332233
StorageDead,
22342234
}
22352235

Diff for: ‎src/librustc_mir/borrow_check/nll/explain_borrow/mod.rs

+8-2
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
use borrow_check::borrow_set::BorrowData;
1212
use borrow_check::nll::region_infer::Cause;
1313
use borrow_check::{Context, MirBorrowckCtxt, WriteKind};
14-
use rustc::mir::{Local, Location, Place, TerminatorKind};
14+
use rustc::mir::{FakeReadCause, Local, Location, Place, TerminatorKind};
1515
use rustc_errors::DiagnosticBuilder;
1616
use rustc::ty::Region;
1717

@@ -142,7 +142,13 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
142142
if spans.for_closure() {
143143
"borrow later captured here by closure"
144144
} else {
145-
"borrow later used here"
145+
// Check if the location represents a `FakeRead`, and adapt the error
146+
// message to the `FakeReadCause` it is from: in particular,
147+
// the ones inserted in optimized `let var = <expr>` patterns.
148+
match self.retrieve_fake_read_cause_for_location(&location) {
149+
Some(FakeReadCause::ForLet) => "borrow later stored here",
150+
_ => "borrow later used here"
151+
}
146152
}
147153
};
148154
err.span_label(spans.var_or_use(), message);

Diff for: ‎src/librustc_mir/borrow_check/nll/invalidation.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -93,9 +93,9 @@ impl<'cg, 'cx, 'tcx, 'gcx> Visitor<'tcx> for InvalidationGenerator<'cg, 'cx, 'tc
9393
JustWrite
9494
);
9595
}
96-
StatementKind::ReadForMatch(ref place) => {
96+
StatementKind::FakeRead(_, ref place) => {
9797
self.access_place(
98-
ContextKind::ReadForMatch.new(location),
98+
ContextKind::FakeRead.new(location),
9999
place,
100100
(Deep, Read(ReadKind::Borrow(BorrowKind::Shared))),
101101
LocalMutationIsAllowed::No,

Diff for: ‎src/librustc_mir/borrow_check/nll/type_check/mod.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -995,7 +995,7 @@ impl<'a, 'gcx, 'tcx> TypeChecker<'a, 'gcx, 'tcx> {
995995
);
996996
}
997997
}
998-
StatementKind::ReadForMatch(_)
998+
StatementKind::FakeRead(..)
999999
| StatementKind::StorageLive(_)
10001000
| StatementKind::StorageDead(_)
10011001
| StatementKind::InlineAsm { .. }

Diff for: ‎src/librustc_mir/build/matches/mod.rs

+26-8
Original file line numberDiff line numberDiff line change
@@ -145,19 +145,16 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
145145
if let (true, Some(borrow_temp)) =
146146
(tcx.emit_read_for_match(), borrowed_input_temp.clone())
147147
{
148-
// inject a fake read of the borrowed input at
149-
// the start of each arm's pattern testing
150-
// code.
151-
//
152-
// This should ensure that you cannot change
153-
// the variant for an enum while you are in
154-
// the midst of matching on it.
148+
// Inject a fake read, see comments on `FakeReadCause::ForMatch`.
155149
let pattern_source_info = self.source_info(pattern.span);
156150
self.cfg.push(
157151
*pre_binding_block,
158152
Statement {
159153
source_info: pattern_source_info,
160-
kind: StatementKind::ReadForMatch(borrow_temp.clone()),
154+
kind: StatementKind::FakeRead(
155+
FakeReadCause::ForMatch,
156+
borrow_temp.clone(),
157+
),
161158
},
162159
);
163160
}
@@ -264,6 +261,18 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
264261
let place =
265262
self.storage_live_binding(block, var, irrefutable_pat.span, OutsideGuard);
266263
unpack!(block = self.into(&place, block, initializer));
264+
265+
266+
// Inject a fake read, see comments on `FakeReadCause::ForLet`.
267+
let source_info = self.source_info(irrefutable_pat.span);
268+
self.cfg.push(
269+
block,
270+
Statement {
271+
source_info,
272+
kind: StatementKind::FakeRead(FakeReadCause::ForLet, place.clone()),
273+
},
274+
);
275+
267276
self.schedule_drop_for_binding(var, irrefutable_pat.span, OutsideGuard);
268277
block.unit()
269278
}
@@ -305,6 +314,15 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
305314
},
306315
);
307316

317+
// Inject a fake read, see comments on `FakeReadCause::ForLet`.
318+
self.cfg.push(
319+
block,
320+
Statement {
321+
source_info,
322+
kind: StatementKind::FakeRead(FakeReadCause::ForLet, place.clone()),
323+
},
324+
);
325+
308326
self.schedule_drop_for_binding(var, irrefutable_pat.span, OutsideGuard);
309327
block.unit()
310328
}

Diff for: ‎src/librustc_mir/dataflow/impls/borrows.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -336,7 +336,7 @@ impl<'a, 'gcx, 'tcx> BitDenotation for Borrows<'a, 'gcx, 'tcx> {
336336
}
337337
}
338338

339-
mir::StatementKind::ReadForMatch(..) |
339+
mir::StatementKind::FakeRead(..) |
340340
mir::StatementKind::SetDiscriminant { .. } |
341341
mir::StatementKind::StorageLive(..) |
342342
mir::StatementKind::Validate(..) |

Diff for: ‎src/librustc_mir/dataflow/move_paths/builder.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -281,7 +281,7 @@ impl<'b, 'a, 'gcx, 'tcx> Gatherer<'b, 'a, 'gcx, 'tcx> {
281281
}
282282
self.gather_rvalue(rval);
283283
}
284-
StatementKind::ReadForMatch(ref place) => {
284+
StatementKind::FakeRead(_, ref place) => {
285285
self.create_move_path(place);
286286
}
287287
StatementKind::InlineAsm { ref outputs, ref inputs, ref asm } => {

Diff for: ‎src/librustc_mir/interpret/step.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -150,9 +150,9 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> {
150150
self.deallocate_local(old_val)?;
151151
}
152152

153-
// No dynamic semantics attached to `ReadForMatch`; MIR
153+
// No dynamic semantics attached to `FakeRead`; MIR
154154
// interpreter is solely intended for borrowck'ed code.
155-
ReadForMatch(..) => {}
155+
FakeRead(..) => {}
156156

157157
// Validity checks.
158158
Validate(op, ref places) => {

Diff for: ‎src/librustc_mir/transform/check_unsafety.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,7 @@ impl<'a, 'tcx> Visitor<'tcx> for UnsafetyChecker<'a, 'tcx> {
108108
self.source_info = statement.source_info;
109109
match statement.kind {
110110
StatementKind::Assign(..) |
111-
StatementKind::ReadForMatch(..) |
111+
StatementKind::FakeRead(..) |
112112
StatementKind::SetDiscriminant { .. } |
113113
StatementKind::StorageLive(..) |
114114
StatementKind::StorageDead(..) |

Diff for: ‎src/librustc_mir/transform/qualify_consts.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -1090,7 +1090,7 @@ impl<'a, 'tcx> Visitor<'tcx> for Qualifier<'a, 'tcx, 'tcx> {
10901090
StatementKind::Assign(ref place, ref rvalue) => {
10911091
this.visit_assign(bb, place, rvalue, location);
10921092
}
1093-
StatementKind::ReadForMatch(..) |
1093+
StatementKind::FakeRead(..) |
10941094
StatementKind::SetDiscriminant { .. } |
10951095
StatementKind::StorageLive(_) |
10961096
StatementKind::StorageDead(_) |

Diff for: ‎src/librustc_mir/transform/qualify_min_const_fn.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -216,7 +216,7 @@ fn check_statement(
216216
check_rvalue(tcx, mir, rval, span)
217217
}
218218

219-
StatementKind::ReadForMatch(_) => Err((span, "match in const fn is unstable".into())),
219+
StatementKind::FakeRead(..) => Err((span, "match in const fn is unstable".into())),
220220

221221
// just an assignment
222222
StatementKind::SetDiscriminant { .. } => Ok(()),

Diff for: ‎src/librustc_mir/transform/remove_noop_landing_pads.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ impl RemoveNoopLandingPads {
4949
) -> bool {
5050
for stmt in &mir[bb].statements {
5151
match stmt.kind {
52-
StatementKind::ReadForMatch(_) |
52+
StatementKind::FakeRead(..) |
5353
StatementKind::StorageLive(_) |
5454
StatementKind::StorageDead(_) |
5555
StatementKind::EndRegion(_) |

Diff for: ‎src/librustc_mir/transform/rustc_peek.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -157,7 +157,7 @@ fn each_block<'a, 'tcx, O>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
157157
mir::StatementKind::Assign(ref place, ref rvalue) => {
158158
(place, rvalue)
159159
}
160-
mir::StatementKind::ReadForMatch(_) |
160+
mir::StatementKind::FakeRead(..) |
161161
mir::StatementKind::StorageLive(_) |
162162
mir::StatementKind::StorageDead(_) |
163163
mir::StatementKind::InlineAsm { .. } |

Diff for: ‎src/librustc_passes/mir_stats.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ impl<'a, 'tcx> mir_visit::Visitor<'tcx> for StatCollector<'a, 'tcx> {
8282
self.record("Statement", statement);
8383
self.record(match statement.kind {
8484
StatementKind::Assign(..) => "StatementKind::Assign",
85-
StatementKind::ReadForMatch(..) => "StatementKind::ReadForMatch",
85+
StatementKind::FakeRead(..) => "StatementKind::FakeRead",
8686
StatementKind::EndRegion(..) => "StatementKind::EndRegion",
8787
StatementKind::Validate(..) => "StatementKind::Validate",
8888
StatementKind::SetDiscriminant { .. } => "StatementKind::SetDiscriminant",

Diff for: ‎src/test/mir-opt/basic_assignment.rs

+2
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ fn main() {
4747
// bb0: {
4848
// StorageLive(_1);
4949
// _1 = const false;
50+
// FakeRead(ForLet, _1);
5051
// StorageLive(_2);
5152
// StorageLive(_3);
5253
// _3 = _1;
@@ -55,6 +56,7 @@ fn main() {
5556
// StorageLive(_4);
5657
// _4 = std::option::Option<std::boxed::Box<u32>>::None;
5758
// AscribeUserType(_4, o, Canonical { variables: [], value: std::option::Option<std::boxed::Box<u32>> });
59+
// FakeRead(ForLet, _4);
5860
// StorageLive(_5);
5961
// StorageLive(_6);
6062
// _6 = move _4;

Diff for: ‎src/test/mir-opt/box_expr.rs

+1
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,7 @@ impl Drop for S {
6363
//
6464
// bb4: {
6565
// StorageDead(_2);
66+
// FakeRead(ForLet, _1);
6667
// StorageLive(_4);
6768
// _4 = move _1;
6869
// _3 = const std::mem::drop(move _4) -> [return: bb5, unwind: bb7];

0 commit comments

Comments
 (0)