mikros/rust
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Auto merge of #53109 - nikomatsakis:nll-escaping-into-return-revert, r=nikomatsakis

Browse Source 
revert #52991

Reverts https://github.com/rust-lang/rust/pull/52991 which is flawed. I have an idea how to fix it but might as well revert first since it is so wildly flawed. That's what I get for opening PRs while on PTO =)

r? @pnkfelix
This commit is contained in:
bors 2018-08-07 16:01:27 +00:00
commit ccb550fb45
11 changed files with 108 additions and 331 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

229
src/librustc_mir/borrow_check/nll/escaping_locals.rs
View File

@ -1,229 +0,0 @@
// Copyright 2016 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.
//! Identify those variables whose entire value will eventually be
//! returned from the fn via the RETURN_PLACE. As an optimization, we
//! can skip computing liveness results for those variables. The idea
//! is that the return type of the fn only ever contains free
//! regions. Therefore, the types of those variables are going to
//! ultimately be contrained to outlive those free regions -- since
//! free regions are always live for the entire body, this implies
//! that the liveness results are not important for those regions.
//! This is most important in the "fns" that we create to represent static
//! values, since those are often really quite large, and all regions in them
//! will ultimately be constrained to be `'static`. Two examples:
//!
//! ```
//! fn foo() -> &'static [u32] { &[] }
//! static FOO: &[u32] = &[];
//! ```
//!
//! In both these cases, the return value will only have static lifetime.
//!
//! NB: The simple logic here relies on the fact that outlives
//! relations in our analysis don't have locations. Otherwise, we
//! would have to restrict ourselves to values that are
//! *unconditionally* returned (which would still cover the "big
//! static value" case).
//!
//! The way that this code works is to use union-find -- we iterate
//! over the MIR and union together two variables X and Y if all
//! regions in the value of Y are going to be stored into X -- that
//! is, if `typeof(X): 'a` requires that `typeof(Y): 'a`. This means
//! that e.g. we can union together `x` and `y` if we have something
//! like `x = (y, 22)`, but not something like `x = y.f` (since there
//! may be regions in the type of `y` that do not appear in the field
//! `f`).
use rustc::mir::visit::Visitor;
use rustc::mir::*;
use rustc_data_structures::indexed_vec::Idx;
use rustc_data_structures::unify as ut;
crate struct EscapingLocals {
unification_table: ut::UnificationTable<ut::InPlace<AssignedLocal>>,
}
impl EscapingLocals {
crate fn compute(mir: &Mir<'tcx>) -> Self {
let mut visitor = GatherAssignedLocalsVisitor::new();
visitor.visit_mir(mir);
EscapingLocals {
unification_table: visitor.unification_table,
}
}
/// True if `local` is known to escape into static
/// memory.
crate fn escapes_into_return(&mut self, local: Local) -> bool {
let return_place = AssignedLocal::from(RETURN_PLACE);
let other_place = AssignedLocal::from(local);
self.unification_table.unioned(return_place, other_place)
}
}
/// The MIR visitor gathering the union-find of the locals used in
/// assignments.
struct GatherAssignedLocalsVisitor {
unification_table: ut::UnificationTable<ut::InPlace<AssignedLocal>>,
}
#[derive(Copy, Clone, Debug, Hash, PartialEq, Eq)]
struct AssignedLocal(u32);
impl ut::UnifyKey for AssignedLocal {
type Value = ();
fn index(&self) -> u32 {
self.0
}
fn from_index(i: u32) -> AssignedLocal {
AssignedLocal(i)
}
fn tag() -> &'static str {
"AssignedLocal"
}
}
impl From<Local> for AssignedLocal {
fn from(item: Local) -> Self {
// newtype_indexes use usize but are u32s.
assert!(item.index() < ::std::u32::MAX as usize);
AssignedLocal(item.index() as u32)
}
}
impl GatherAssignedLocalsVisitor {
fn new() -> Self {
Self {
unification_table: ut::UnificationTable::new(),
}
}
fn union_locals_if_needed(&mut self, lvalue: Option<Local>, rvalue: Option<Local>) {
if let Some(lvalue) = lvalue {
if let Some(rvalue) = rvalue {
if lvalue != rvalue {
debug!("EscapingLocals: union {:?} and {:?}", lvalue, rvalue);
self.unification_table
.union(AssignedLocal::from(lvalue), AssignedLocal::from(rvalue));
}
}
}
}
}
// Returns the potential `Local` associated to this `Place` or `PlaceProjection`
fn find_local_in_place(place: &Place) -> Option<Local> {
match place {
Place::Local(local) => Some(*local),
// If you do e.g. `x = a.f` then only *part* of the type of
// `a` escapes into `x` (the part contained in `f`); if `a`'s
// type has regions that don't appear in `f`, those might not
// escape.
Place::Projection(..) => None,
Place::Static { .. } | Place::Promoted { .. } => None,
}
}
// Returns the potential `Local` in this `Operand`.
fn find_local_in_operand(op: &Operand) -> Option<Local> {
// Conservatively check a subset of `Operand`s we know our
// benchmarks track, for example `html5ever`.
match op {
Operand::Copy(place) | Operand::Move(place) => find_local_in_place(place),
Operand::Constant(_) => None,
}
}
impl Visitor<'tcx> for GatherAssignedLocalsVisitor {
fn visit_mir(&mut self, mir: &Mir<'tcx>) {
// We need as many union-find keys as there are locals
for _ in 0..mir.local_decls.len() {
self.unification_table.new_key(());
}
self.super_mir(mir);
}
fn visit_assign(
&mut self,
block: BasicBlock,
place: &Place<'tcx>,
rvalue: &Rvalue<'tcx>,
location: Location,
) {
let local = find_local_in_place(place);
// Find those cases where there is a `Place` consumed by
// `rvalue` and we know that all regions in its type will be
// incorporated into `place`, the `Place` we are assigning to.
match rvalue {
// `x = y` is the simplest possible case.
Rvalue::Use(op) => self.union_locals_if_needed(local, find_local_in_operand(op)),
// `X = &'r P` -- the type of `X` will be `&'r T_P`, where
// `T_P` is the type of `P`.
Rvalue::Ref(_, _, place) => {
// Special case: if you have `X = &*Y` (or `X = &**Y`
// etc), then the outlives relationships will ensure
// that all regions in `Y` are constrained by regions
// in `X` -- this is because the lifetimes of the
// references we deref through are required to outlive
// the borrow lifetime `'r` (which appears in `X`).
//
// (We don't actually need to check the type of `Y`:
// since `ProjectionElem::Deref` represents a built-in
// deref and not an overloaded deref, if the thing we
// deref through is not a reference, then it must be a
// `Box` or `*const`, in which case it contains no
// references.)
let mut place_ref = place;
while let Place::Projection(proj) = place_ref {
if let ProjectionElem::Deref = proj.elem {
place_ref = &proj.base;
} else {
break;
}
}
self.union_locals_if_needed(local, find_local_in_place(place_ref))
}
Rvalue::Cast(kind, op, _) => match kind {
CastKind::Unsize => {
// Casting a `&[T; N]` to `&[T]` or `&Foo` to `&Trait` --
// in both cases, no regions are "lost".
self.union_locals_if_needed(local, find_local_in_operand(op))
}
_ => (),
},
// Constructing an aggregate like `(x,)` or `Foo { x }`
// includes the full type of `x`.
Rvalue::Aggregate(_, ops) => {
for rvalue in ops.iter().map(find_local_in_operand) {
self.union_locals_if_needed(local, rvalue);
}
}
// For other things, be conservative and do not union.
_ => (),
};
self.super_assign(block, place, rvalue, location);
}
}

48
src/librustc_mir/borrow_check/nll/liveness_map.rs
View File

@ -16,10 +16,9 @@
//! liveness code so that it only operates over variables with regions in their
//! types, instead of all variables.
use borrow_check::nll::escaping_locals::EscapingLocals;
use rustc::mir::{Local, Mir};
use rustc::ty::TypeFoldable;
use rustc_data_structures::indexed_vec::IndexVec;
use rustc::mir::{Mir, Local};
use util::liveness::LiveVariableMap;
use rustc_data_structures::indexed_vec::Idx;
@ -30,13 +29,14 @@
crate struct NllLivenessMap {
/// For each local variable, contains either None (if the type has no regions)
/// or Some(i) with a suitable index.
from_local: IndexVec<Local, Option<LocalWithRegion>>,
pub from_local: IndexVec<Local, Option<LocalWithRegion>>,
/// For each LocalWithRegion, maps back to the original Local index.
to_local: IndexVec<LocalWithRegion, Local>,
pub to_local: IndexVec<LocalWithRegion, Local>,
}
impl LiveVariableMap for NllLivenessMap {
fn from_local(&self, local: Local) -> Option<Self::LiveVar> {
self.from_local[local]
}
@ -55,43 +55,21 @@ fn num_variables(&self) -> usize {
impl NllLivenessMap {
/// Iterates over the variables in Mir and assigns each Local whose type contains
/// regions a LocalWithRegion index. Returns a map for converting back and forth.
crate fn compute(mir: &Mir<'tcx>) -> Self {
let mut escaping_locals = EscapingLocals::compute(mir);
pub fn compute(mir: &Mir) -> Self {
let mut to_local = IndexVec::default();
let mut escapes_into_return = 0;
let mut no_regions = 0;
let from_local: IndexVec<Local, Option<_>> = mir
let from_local: IndexVec<Local,Option<_>> = mir
.local_decls
.iter_enumerated()
.map(|(local, local_decl)| {
if escaping_locals.escapes_into_return(local) {
// If the local escapes into the return value,
// then the return value will force all of the
// regions in its type to outlive free regions
// (e.g., `'static`) and hence liveness is not
// needed. This is particularly important for big
// statics.
escapes_into_return += 1;
None
} else if local_decl.ty.has_free_regions() {
let l = to_local.push(local);
debug!("liveness_map: {:?} = {:?}", local, l);
Some(l)
} else {
no_regions += 1;
None
if local_decl.ty.has_free_regions() {
Some(to_local.push(local))
}
else {
None
}
}).collect();
debug!("liveness_map: {} variables need liveness", to_local.len());
debug!("liveness_map: {} escapes into return", escapes_into_return);
debug!("liveness_map: {} no regions", no_regions);
Self {
from_local,
to_local,
}
Self { from_local, to_local }
}
}

13
src/librustc_mir/borrow_check/nll/mod.rs
View File

@ -39,9 +39,7 @@
use util as mir_util;
use util::pretty::{self, ALIGN};
mod constraints;
mod constraint_generation;
mod escaping_locals;
pub mod explain_borrow;
mod facts;
mod invalidation;
@ -51,6 +49,8 @@
mod universal_regions;
crate mod liveness_map;
mod constraints;
use self::facts::AllFacts;
use self::region_infer::RegionInferenceContext;
use self::universal_regions::UniversalRegions;
@ -120,7 +120,6 @@ pub(in borrow_check) fn compute_regions<'cx, 'gcx, 'tcx>(
location_table,
borrow_set,
&liveness,
&liveness_map,
&mut all_facts,
flow_inits,
move_data,
@ -206,7 +205,6 @@ pub(in borrow_check) fn compute_regions<'cx, 'gcx, 'tcx>(
dump_mir_results(
infcx,
&liveness,
&liveness_map,
MirSource::item(def_id),
&mir,
&regioncx,
@ -223,7 +221,6 @@ pub(in borrow_check) fn compute_regions<'cx, 'gcx, 'tcx>(
fn dump_mir_results<'a, 'gcx, 'tcx>(
infcx: &InferCtxt<'a, 'gcx, 'tcx>,
liveness: &LivenessResults<LocalWithRegion>,
liveness_map: &NllLivenessMap,
source: MirSource,
mir: &Mir<'tcx>,
regioncx: &RegionInferenceContext,
@ -233,6 +230,8 @@ fn dump_mir_results<'a, 'gcx, 'tcx>(
return;
}
let map = &NllLivenessMap::compute(mir);
let regular_liveness_per_location: FxHashMap<_, _> = mir
.basic_blocks()
.indices()
@ -240,7 +239,7 @@ fn dump_mir_results<'a, 'gcx, 'tcx>(
let mut results = vec![];
liveness
.regular
.simulate_block(&mir, bb, liveness_map, |location, local_set| {
.simulate_block(&mir, bb, map, |location, local_set| {
results.push((location, local_set.clone()));
});
results
@ -254,7 +253,7 @@ fn dump_mir_results<'a, 'gcx, 'tcx>(
let mut results = vec![];
liveness
.drop
.simulate_block(&mir, bb, liveness_map, |location, local_set| {
.simulate_block(&mir, bb, map, |location, local_set| {
results.push((location, local_set.clone()));
});
results

15
src/librustc_mir/borrow_check/nll/type_check/liveness.rs
View File

@ -37,7 +37,6 @@ pub(super) fn generate<'gcx, 'tcx>(
cx: &mut TypeChecker<'_, 'gcx, 'tcx>,
mir: &Mir<'tcx>,
liveness: &LivenessResults<LocalWithRegion>,
liveness_map: &NllLivenessMap,
flow_inits: &mut FlowAtLocation<MaybeInitializedPlaces<'_, 'gcx, 'tcx>>,
move_data: &MoveData<'tcx>,
) {
@ -45,10 +44,10 @@ pub(super) fn generate<'gcx, 'tcx>(
cx,
mir,
liveness,
liveness_map,
flow_inits,
move_data,
drop_data: FxHashMap(),
map: &NllLivenessMap::compute(mir),
};
for bb in mir.basic_blocks().indices() {
@ -66,10 +65,10 @@ struct TypeLivenessGenerator<'gen, 'typeck, 'flow, 'gcx, 'tcx>
cx: &'gen mut TypeChecker<'typeck, 'gcx, 'tcx>,
mir: &'gen Mir<'tcx>,
liveness: &'gen LivenessResults<LocalWithRegion>,
liveness_map: &'gen NllLivenessMap,
flow_inits: &'gen mut FlowAtLocation<MaybeInitializedPlaces<'flow, 'gcx, 'tcx>>,
move_data: &'gen MoveData<'tcx>,
drop_data: FxHashMap<Ty<'tcx>, DropData<'tcx>>,
map: &'gen NllLivenessMap,
}
struct DropData<'tcx> {
@ -87,9 +86,9 @@ fn add_liveness_constraints(&mut self, bb: BasicBlock) {
self.liveness
.regular
.simulate_block(self.mir, bb, self.liveness_map, |location, live_locals| {
.simulate_block(self.mir, bb, self.map, |location, live_locals| {
for live_local in live_locals.iter() {
let local = self.liveness_map.from_live_var(live_local);
let local = self.map.from_live_var(live_local);
let live_local_ty = self.mir.local_decls[local].ty;
Self::push_type_live_constraint(&mut self.cx, live_local_ty, location);
}
@ -98,7 +97,7 @@ fn add_liveness_constraints(&mut self, bb: BasicBlock) {
let mut all_live_locals: Vec<(Location, Vec<LocalWithRegion>)> = vec![];
self.liveness
.drop
.simulate_block(self.mir, bb, self.liveness_map, |location, live_locals| {
.simulate_block(self.mir, bb, self.map, |location, live_locals| {
all_live_locals.push((location, live_locals.iter().collect()));
});
debug!(
@ -125,7 +124,7 @@ fn add_liveness_constraints(&mut self, bb: BasicBlock) {
});
}
let local = self.liveness_map.from_live_var(live_local);
let local = self.map.from_live_var(live_local);
let mpi = self.move_data.rev_lookup.find_local(local);
if let Some(initialized_child) = self.flow_inits.has_any_child_of(mpi) {
debug!(
@ -134,7 +133,7 @@ fn add_liveness_constraints(&mut self, bb: BasicBlock) {
self.move_data.move_paths[initialized_child]
);
let local = self.liveness_map.from_live_var(live_local);
let local = self.map.from_live_var(live_local);
let live_local_ty = self.mir.local_decls[local].ty;
self.add_drop_live_constraint(live_local, live_local_ty, location);
}

10
src/librustc_mir/borrow_check/nll/type_check/mod.rs
View File

@ -15,12 +15,9 @@
use borrow_check::location::LocationTable;
use borrow_check::nll::constraints::{ConstraintSet, OutlivesConstraint};
use borrow_check::nll::facts::AllFacts;
use borrow_check::nll::liveness_map::NllLivenessMap;
use borrow_check::nll::region_infer::values::{LivenessValues, RegionValueElements};
use borrow_check::nll::region_infer::values::{RegionValueElements, LivenessValues};
use borrow_check::nll::region_infer::{ClosureRegionRequirementsExt, TypeTest};
use borrow_check::nll::type_check::free_region_relations::{
CreateResult, UniversalRegionRelations,
};
use borrow_check::nll::type_check::free_region_relations::{CreateResult, UniversalRegionRelations};
use borrow_check::nll::universal_regions::UniversalRegions;
use borrow_check::nll::LocalWithRegion;
use borrow_check::nll::ToRegionVid;
@ -119,7 +116,6 @@ pub(crate) fn type_check<'gcx, 'tcx>(
location_table: &LocationTable,
borrow_set: &BorrowSet<'tcx>,
liveness: &LivenessResults<LocalWithRegion>,
liveness_map: &NllLivenessMap,
all_facts: &mut Option<AllFacts>,
flow_inits: &mut FlowAtLocation<MaybeInitializedPlaces<'_, 'gcx, 'tcx>>,
move_data: &MoveData<'tcx>,
@ -170,7 +166,7 @@ pub(crate) fn type_check<'gcx, 'tcx>(
Some(&mut borrowck_context),
Some(errors_buffer),
|cx| {
liveness::generate(cx, mir, liveness, liveness_map, flow_inits, move_data);
liveness::generate(cx, mir, liveness, flow_inits, move_data);
cx.equate_inputs_and_outputs(
mir,
mir_def_id,

29
src/test/ui/borrowck/issue-52713-bug.rs Normal file
View File

@ -0,0 +1,29 @@
// Copyright 2014 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.
// Regression test for a bug in #52713: this was an optimization for
// computing liveness that wound up accidentally causing the program
// below to be accepted.
#![feature(nll)]
fn foo<'a>(x: &'a mut u32) -> u32 {
let mut x = 22;
let y = &x;
if false {
return x;
}
x += 1; //~ ERROR
println!("{}", y);
return 0;
}
fn main() { }

14
src/test/ui/borrowck/issue-52713-bug.stderr Normal file
View File

@ -0,0 +1,14 @@
error[E0506]: cannot assign to `x` because it is borrowed
--> $DIR/issue-52713-bug.rs:24:5
|
LL | let y = &x;
| -- borrow of `x` occurs here
...
LL | x += 1; //~ ERROR
| ^^^^^^ assignment to borrowed `x` occurs here
LL | println!("{}", y);
| - borrow later used here
error: aborting due to previous error
For more information about this error, try `rustc --explain E0506`.

34
src/test/ui/borrowck/promote-ref-mut-in-let-issue-46557.nll.stderr
View File

@ -1,24 +1,30 @@
error[E0597]: borrowed value does not live long enough
--> $DIR/promote-ref-mut-in-let-issue-46557.rs:15:21
|
LL | let ref mut x = 1234543; //~ ERROR
| ^^^^^^^ temporary value does not live long enough
LL | x
LL | }
| - temporary value only lives until here
|
= note: borrowed value must be valid for the static lifetime...
LL | fn gimme_static_mut_let() -> &'static mut u32 {
| _______________________________________________-
LL | | let ref mut x = 1234543; //~ ERROR
| | ^^^^^^^ temporary value does not live long enough
LL | | x
LL | | }
| | -
| | |
| |_temporary value only lives until here
| borrow later used here
error[E0597]: borrowed value does not live long enough
--> $DIR/promote-ref-mut-in-let-issue-46557.rs:20:25
|
LL | let (ref mut x, ) = (1234543, ); //~ ERROR
| ^^^^^^^^^^^ temporary value does not live long enough
LL | x
LL | }
| - temporary value only lives until here
|
= note: borrowed value must be valid for the static lifetime...
LL | fn gimme_static_mut_let_nested() -> &'static mut u32 {
| ______________________________________________________-
LL | | let (ref mut x, ) = (1234543, ); //~ ERROR
| | ^^^^^^^^^^^ temporary value does not live long enough
LL | | x
LL | | }
| | -
| | |
| |_temporary value only lives until here
| borrow later used here
error[E0597]: borrowed value does not live long enough
--> $DIR/promote-ref-mut-in-let-issue-46557.rs:25:11

15
src/test/ui/nll/get_default.nll.stderr
View File

@ -63,18 +63,9 @@ LL | match map.get() {
LL | Some(v) => {
LL | map.set(String::new()); // Both AST and MIR error here
| ^^^^^^^^^^^^^^^^^^^^^^ mutable borrow occurs here
|
note: borrowed value must be valid for the anonymous lifetime #1 defined on the function body at 41:1...
--> $DIR/get_default.rs:41:1
|
LL | / fn err(map: &mut Map) -> &String {
LL | | loop {
LL | | match map.get() {
LL | | Some(v) => {
... |
LL | | }
LL | | }
| |_^
...
LL | return v;
| - borrow later used here
error[E0502]: cannot borrow `*map` as mutable because it is also borrowed as immutable (Mir)
--> $DIR/get_default.rs:51:17

15
src/test/ui/nll/get_default.stderr
View File

@ -63,18 +63,9 @@ LL | match map.get() {
LL | Some(v) => {
LL | map.set(String::new()); // Both AST and MIR error here
| ^^^ mutable borrow occurs here
|
note: borrowed value must be valid for the anonymous lifetime #1 defined on the function body at 41:1...
--> $DIR/get_default.rs:41:1
|
LL | / fn err(map: &mut Map) -> &String {
LL | | loop {
LL | | match map.get() {
LL | | Some(v) => {
... |
LL | | }
LL | | }
| |_^
...
LL | return v;
| - borrow later used here
error[E0502]: cannot borrow `*map` as mutable because it is also borrowed as immutable (Mir)
--> $DIR/get_default.rs:51:17

17
src/test/ui/nll/return-ref-mut-issue-46557.stderr
View File

@ -1,13 +1,16 @@
error[E0597]: borrowed value does not live long enough
--> $DIR/return-ref-mut-issue-46557.rs:17:21
|
LL | let ref mut x = 1234543; //~ ERROR borrowed value does not live long enough [E0597]
| ^^^^^^^ temporary value does not live long enough
LL | x
LL | }
| - temporary value only lives until here
|
= note: borrowed value must be valid for the static lifetime...
LL | fn gimme_static_mut() -> &'static mut u32 {
| ___________________________________________-
LL | | let ref mut x = 1234543; //~ ERROR borrowed value does not live long enough [E0597]
| | ^^^^^^^ temporary value does not live long enough
LL | | x
LL | | }
| | -
| | |
| |_temporary value only lives until here
| borrow later used here
error: aborting due to previous error