Skip to content

Commit

Permalink
Auto merge of #39066 - arielb1:lifetime-extension-test, r=nikomatsakis
Browse files Browse the repository at this point in the history
End temporary lifetimes being extended by `let X: &_` hints

cc #39283

r? @nikomatsakis
  • Loading branch information
bors committed Jan 26, 2017
2 parents 491b978 + 82a2805 commit 8430042
Show file tree
Hide file tree
Showing 14 changed files with 205 additions and 81 deletions.
5 changes: 0 additions & 5 deletions src/librustc/infer/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ use hir;
use middle::free_region::FreeRegionMap;
use middle::mem_categorization as mc;
use middle::mem_categorization::McResult;
use middle::region::CodeExtent;
use middle::lang_items;
use mir::tcx::LvalueTy;
use ty::subst::{Kind, Subst, Substs};
Expand Down Expand Up @@ -1622,10 +1621,6 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
self.tables.borrow().method_map.contains_key(&ty::MethodCall::expr(id))
}

pub fn temporary_scope(&self, rvalue_id: ast::NodeId) -> Option<CodeExtent> {
self.tcx.region_maps.temporary_scope(rvalue_id)
}

pub fn upvar_capture(&self, upvar_id: ty::UpvarId) -> Option<ty::UpvarCapture<'tcx>> {
self.tables.borrow().upvar_capture_map.get(&upvar_id).cloned()
}
Expand Down
1 change: 1 addition & 0 deletions src/librustc/middle/expr_use_visitor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -296,6 +296,7 @@ impl<'a, 'gcx, 'tcx> ExprUseVisitor<'a, 'gcx, 'tcx> {
arg.id,
arg.pat.span,
fn_body_scope_r, // Args live only as long as the fn body.
fn_body_scope_r,
arg_ty);

self.walk_irrefutable_pat(arg_cmt, &arg.pat);
Expand Down
30 changes: 21 additions & 9 deletions src/librustc/middle/mem_categorization.rs
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,8 @@ use std::rc::Rc;

#[derive(Clone, PartialEq)]
pub enum Categorization<'tcx> {
Rvalue(&'tcx ty::Region), // temporary val, argument is its scope
// temporary val, argument is its scope
Rvalue(&'tcx ty::Region, &'tcx ty::Region),
StaticItem,
Upvar(Upvar), // upvar referenced by closure env
Local(ast::NodeId), // local variable
Expand Down Expand Up @@ -760,11 +761,18 @@ impl<'a, 'gcx, 'tcx> MemCategorizationContext<'a, 'gcx, 'tcx> {

/// Returns the lifetime of a temporary created by expr with id `id`.
/// This could be `'static` if `id` is part of a constant expression.
pub fn temporary_scope(&self, id: ast::NodeId) -> &'tcx ty::Region {
self.tcx().mk_region(match self.infcx.temporary_scope(id) {
pub fn temporary_scope(&self, id: ast::NodeId) -> (&'tcx ty::Region, &'tcx ty::Region)
{
let (scope, old_scope) =
self.tcx().region_maps.old_and_new_temporary_scope(id);
(self.tcx().mk_region(match scope {
Some(scope) => ty::ReScope(scope),
None => ty::ReStatic
}),
self.tcx().mk_region(match old_scope {
Some(scope) => ty::ReScope(scope),
None => ty::ReStatic
})
}))
}

pub fn cat_rvalue_node(&self,
Expand All @@ -785,12 +793,13 @@ impl<'a, 'gcx, 'tcx> MemCategorizationContext<'a, 'gcx, 'tcx> {
// Compute maximum lifetime of this rvalue. This is 'static if
// we can promote to a constant, otherwise equal to enclosing temp
// lifetime.
let re = if promotable {
self.tcx().mk_region(ty::ReStatic)
let (re, old_re) = if promotable {
(self.tcx().mk_region(ty::ReStatic),
self.tcx().mk_region(ty::ReStatic))
} else {
self.temporary_scope(id)
};
let ret = self.cat_rvalue(id, span, re, expr_ty);
let ret = self.cat_rvalue(id, span, re, old_re, expr_ty);
debug!("cat_rvalue_node ret {:?}", ret);
ret
}
Expand All @@ -799,11 +808,12 @@ impl<'a, 'gcx, 'tcx> MemCategorizationContext<'a, 'gcx, 'tcx> {
cmt_id: ast::NodeId,
span: Span,
temp_scope: &'tcx ty::Region,
old_temp_scope: &'tcx ty::Region,
expr_ty: Ty<'tcx>) -> cmt<'tcx> {
let ret = Rc::new(cmt_ {
id:cmt_id,
span:span,
cat:Categorization::Rvalue(temp_scope),
cat:Categorization::Rvalue(temp_scope, old_temp_scope),
mutbl:McDeclared,
ty:expr_ty,
note: NoteNone
Expand Down Expand Up @@ -1386,7 +1396,9 @@ impl<'tcx> fmt::Debug for Categorization<'tcx> {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
match *self {
Categorization::StaticItem => write!(f, "static"),
Categorization::Rvalue(r) => write!(f, "rvalue({:?})", r),
Categorization::Rvalue(r, or) => {
write!(f, "rvalue({:?}, {:?})", r, or)
}
Categorization::Local(id) => {
let name = ty::tls::with(|tcx| tcx.local_var_name_str(id));
write!(f, "local({})", name)
Expand Down
62 changes: 52 additions & 10 deletions src/librustc/middle/region.rs
Original file line number Diff line number Diff line change
Expand Up @@ -272,6 +272,13 @@ pub struct RegionMaps {
/// block (see `terminating_scopes`).
rvalue_scopes: RefCell<NodeMap<CodeExtent>>,

/// Records the value of rvalue scopes before they were shrunk by
/// #36082, for error reporting.
///
/// FIXME: this should be temporary. Remove this by 1.18.0 or
/// so.
shrunk_rvalue_scopes: RefCell<NodeMap<CodeExtent>>,

/// Encodes the hierarchy of fn bodies. Every fn body (including
/// closures) forms its own distinct region hierarchy, rooted in
/// the block that is the fn body. This map points from the id of
Expand Down Expand Up @@ -419,11 +426,7 @@ impl RegionMaps {
e(child, parent)
}
}
pub fn each_rvalue_scope<E>(&self, mut e:E) where E: FnMut(&ast::NodeId, &CodeExtent) {
for (child, parent) in self.rvalue_scopes.borrow().iter() {
e(child, parent)
}
}

/// Records that `sub_fn` is defined within `sup_fn`. These ids
/// should be the id of the block that is the fn body, which is
/// also the root of the region hierarchy for that fn.
Expand Down Expand Up @@ -457,6 +460,12 @@ impl RegionMaps {
self.rvalue_scopes.borrow_mut().insert(var, lifetime);
}

fn record_shrunk_rvalue_scope(&self, var: ast::NodeId, lifetime: CodeExtent) {
debug!("record_rvalue_scope(sub={:?}, sup={:?})", var, lifetime);
assert!(var != lifetime.node_id(self));
self.shrunk_rvalue_scopes.borrow_mut().insert(var, lifetime);
}

pub fn opt_encl_scope(&self, id: CodeExtent) -> Option<CodeExtent> {
//! Returns the narrowest scope that encloses `id`, if any.
self.scope_map.borrow()[id.0 as usize].into_option()
Expand All @@ -476,6 +485,30 @@ impl RegionMaps {
}
}

pub fn temporary_scope2(&self, expr_id: ast::NodeId) -> (Option<CodeExtent>, bool) {
let temporary_scope = self.temporary_scope(expr_id);
let was_shrunk = match self.shrunk_rvalue_scopes.borrow().get(&expr_id) {
Some(&s) => {
info!("temporary_scope2({:?}, scope={:?}, shrunk={:?})",
expr_id, temporary_scope, s);
temporary_scope != Some(s)
}
_ => false
};
info!("temporary_scope2({:?}) - was_shrunk={:?}", expr_id, was_shrunk);
(temporary_scope, was_shrunk)
}

pub fn old_and_new_temporary_scope(&self, expr_id: ast::NodeId) ->
(Option<CodeExtent>, Option<CodeExtent>)
{
let temporary_scope = self.temporary_scope(expr_id);
(temporary_scope,
self.shrunk_rvalue_scopes
.borrow().get(&expr_id).cloned()
.or(temporary_scope))
}

pub fn temporary_scope(&self, expr_id: ast::NodeId) -> Option<CodeExtent> {
//! Returns the scope when temp created by expr_id will be cleaned up

Expand Down Expand Up @@ -929,8 +962,10 @@ fn resolve_local<'a, 'tcx>(visitor: &mut RegionResolutionVisitor<'tcx, 'a>,
let is_borrow =
if let Some(ref ty) = local.ty { is_borrowed_ty(&ty) } else { false };

if is_binding_pat(&local.pat) || is_borrow {
record_rvalue_scope(visitor, &expr, blk_scope);
if is_binding_pat(&local.pat) {
record_rvalue_scope(visitor, &expr, blk_scope, false);
} else if is_borrow {
record_rvalue_scope(visitor, &expr, blk_scope, true);
}
}

Expand Down Expand Up @@ -995,7 +1030,7 @@ fn resolve_local<'a, 'tcx>(visitor: &mut RegionResolutionVisitor<'tcx, 'a>,
match expr.node {
hir::ExprAddrOf(_, ref subexpr) => {
record_rvalue_scope_if_borrow_expr(visitor, &subexpr, blk_id);
record_rvalue_scope(visitor, &subexpr, blk_id);
record_rvalue_scope(visitor, &subexpr, blk_id, false);
}
hir::ExprStruct(_, ref fields, _) => {
for field in fields {
Expand Down Expand Up @@ -1040,15 +1075,21 @@ fn resolve_local<'a, 'tcx>(visitor: &mut RegionResolutionVisitor<'tcx, 'a>,
/// Note: ET is intended to match "rvalues or lvalues based on rvalues".
fn record_rvalue_scope<'a>(visitor: &mut RegionResolutionVisitor,
expr: &'a hir::Expr,
blk_scope: CodeExtent) {
blk_scope: CodeExtent,
is_shrunk: bool) {
let mut expr = expr;
loop {
// Note: give all the expressions matching `ET` with the
// extended temporary lifetime, not just the innermost rvalue,
// because in trans if we must compile e.g. `*rvalue()`
// into a temporary, we request the temporary scope of the
// outer expression.
visitor.region_maps.record_rvalue_scope(expr.id, blk_scope);
if is_shrunk {
// this changed because of #36082
visitor.region_maps.record_shrunk_rvalue_scope(expr.id, blk_scope);
} else {
visitor.region_maps.record_rvalue_scope(expr.id, blk_scope);
}

match expr.node {
hir::ExprAddrOf(_, ref subexpr) |
Expand Down Expand Up @@ -1225,6 +1266,7 @@ pub fn resolve_crate(sess: &Session, map: &hir_map::Map) -> RegionMaps {
scope_map: RefCell::new(vec![]),
var_map: RefCell::new(NodeMap()),
rvalue_scopes: RefCell::new(NodeMap()),
shrunk_rvalue_scopes: RefCell::new(NodeMap()),
fn_tree: RefCell::new(NodeMap()),
};
let root_extent = maps.bogus_code_extent(
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_borrowck/borrowck/gather_loans/lifetime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ impl<'a, 'tcx> GuaranteeLifetimeContext<'a, 'tcx> {
//! rooting etc, and presuming `cmt` is not mutated.

match cmt.cat {
Categorization::Rvalue(temp_scope) => {
Categorization::Rvalue(temp_scope, _) => {
temp_scope
}
Categorization::Upvar(..) => {
Expand Down
13 changes: 12 additions & 1 deletion src/librustc_borrowck/borrowck/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -967,7 +967,7 @@ impl<'a, 'tcx> BorrowckCtxt<'a, 'tcx> {
err_mutbl => self.note_and_explain_mutbl_error(db, &err, &error_span),
err_out_of_scope(super_scope, sub_scope, cause) => {
let (value_kind, value_msg) = match err.cmt.cat {
mc::Categorization::Rvalue(_) =>
mc::Categorization::Rvalue(..) =>
("temporary value", "temporary value created here"),
_ =>
("borrowed value", "borrow occurs here")
Expand Down Expand Up @@ -1061,6 +1061,17 @@ impl<'a, 'tcx> BorrowckCtxt<'a, 'tcx> {
if let Some(_) = statement_scope_span(self.tcx, super_scope) {
db.note("consider using a `let` binding to increase its lifetime");
}



match err.cmt.cat {
mc::Categorization::Rvalue(r, or) if r != or => {
db.note("\
before rustc 1.16, this temporary lived longer - see issue #39283 \
(https://github.com/rust-lang/rust/issues/39283)");
}
_ => {}
}
}

err_borrowed_pointer_too_short(loan_scope, ptr_scope) => {
Expand Down
3 changes: 2 additions & 1 deletion src/librustc_mir/build/expr/as_constant.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,8 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {

fn expr_as_constant(&mut self, expr: Expr<'tcx>) -> Constant<'tcx> {
let this = self;
let Expr { ty, temp_lifetime: _, span, kind } = expr;
let Expr { ty, temp_lifetime: _, temp_lifetime_was_shrunk: _, span, kind }
= expr;
match kind {
ExprKind::Scope { extent: _, value } =>
this.as_constant(value),
Expand Down
7 changes: 7 additions & 0 deletions src/librustc_mir/build/expr/as_temp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,13 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
let expr_span = expr.span;
let source_info = this.source_info(expr_span);

if expr.temp_lifetime_was_shrunk && this.hir.needs_drop(expr_ty) {
this.hir.tcx().sess.span_warn(
expr_span,
"this temporary used to live longer - see issue #39283 \
(https://github.com/rust-lang/rust/issues/39283)");
}

if temp_lifetime.is_some() {
this.cfg.push(block, Statement {
source_info: source_info,
Expand Down
3 changes: 2 additions & 1 deletion src/librustc_mir/hair/cx/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -83,10 +83,11 @@ pub fn to_expr_ref<'a, 'gcx, 'tcx>(cx: &mut Cx<'a, 'gcx, 'tcx>,
block: &'tcx hir::Block)
-> ExprRef<'tcx> {
let block_ty = cx.tables().node_id_to_type(block.id);
let temp_lifetime = cx.tcx.region_maps.temporary_scope(block.id);
let (temp_lifetime, was_shrunk) = cx.tcx.region_maps.temporary_scope2(block.id);
let expr = Expr {
ty: block_ty,
temp_lifetime: temp_lifetime,
temp_lifetime_was_shrunk: was_shrunk,
span: block.span,
kind: ExprKind::Block { body: block },
};
Expand Down
Loading

0 comments on commit 8430042

Please sign in to comment.