Skip to content
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

rustc: remove temporary lifetime extension by borrow hint #42396

Merged
merged 1 commit into from
Jun 3, 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
1 change: 0 additions & 1 deletion src/librustc/middle/expr_use_visitor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -298,7 +298,6 @@ 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: 10 additions & 20 deletions src/librustc/middle/mem_categorization.rs
Original file line number Diff line number Diff line change
Expand Up @@ -88,8 +88,7 @@ use std::rc::Rc;

#[derive(Clone, PartialEq)]
pub enum Categorization<'tcx> {
// temporary val, argument is its scope
Rvalue(ty::Region<'tcx>, ty::Region<'tcx>),
Rvalue(ty::Region<'tcx>), // temporary val, argument is its scope
StaticItem,
Upvar(Upvar), // upvar referenced by closure env
Local(ast::NodeId), // local variable
Expand Down Expand Up @@ -827,18 +826,13 @@ 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) -> (ty::Region<'tcx>, ty::Region<'tcx>)
pub fn temporary_scope(&self, id: ast::NodeId) -> ty::Region<'tcx>
{
let (scope, old_scope) =
self.region_maps.old_and_new_temporary_scope(id);
(self.tcx().mk_region(match scope {
let scope = self.region_maps.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 @@ -858,13 +852,12 @@ 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, old_re) = if promotable {
(self.tcx().types.re_static,
self.tcx().types.re_static)
let re = if promotable {
self.tcx().types.re_static
} else {
self.temporary_scope(id)
};
let ret = self.cat_rvalue(id, span, re, old_re, expr_ty);
let ret = self.cat_rvalue(id, span, re, expr_ty);
debug!("cat_rvalue_node ret {:?}", ret);
ret
}
Expand All @@ -873,12 +866,11 @@ impl<'a, 'gcx, 'tcx> MemCategorizationContext<'a, 'gcx, 'tcx> {
cmt_id: ast::NodeId,
span: Span,
temp_scope: ty::Region<'tcx>,
old_temp_scope: ty::Region<'tcx>,
expr_ty: Ty<'tcx>) -> cmt<'tcx> {
let ret = Rc::new(cmt_ {
id:cmt_id,
span:span,
cat:Categorization::Rvalue(temp_scope, old_temp_scope),
cat:Categorization::Rvalue(temp_scope),
mutbl:McDeclared,
ty:expr_ty,
note: NoteNone
Expand Down Expand Up @@ -1415,9 +1407,7 @@ 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, or) => {
write!(f, "rvalue({:?}, {:?})", r, or)
}
Categorization::Rvalue(r) => { write!(f, "rvalue({:?})", r) }
Categorization::Local(id) => {
let name = ty::tls::with(|tcx| tcx.local_var_name_str(id));
write!(f, "local({})", name)
Expand Down
72 changes: 5 additions & 67 deletions src/librustc/middle/region.rs
Original file line number Diff line number Diff line change
Expand Up @@ -223,13 +223,6 @@ pub struct RegionMaps {
/// block (see `terminating_scopes`).
rvalue_scopes: 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: 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 @@ -301,7 +294,6 @@ impl<'tcx> RegionMaps {
destruction_scopes: FxHashMap(),
var_map: NodeMap(),
rvalue_scopes: NodeMap(),
shrunk_rvalue_scopes: NodeMap(),
fn_tree: NodeMap(),
}
}
Expand Down Expand Up @@ -370,12 +362,6 @@ impl<'tcx> RegionMaps {
self.rvalue_scopes.insert(var, lifetime);
}

fn record_shrunk_rvalue_scope(&mut self, var: ast::NodeId, lifetime: CodeExtent) {
debug!("record_rvalue_scope(sub={:?}, sup={:?})", var, lifetime);
assert!(var != lifetime.node_id());
self.shrunk_rvalue_scopes.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.get(&id).cloned()
Expand All @@ -395,32 +381,6 @@ impl<'tcx> 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.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
.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 @@ -896,10 +856,7 @@ fn resolve_local<'a, 'tcx>(visitor: &mut RegionResolutionVisitor<'a, 'tcx>,
// Rule A. `let (ref x, ref y) = (foo().x, 44)`. The rvalue `(22, 44)`
// would have an extended lifetime, but not `foo()`.
//
// Rule B. `let x: &[...] = [foo().x]`. The rvalue `[foo().x]`
// would have an extended lifetime, but not `foo()`.
//
// Rule C. `let x = &foo().x`. The rvalue ``foo()` would have extended
// Rule B. `let x = &foo().x`. The rvalue ``foo()` would have extended
// lifetime.
//
// In some cases, multiple rules may apply (though not to the same
Expand All @@ -916,13 +873,8 @@ fn resolve_local<'a, 'tcx>(visitor: &mut RegionResolutionVisitor<'a, 'tcx>,
if let Some(ref expr) = local.init {
record_rvalue_scope_if_borrow_expr(visitor, &expr, blk_scope);

let is_borrow =
if let Some(ref ty) = local.ty { is_borrowed_ty(&ty) } else { false };

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);
record_rvalue_scope(visitor, &expr, blk_scope);
}
}

Expand Down Expand Up @@ -963,14 +915,6 @@ fn resolve_local<'a, 'tcx>(visitor: &mut RegionResolutionVisitor<'a, 'tcx>,
}
}

/// True if `ty` is a borrowed pointer type like `&int` or `&[...]`.
fn is_borrowed_ty(ty: &hir::Ty) -> bool {
match ty.node {
hir::TyRptr(..) => true,
_ => false
}
}

/// If `expr` matches the `E&` grammar, then records an extended rvalue scope as appropriate:
///
/// E& = & ET
Expand All @@ -989,7 +933,7 @@ fn resolve_local<'a, 'tcx>(visitor: &mut RegionResolutionVisitor<'a, 'tcx>,
match expr.node {
hir::ExprAddrOf(_, ref subexpr) => {
record_rvalue_scope_if_borrow_expr(visitor, &subexpr, blk_id);
record_rvalue_scope(visitor, &subexpr, blk_id, false);
record_rvalue_scope(visitor, &subexpr, blk_id);
}
hir::ExprStruct(_, ref fields, _) => {
for field in fields {
Expand Down Expand Up @@ -1034,21 +978,15 @@ fn resolve_local<'a, 'tcx>(visitor: &mut RegionResolutionVisitor<'a, 'tcx>,
/// Note: ET is intended to match "rvalues or lvalues based on rvalues".
fn record_rvalue_scope<'a, 'tcx>(visitor: &mut RegionResolutionVisitor<'a, 'tcx>,
expr: &hir::Expr,
blk_scope: CodeExtent,
is_shrunk: bool) {
blk_scope: CodeExtent) {
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.
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);
}
visitor.region_maps.record_rvalue_scope(expr.id, blk_scope);

match expr.node {
hir::ExprAddrOf(_, ref subexpr) |
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
11 changes: 0 additions & 11 deletions src/librustc_borrowck/borrowck/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1125,17 +1125,6 @@ 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
2 changes: 1 addition & 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,7 @@ 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: _, temp_lifetime_was_shrunk: _, span, kind }
let Expr { ty, temp_lifetime: _, span, kind }
= expr;
match kind {
ExprKind::Scope { extent: _, value } =>
Expand Down
7 changes: 0 additions & 7 deletions src/librustc_mir/build/expr/as_temp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,13 +50,6 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
let temp = this.temp(expr_ty.clone(), 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 !expr_ty.is_never() && temp_lifetime.is_some() {
this.cfg.push(block, Statement {
source_info: source_info,
Expand Down
3 changes: 1 addition & 2 deletions src/librustc_mir/hair/cx/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -82,11 +82,10 @@ 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, was_shrunk) = cx.region_maps.temporary_scope2(block.id);
let temp_lifetime = cx.region_maps.temporary_scope(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