Skip to content

Polonius: fix some cases of killed fact generation, and most of the ui test suite #62736

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.

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 29 commits into from
Jul 25, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
c442dae
Ignore NLL migrate mode test in the Polonius compare-mode
lqd Jun 11, 2019
d4ca9a3
Ignore test issue-45983 in the polonius compare mode
lqd Jun 11, 2019
273bfd4
Ignore two-phase-reservation-sharing-interference-2.rs in Polonius co…
lqd Jun 11, 2019
63c837e
Ignore feature-gate-nll.rs in Polonius compare mode
lqd Jun 11, 2019
51c15fa
Ignore test issue-45696-scribble-on-boxed-borrow.rs in Polonius compa…
lqd Jun 11, 2019
ff350f8
Bless output of test ui/emit-artifact-notifications.rs for Polonius
lqd Jun 11, 2019
2824db1
Bless output of test save-analysis/emit-notifications.rs for Polonius
lqd Jun 11, 2019
9410104
Bless output of test nll/loan_ends_mid_block_pair.rs for Polonius
lqd Jun 11, 2019
6a7c15e
Bless output of test consts/promote_const_let.rs for Polonius
lqd Jun 12, 2019
c5a1bc1
Bless output of test nll/get_default.rs for Polonius
lqd Jun 12, 2019
292d5c1
Bless output of test generator/ref-escapes-but-not-over-yield.rs for …
lqd Jun 12, 2019
6fe5292
Bless output of test borrowck/borrowck-escaping-closure-error-2.rs f…
lqd Jun 28, 2019
7db61e7
Bless output of test borrowck/two-phase-surprise-no-conflict.rs for P…
lqd Jun 28, 2019
08c25b5
Bless output of test dropck/dropck_trait_cycle_checked.rs for Polonius
lqd Jun 28, 2019
9a82f52
Create a dedicated polonius test folder
lqd Jul 5, 2019
6d9a4f9
Polonius facts: kill loans on Call terminators and StorageDead
lqd Jul 15, 2019
0bd2b32
Bless output of test borrowck/promote-ref-mut-in-let-issue-46557.rs …
lqd Jul 15, 2019
ed1625f
Ignore test hrtb/issue-30786.rs in Polonius compare mode
lqd Jul 15, 2019
40e6b02
Bless output of test nll/return-ref-mut-issue-46557.rs for Polonius
lqd Jul 15, 2019
9bd9b0d
Add test extracted from rand, checking that StorageDead kills loans
lqd Jul 15, 2019
606f798
Rename test so that both "kills-loans" tests match names
lqd Jul 15, 2019
9e0fb6f
Make both polonius loans tests check-pass
lqd Jul 15, 2019
823ab42
Bless output of test unboxed-closures/unboxed-closures-failed-recursi…
lqd Jul 15, 2019
2f3e36f
Polonius: generate `killed` facts for assignments to projections
lqd Jul 16, 2019
d41e002
Add test checking various assignments are accepted in Polonius
lqd Jul 16, 2019
770129c
Add test to check that assignments to projections do not kill too man…
lqd Jul 16, 2019
c7f9a71
issue-46589 passes in Polonius and fails in NLL, duplicate it and man…
lqd Jul 16, 2019
c0eab36
Bless output of test nll/loan_ends_mid_block_pair.rs for Polonius, again
lqd Jul 16, 2019
e16bede
fix tidy
lqd Jul 16, 2019
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
137 changes: 120 additions & 17 deletions src/librustc_mir/borrow_check/nll/constraint_generation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,15 @@ use crate::borrow_check::location::LocationTable;
use crate::borrow_check::nll::ToRegionVid;
use crate::borrow_check::nll::facts::AllFacts;
use crate::borrow_check::nll::region_infer::values::LivenessValues;
use crate::borrow_check::places_conflict;
use rustc::infer::InferCtxt;
use rustc::mir::visit::TyContext;
use rustc::mir::visit::Visitor;
use rustc::mir::{BasicBlock, BasicBlockData, Location, Body, Place, PlaceBase, Rvalue};
use rustc::mir::{SourceInfo, Statement, Terminator};
use rustc::mir::UserTypeProjection;
use rustc::mir::{
BasicBlock, BasicBlockData, Body, Local, Location, Place, PlaceBase, Projection,
ProjectionElem, Rvalue, SourceInfo, Statement, StatementKind, Terminator, TerminatorKind,
UserTypeProjection,
};
use rustc::ty::fold::TypeFoldable;
use rustc::ty::{self, ClosureSubsts, GeneratorSubsts, RegionVid, Ty};
use rustc::ty::subst::SubstsRef;
Expand All @@ -27,6 +30,7 @@ pub(super) fn generate_constraints<'cx, 'tcx>(
liveness_constraints,
location_table,
all_facts,
body,
};

for (bb, data) in body.basic_blocks().iter_enumerated() {
Expand All @@ -41,6 +45,7 @@ struct ConstraintGeneration<'cg, 'cx, 'tcx> {
location_table: &'cg LocationTable,
liveness_constraints: &'cg mut LivenessValues<RegionVid>,
borrow_set: &'cg BorrowSet<'tcx>,
body: &'cg Body<'tcx>,
}

impl<'cg, 'cx, 'tcx> Visitor<'tcx> for ConstraintGeneration<'cg, 'cx, 'tcx> {
Expand Down Expand Up @@ -114,6 +119,17 @@ impl<'cg, 'cx, 'tcx> Visitor<'tcx> for ConstraintGeneration<'cg, 'cx, 'tcx> {
self.location_table
.start_index(location.successor_within_block()),
));

// If there are borrows on this now dead local, we need to record them as `killed`.
if let StatementKind::StorageDead(ref local) = statement.kind {
record_killed_borrows_for_local(
all_facts,
self.borrow_set,
self.location_table,
local,
location,
);
}
}

self.super_statement(statement, location);
Expand All @@ -127,20 +143,7 @@ impl<'cg, 'cx, 'tcx> Visitor<'tcx> for ConstraintGeneration<'cg, 'cx, 'tcx> {
) {
// When we see `X = ...`, then kill borrows of
// `(*X).foo` and so forth.
if let Some(all_facts) = self.all_facts {
if let Place {
base: PlaceBase::Local(temp),
projection: None,
} = place {
if let Some(borrow_indices) = self.borrow_set.local_map.get(temp) {
all_facts.killed.reserve(borrow_indices.len());
for &borrow_index in borrow_indices {
let location_index = self.location_table.mid_index(location);
all_facts.killed.push((borrow_index, location_index));
}
}
}
}
self.record_killed_borrows_for_place(place, location);

self.super_assign(place, rvalue, location);
}
Expand All @@ -167,6 +170,14 @@ impl<'cg, 'cx, 'tcx> Visitor<'tcx> for ConstraintGeneration<'cg, 'cx, 'tcx> {
}
}

// A `Call` terminator's return value can be a local which has borrows,
// so we need to record those as `killed` as well.
if let TerminatorKind::Call { ref destination, .. } = terminator.kind {
if let Some((place, _)) = destination {
self.record_killed_borrows_for_place(place, location);
}
}

self.super_terminator(terminator, location);
}

Expand Down Expand Up @@ -201,4 +212,96 @@ impl<'cx, 'cg, 'tcx> ConstraintGeneration<'cx, 'cg, 'tcx> {
self.liveness_constraints.add_element(vid, location);
});
}

/// When recording facts for Polonius, records the borrows on the specified place
/// as `killed`. For example, when assigning to a local, or on a call's return destination.
fn record_killed_borrows_for_place(&mut self, place: &Place<'tcx>, location: Location) {
if let Some(all_facts) = self.all_facts {
// Depending on the `Place` we're killing:
// - if it's a local, or a single deref of a local,
// we kill all the borrows on the local.
// - if it's a deeper projection, we have to filter which
// of the borrows are killed: the ones whose `borrowed_place`
// conflicts with the `place`.
match place {
Place {
base: PlaceBase::Local(local),
projection: None,
} |
Place {
base: PlaceBase::Local(local),
projection: Some(box Projection {
base: None,
elem: ProjectionElem::Deref,
}),
} => {
debug!(
"Recording `killed` facts for borrows of local={:?} at location={:?}",
local, location
);

record_killed_borrows_for_local(
all_facts,
self.borrow_set,
self.location_table,
local,
location,
);
}

Place {
base: PlaceBase::Static(_),
..
} => {
// Ignore kills of static or static mut variables.
}

Place {
base: PlaceBase::Local(local),
projection: Some(_),
} => {
// Kill conflicting borrows of the innermost local.
debug!(
"Recording `killed` facts for borrows of \
innermost projected local={:?} at location={:?}",
local, location
);

if let Some(borrow_indices) = self.borrow_set.local_map.get(local) {
for &borrow_index in borrow_indices {
let places_conflict = places_conflict::places_conflict(
self.infcx.tcx,
self.body,
&self.borrow_set.borrows[borrow_index].borrowed_place,
place,
places_conflict::PlaceConflictBias::NoOverlap,
);

if places_conflict {
let location_index = self.location_table.mid_index(location);
all_facts.killed.push((borrow_index, location_index));
}
}
}
}
}
}
}
}

/// When recording facts for Polonius, records the borrows on the specified local as `killed`.
fn record_killed_borrows_for_local(
all_facts: &mut AllFacts,
borrow_set: &BorrowSet<'_>,
location_table: &LocationTable,
local: &Local,
location: Location,
) {
if let Some(borrow_indices) = borrow_set.local_map.get(local) {
all_facts.killed.reserve(borrow_indices.len());
for &borrow_index in borrow_indices {
let location_index = location_table.mid_index(location);
all_facts.killed.push((borrow_index, location_index));
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
error[E0597]: `books` does not live long enough
--> $DIR/borrowck-escaping-closure-error-2.rs:11:17
|
LL | Box::new(|| books.push(4))
| ------------^^^^^---------
| | | |
| | | borrowed value does not live long enough
| | value captured here
| borrow later used here
LL |
LL | }
| - `books` dropped here while still borrowed

error: aborting due to previous error

For more information about this error, try `rustc --explain E0597`.
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
warning[E0507]: cannot move out of `foo` in pattern guard
--> $DIR/borrowck-migrate-to-nll.rs:25:18
--> $DIR/borrowck-migrate-to-nll.rs:26:18
|
LL | (|| { let bar = foo; bar.take() })();
| ^^ ---
Expand Down
1 change: 1 addition & 0 deletions src/test/ui/borrowck/borrowck-migrate-to-nll.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
// just ignore it instead:

// ignore-compare-mode-nll
// ignore-compare-mode-polonius

// revisions: zflag edition
//[zflag]compile-flags: -Z borrowck=migrate
Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/borrowck/borrowck-migrate-to-nll.zflag.stderr
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
warning[E0507]: cannot move out of `foo` in pattern guard
--> $DIR/borrowck-migrate-to-nll.rs:25:18
--> $DIR/borrowck-migrate-to-nll.rs:26:18
|
LL | (|| { let bar = foo; bar.take() })();
| ^^ ---
Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/borrowck/issue-45983.migrate.stderr
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
error: borrowed data cannot be stored outside of its closure
--> $DIR/issue-45983.rs:19:27
--> $DIR/issue-45983.rs:20:27
|
LL | let x = None;
| - borrowed data cannot be stored into here...
Expand Down
4 changes: 2 additions & 2 deletions src/test/ui/borrowck/issue-45983.nll.stderr
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
error[E0521]: borrowed data escapes outside of closure
--> $DIR/issue-45983.rs:19:18
--> $DIR/issue-45983.rs:20:18
|
LL | let x = None;
| - `x` is declared here, outside of the closure body
Expand All @@ -9,7 +9,7 @@ LL | give_any(|y| x = Some(y));
| `y` is a reference that is only valid in the closure body

error[E0594]: cannot assign to `x`, as it is not declared as mutable
--> $DIR/issue-45983.rs:19:18
--> $DIR/issue-45983.rs:20:18
|
LL | let x = None;
| - help: consider changing this to be mutable: `mut x`
Expand Down
1 change: 1 addition & 0 deletions src/test/ui/borrowck/issue-45983.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
// revisions, don't worry about the --compare-mode=nll on this test.

// ignore-compare-mode-nll
// ignore-compare-mode-polonius

//[nll]compile-flags: -Z borrowck=mir

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
error[E0716]: temporary value dropped while borrowed
--> $DIR/promote-ref-mut-in-let-issue-46557.rs:5:21
|
LL | let ref mut x = 1234543;
| ^^^^^^^ creates a temporary which is freed while still in use
LL | x
| - borrow later used here
LL | }
| - temporary value is freed at the end of this statement
|
= note: consider using a `let` binding to create a longer lived value

error[E0716]: temporary value dropped while borrowed
--> $DIR/promote-ref-mut-in-let-issue-46557.rs:10:25
|
LL | let (ref mut x, ) = (1234543, );
| ^^^^^^^^^^^ creates a temporary which is freed while still in use
LL | x
| - borrow later used here
LL | }
| - temporary value is freed at the end of this statement
|
= note: consider using a `let` binding to create a longer lived value

error[E0515]: cannot return value referencing temporary value
--> $DIR/promote-ref-mut-in-let-issue-46557.rs:15:5
|
LL | match 1234543 {
| ^ ------- temporary value created here
| _____|
| |
LL | | ref mut x => x
LL | | }
| |_____^ returns a value referencing data owned by the current function

error[E0515]: cannot return value referencing temporary value
--> $DIR/promote-ref-mut-in-let-issue-46557.rs:21:5
|
LL | match (123443,) {
| ^ --------- temporary value created here
| _____|
| |
LL | | (ref mut x,) => x,
LL | | }
| |_____^ returns a value referencing data owned by the current function

error[E0515]: cannot return reference to temporary value
--> $DIR/promote-ref-mut-in-let-issue-46557.rs:27:5
|
LL | &mut 1234543
| ^^^^^-------
| | |
| | temporary value created here
| returns a reference to data owned by the current function

error: aborting due to 5 previous errors

Some errors have detailed explanations: E0515, E0716.
For more information about an error, try `rustc --explain E0515`.
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
error[E0502]: cannot borrow `v` as mutable because it is also borrowed as immutable
--> $DIR/two-phase-reservation-sharing-interference-2.rs:18:5
--> $DIR/two-phase-reservation-sharing-interference-2.rs:19:5
|
LL | let shared = &v;
| -- immutable borrow occurs here
Expand All @@ -11,7 +11,7 @@ LL | v.extend(shared);
| mutable borrow occurs here

error[E0502]: cannot borrow `v` as mutable because it is also borrowed as immutable
--> $DIR/two-phase-reservation-sharing-interference-2.rs:28:5
--> $DIR/two-phase-reservation-sharing-interference-2.rs:29:5
|
LL | v.extend(&v);
| ^^------^--^
Expand All @@ -21,7 +21,7 @@ LL | v.extend(&v);
| mutable borrow occurs here

warning: cannot borrow `v` as mutable because it is also borrowed as immutable
--> $DIR/two-phase-reservation-sharing-interference-2.rs:39:5
--> $DIR/two-phase-reservation-sharing-interference-2.rs:40:5
|
LL | let shared = &v;
| -- immutable borrow occurs here
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
error[E0502]: cannot borrow `v` as mutable because it is also borrowed as immutable
--> $DIR/two-phase-reservation-sharing-interference-2.rs:18:5
--> $DIR/two-phase-reservation-sharing-interference-2.rs:19:5
|
LL | let shared = &v;
| -- immutable borrow occurs here
Expand All @@ -11,7 +11,7 @@ LL | v.extend(shared);
| mutable borrow occurs here

error[E0502]: cannot borrow `v` as mutable because it is also borrowed as immutable
--> $DIR/two-phase-reservation-sharing-interference-2.rs:28:5
--> $DIR/two-phase-reservation-sharing-interference-2.rs:29:5
|
LL | v.extend(&v);
| ^^------^--^
Expand All @@ -21,7 +21,7 @@ LL | v.extend(&v);
| mutable borrow occurs here

warning: cannot borrow `v` as mutable because it is also borrowed as immutable
--> $DIR/two-phase-reservation-sharing-interference-2.rs:39:5
--> $DIR/two-phase-reservation-sharing-interference-2.rs:40:5
|
LL | let shared = &v;
| -- immutable borrow occurs here
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
error[E0502]: cannot borrow `v` as mutable because it is also borrowed as immutable
--> $DIR/two-phase-reservation-sharing-interference-2.rs:18:5
--> $DIR/two-phase-reservation-sharing-interference-2.rs:19:5
|
LL | let shared = &v;
| -- immutable borrow occurs here
Expand All @@ -10,7 +10,7 @@ LL | v.extend(shared);
| mutable borrow occurs here

error[E0502]: cannot borrow `v` as mutable because it is also borrowed as immutable
--> $DIR/two-phase-reservation-sharing-interference-2.rs:28:5
--> $DIR/two-phase-reservation-sharing-interference-2.rs:29:5
|
LL | v.extend(&v);
| ^^------^--^
Expand All @@ -20,7 +20,7 @@ LL | v.extend(&v);
| mutable borrow occurs here

error[E0502]: cannot borrow `v` as mutable because it is also borrowed as immutable
--> $DIR/two-phase-reservation-sharing-interference-2.rs:39:5
--> $DIR/two-phase-reservation-sharing-interference-2.rs:40:5
|
LL | let shared = &v;
| -- immutable borrow occurs here
Expand Down
Loading