Skip to content

End temporary lifetimes being extended by let X: &_ hints #39066

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 1 commit into from
Jan 26, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
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: &ast_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)");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice, thanks for writing that up

}
_ => {}
}
}

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