diff options
| -rw-r--r-- | src/librustc/middle/region.rs | 76 | ||||
| -rw-r--r-- | src/librustc_mir/transform/qualify_consts.rs | 71 | ||||
| -rw-r--r-- | src/librustc_passes/consts.rs | 37 | ||||
| -rw-r--r-- | src/test/compile-fail/check-static-values-constraints.rs | 3 | ||||
| -rw-r--r-- | src/test/compile-fail/static-drop-scope.rs | 26 | ||||
| -rw-r--r-- | src/test/run-pass/rvalue-static-promotion.rs | 18 |
6 files changed, 177 insertions, 54 deletions
diff --git a/src/librustc/middle/region.rs b/src/librustc/middle/region.rs index a611b3b937b..8b01d5045c6 100644 --- a/src/librustc/middle/region.rs +++ b/src/librustc/middle/region.rs @@ -223,7 +223,9 @@ pub struct RegionMaps { /// table, the appropriate cleanup scope is the innermost /// enclosing statement, conditional expression, or repeating /// block (see `terminating_scopes`). - rvalue_scopes: NodeMap<CodeExtent>, + /// In constants, None is used to indicate that certain expressions + /// escape into 'static and should have no local cleanup scope. + rvalue_scopes: NodeMap<Option<CodeExtent>>, /// Encodes the hierarchy of fn bodies. Every fn body (including /// closures) forms its own distinct region hierarchy, rooted in @@ -358,9 +360,11 @@ impl<'tcx> RegionMaps { self.var_map.insert(var, lifetime); } - fn record_rvalue_scope(&mut self, var: ast::NodeId, lifetime: CodeExtent) { + fn record_rvalue_scope(&mut self, var: ast::NodeId, lifetime: Option<CodeExtent>) { debug!("record_rvalue_scope(sub={:?}, sup={:?})", var, lifetime); - assert!(var != lifetime.node_id()); + if let Some(lifetime) = lifetime { + assert!(var != lifetime.node_id()); + } self.rvalue_scopes.insert(var, lifetime); } @@ -389,7 +393,7 @@ impl<'tcx> RegionMaps { // check for a designated rvalue scope if let Some(&s) = self.rvalue_scopes.get(&expr_id) { debug!("temporary_scope({:?}) = {:?} [custom]", expr_id, s); - return Some(s); + return s; } // else, locate the innermost terminating scope @@ -803,16 +807,11 @@ fn resolve_expr<'a, 'tcx>(visitor: &mut RegionResolutionVisitor<'a, 'tcx>, expr: } fn resolve_local<'a, 'tcx>(visitor: &mut RegionResolutionVisitor<'a, 'tcx>, - local: &'tcx hir::Local) { - debug!("resolve_local(local.id={:?},local.init={:?})", - local.id,local.init.is_some()); + pat: Option<&'tcx hir::Pat>, + init: Option<&'tcx hir::Expr>) { + debug!("resolve_local(pat={:?}, init={:?})", pat, init); - // For convenience in trans, associate with the local-id the var - // scope that will be used for any bindings declared in this - // pattern. let blk_scope = visitor.cx.var_parent; - let blk_scope = blk_scope.expect("locals must be within a block"); - visitor.region_maps.record_var_scope(local.id, blk_scope); // As an exception to the normal rules governing temporary // lifetimes, initializers in a let have a temporary lifetime @@ -872,15 +871,22 @@ fn resolve_local<'a, 'tcx>(visitor: &mut RegionResolutionVisitor<'a, 'tcx>, // // FIXME(#6308) -- Note that `[]` patterns work more smoothly post-DST. - if let Some(ref expr) = local.init { + if let Some(expr) = init { record_rvalue_scope_if_borrow_expr(visitor, &expr, blk_scope); - if is_binding_pat(&local.pat) { - record_rvalue_scope(visitor, &expr, blk_scope); + if let Some(pat) = pat { + if is_binding_pat(pat) { + record_rvalue_scope(visitor, &expr, blk_scope); + } } } - intravisit::walk_local(visitor, local); + if let Some(pat) = pat { + visitor.visit_pat(pat); + } + if let Some(expr) = init { + visitor.visit_expr(expr); + } /// True if `pat` match the `P&` nonterminal: /// @@ -954,7 +960,7 @@ fn resolve_local<'a, 'tcx>(visitor: &mut RegionResolutionVisitor<'a, 'tcx>, fn record_rvalue_scope_if_borrow_expr<'a, 'tcx>( visitor: &mut RegionResolutionVisitor<'a, 'tcx>, expr: &hir::Expr, - blk_id: CodeExtent) + blk_id: Option<CodeExtent>) { match expr.node { hir::ExprAddrOf(_, ref subexpr) => { @@ -1004,7 +1010,7 @@ 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) { + blk_scope: Option<CodeExtent>) { let mut expr = expr; loop { // Note: give all the expressions matching `ET` with the @@ -1077,12 +1083,7 @@ impl<'a, 'tcx> Visitor<'tcx> for RegionResolutionVisitor<'a, 'tcx> { let outer_cx = self.cx; let outer_ts = mem::replace(&mut self.terminating_scopes, NodeSet()); - - // Only functions have an outer terminating (drop) scope, - // while temporaries in constant initializers are 'static. - if let MirSource::Fn(_) = MirSource::from_node(self.tcx, owner_id) { - self.terminating_scopes.insert(body_id.node_id); - } + self.terminating_scopes.insert(body_id.node_id); if let Some(root_id) = self.cx.root_id { self.region_maps.record_fn_parent(body_id.node_id, root_id); @@ -1100,7 +1101,30 @@ impl<'a, 'tcx> Visitor<'tcx> for RegionResolutionVisitor<'a, 'tcx> { // The body of the every fn is a root scope. self.cx.parent = self.cx.var_parent; - self.visit_expr(&body.value); + if let MirSource::Fn(_) = MirSource::from_node(self.tcx, owner_id) { + self.visit_expr(&body.value); + } else { + // Only functions have an outer terminating (drop) scope, while + // temporaries in constant initializers may be 'static, but only + // according to rvalue lifetime semantics, using the same + // syntactical rules used for let initializers. + // + // E.g. in `let x = &f();`, the temporary holding the result from + // the `f()` call lives for the entirety of the surrounding block. + // + // Similarly, `const X: ... = &f();` would have the result of `f()` + // live for `'static`, implying (if Drop restrictions on constants + // ever get lifted) that the value *could* have a destructor, but + // it'd get leaked instead of the destructor running during the + // evaluation of `X` (if at all allowed by CTFE). + // + // However, `const Y: ... = g(&f());`, like `let y = g(&f());`, + // would *not* let the `f()` temporary escape into an outer scope + // (i.e. `'static`), which means that after `g` returns, it drops, + // and all the associated destruction scope rules apply. + self.cx.var_parent = None; + resolve_local(self, None, Some(&body.value)); + } // Restore context we had at the start. self.cx = outer_cx; @@ -1120,7 +1144,7 @@ impl<'a, 'tcx> Visitor<'tcx> for RegionResolutionVisitor<'a, 'tcx> { resolve_expr(self, ex); } fn visit_local(&mut self, l: &'tcx Local) { - resolve_local(self, l); + resolve_local(self, Some(&l.pat), l.init.as_ref().map(|e| &**e)); } } diff --git a/src/librustc_mir/transform/qualify_consts.rs b/src/librustc_mir/transform/qualify_consts.rs index 9126949156c..2198b533ea0 100644 --- a/src/librustc_mir/transform/qualify_consts.rs +++ b/src/librustc_mir/transform/qualify_consts.rs @@ -120,6 +120,7 @@ struct Qualifier<'a, 'gcx: 'a+'tcx, 'tcx: 'a> { return_qualif: Option<Qualif>, qualif: Qualif, const_fn_arg_vars: BitVector, + local_needs_drop: IndexVec<Local, Option<Span>>, temp_promotion_state: IndexVec<Local, TempState>, promotion_candidates: Vec<Candidate> } @@ -146,6 +147,7 @@ impl<'a, 'tcx> Qualifier<'a, 'tcx, 'tcx> { return_qualif: None, qualif: Qualif::empty(), const_fn_arg_vars: BitVector::new(mir.local_decls.len()), + local_needs_drop: IndexVec::from_elem(None, &mir.local_decls), temp_promotion_state: temps, promotion_candidates: vec![] } @@ -194,15 +196,25 @@ impl<'a, 'tcx> Qualifier<'a, 'tcx, 'tcx> { } /// Check for NEEDS_DROP (from an ADT or const fn call) and + /// error, unless we're in a function. + fn always_deny_drop(&self) { + self.deny_drop_with_feature_gate_override(false); + } + + /// Check for NEEDS_DROP (from an ADT or const fn call) and /// error, unless we're in a function, or the feature-gate /// for globals with destructors is enabled. fn deny_drop(&self) { + self.deny_drop_with_feature_gate_override(true); + } + + fn deny_drop_with_feature_gate_override(&self, allow_gate: bool) { if self.mode == Mode::Fn || !self.qualif.intersects(Qualif::NEEDS_DROP) { return; } // Static and const fn's allow destructors, but they're feature-gated. - let msg = if self.mode != Mode::Const { + let msg = if allow_gate && self.mode != Mode::Const { // Feature-gate for globals with destructors is enabled. if self.tcx.sess.features.borrow().drop_types_in_const { return; @@ -223,7 +235,7 @@ impl<'a, 'tcx> Qualifier<'a, 'tcx, 'tcx> { let mut err = struct_span_err!(self.tcx.sess, self.span, E0493, "{}", msg); - if self.mode != Mode::Const { + if allow_gate && self.mode != Mode::Const { help!(&mut err, "in Nightly builds, add `#![feature(drop_types_in_const)]` \ to the crate attributes to enable"); @@ -231,7 +243,8 @@ impl<'a, 'tcx> Qualifier<'a, 'tcx, 'tcx> { self.find_drop_implementation_method_span() .map(|span| err.span_label(span, "destructor defined here")); - err.span_label(self.span, "constants cannot have destructors"); + err.span_label(self.span, + format!("{}s cannot have destructors", self.mode)); } err.emit(); @@ -314,6 +327,15 @@ impl<'a, 'tcx> Qualifier<'a, 'tcx, 'tcx> { return; } + // When initializing a local, record whether the *value* being + // stored in it needs dropping, which it may not, even if its + // type does, e.g. `None::<String>`. + if let Lvalue::Local(local) = *dest { + if qualif.intersects(Qualif::NEEDS_DROP) { + self.local_needs_drop[local] = Some(self.span); + } + } + match *dest { Lvalue::Local(index) if self.mir.local_kind(index) == LocalKind::Temp => { debug!("store to temp {:?}", index); @@ -360,7 +382,6 @@ impl<'a, 'tcx> Qualifier<'a, 'tcx, 'tcx> { let target = match mir[bb].terminator().kind { TerminatorKind::Goto { target } | - // Drops are considered noops. TerminatorKind::Drop { target, .. } | TerminatorKind::Assert { target, .. } | TerminatorKind::Call { destination: Some((_, target)), .. } => { @@ -560,22 +581,32 @@ impl<'a, 'tcx> Visitor<'tcx> for Qualifier<'a, 'tcx, 'tcx> { fn visit_operand(&mut self, operand: &Operand<'tcx>, location: Location) { match *operand { - Operand::Consume(_) => { + Operand::Consume(ref lvalue) => { self.nest(|this| { this.super_operand(operand, location); this.try_consume(); }); + + // Mark the consumed locals to indicate later drops are noops. + if let Lvalue::Local(local) = *lvalue { + self.local_needs_drop[local] = None; + } } Operand::Constant(ref constant) => { - if let Literal::Item { def_id, substs } = constant.literal { - // Don't peek inside generic (associated) constants. - if substs.types().next().is_some() { + if let Literal::Item { def_id, substs: _ } = constant.literal { + // Don't peek inside trait associated constants. + if self.tcx.trait_of_item(def_id).is_some() { self.add_type(constant.ty); } else { let bits = self.tcx.at(constant.span).mir_const_qualif(def_id); let qualif = Qualif::from_bits(bits).expect("invalid mir_const_qualif"); self.add(qualif); + + // Just in case the type is more specific than + // the definition, e.g. impl associated const + // with type parameters, take it into account. + self.qualif.restrict(constant.ty, self.tcx, self.param_env); } // Let `const fn` transitively have destructors, @@ -866,6 +897,30 @@ impl<'a, 'tcx> Visitor<'tcx> for Qualifier<'a, 'tcx, 'tcx> { } self.assign(dest, location); } + } else if let TerminatorKind::Drop { location: ref lvalue, .. } = *kind { + self.super_terminator_kind(bb, kind, location); + + // Deny *any* live drops anywhere other than functions. + if self.mode != Mode::Fn { + // HACK(eddyb) Emulate a bit of dataflow analysis, + // conservatively, that drop elaboration will do. + let needs_drop = if let Lvalue::Local(local) = *lvalue { + self.local_needs_drop[local] + } else { + None + }; + + if let Some(span) = needs_drop { + let ty = lvalue.ty(self.mir, self.tcx).to_ty(self.tcx); + self.add_type(ty); + + // Use the original assignment span to be more precise. + let old_span = self.span; + self.span = span; + self.always_deny_drop(); + self.span = old_span; + } + } } else { // Qualify any operands inside other terminators. self.super_terminator_kind(bb, kind, location); diff --git a/src/librustc_passes/consts.rs b/src/librustc_passes/consts.rs index 918be14ce9d..41ba7d678e8 100644 --- a/src/librustc_passes/consts.rs +++ b/src/librustc_passes/consts.rs @@ -87,19 +87,14 @@ impl<'a, 'gcx> CheckCrateVisitor<'a, 'gcx> { } } - // Adds the worst effect out of all the values of one type. - fn add_type(&mut self, ty: Ty<'gcx>) { - if !ty.is_freeze(self.tcx, self.param_env, DUMMY_SP) { - self.promotable = false; - } - - if ty.needs_drop(self.tcx, self.param_env) { - self.promotable = false; - } + // Returns true iff all the values of the type are promotable. + fn type_has_only_promotable_values(&mut self, ty: Ty<'gcx>) -> bool { + ty.is_freeze(self.tcx, self.param_env, DUMMY_SP) && + !ty.needs_drop(self.tcx, self.param_env) } fn handle_const_fn_call(&mut self, def_id: DefId, ret_ty: Ty<'gcx>) { - self.add_type(ret_ty); + self.promotable &= self.type_has_only_promotable_values(ret_ty); self.promotable &= if let Some(fn_id) = self.tcx.hir.as_local_node_id(def_id) { FnLikeNode::from_node(self.tcx.hir.get(fn_id)).map_or(false, |fn_like| { @@ -333,20 +328,30 @@ fn check_expr<'a, 'tcx>(v: &mut CheckCrateVisitor<'a, 'tcx>, e: &hir::Expr, node match def { Def::VariantCtor(..) | Def::StructCtor(..) | Def::Fn(..) | Def::Method(..) => {} - Def::AssociatedConst(_) => v.add_type(node_ty), - Def::Const(did) => { - v.promotable &= if let Some(node_id) = v.tcx.hir.as_local_node_id(did) { - match v.tcx.hir.expect_item(node_id).node { - hir::ItemConst(_, body) => { + + Def::Const(did) | + Def::AssociatedConst(did) => { + let promotable = if v.tcx.trait_of_item(did).is_some() { + // Don't peek inside trait associated constants. + false + } else if let Some(node_id) = v.tcx.hir.as_local_node_id(did) { + match v.tcx.hir.maybe_body_owned_by(node_id) { + Some(body) => { v.visit_nested_body(body); v.tcx.rvalue_promotable_to_static.borrow()[&body.node_id] } - _ => false + None => false } } else { v.tcx.const_is_rvalue_promotable_to_static(did) }; + + // Just in case the type is more specific than the definition, + // e.g. impl associated const with type parameters, check it. + // Also, trait associated consts are relaxed by this. + v.promotable &= promotable || v.type_has_only_promotable_values(node_ty); } + _ => { v.promotable = false; } diff --git a/src/test/compile-fail/check-static-values-constraints.rs b/src/test/compile-fail/check-static-values-constraints.rs index 3642add3259..c349aababd6 100644 --- a/src/test/compile-fail/check-static-values-constraints.rs +++ b/src/test/compile-fail/check-static-values-constraints.rs @@ -86,8 +86,9 @@ static STATIC8: SafeStruct = SafeStruct{field1: SafeEnum::Variant1, // This example should fail because field1 in the base struct is not safe static STATIC9: SafeStruct = SafeStruct{field1: SafeEnum::Variant1, ..SafeStruct{field1: SafeEnum::Variant3(WithDtor), +//~^ ERROR destructors in statics are an unstable feature +//~| ERROR statics are not allowed to have destructors field2: SafeEnum::Variant1}}; -//~^^ ERROR destructors in statics are an unstable feature struct UnsafeStruct; diff --git a/src/test/compile-fail/static-drop-scope.rs b/src/test/compile-fail/static-drop-scope.rs new file mode 100644 index 00000000000..e5f10b65cee --- /dev/null +++ b/src/test/compile-fail/static-drop-scope.rs @@ -0,0 +1,26 @@ +// Copyright 2017 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. + +#![feature(drop_types_in_const)] + +struct WithDtor; + +impl Drop for WithDtor { + fn drop(&mut self) {} +} + +static FOO: Option<&'static WithDtor> = Some(&WithDtor); +//~^ ERROR statics are not allowed to have destructors +//~| ERROR borrowed value does not live long enoug + +static BAR: i32 = (WithDtor, 0).1; +//~^ ERROR statics are not allowed to have destructors + +fn main () {} diff --git a/src/test/run-pass/rvalue-static-promotion.rs b/src/test/run-pass/rvalue-static-promotion.rs index e57491930a4..acf96b566df 100644 --- a/src/test/run-pass/rvalue-static-promotion.rs +++ b/src/test/run-pass/rvalue-static-promotion.rs @@ -8,8 +8,20 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -#[allow(unused_variables)] +use std::cell::Cell; + +const NONE_CELL_STRING: Option<Cell<String>> = None; + +struct Foo<T>(T); +impl<T> Foo<T> { + const FOO: Option<Box<T>> = None; +} + fn main() { - let x: &'static u32 = &42; - let y: &'static Option<u32> = &None; + let _: &'static u32 = &42; + let _: &'static Option<u32> = &None; + + // We should be able to peek at consts and see they're None. + let _: &'static Option<Cell<String>> = &NONE_CELL_STRING; + let _: &'static Option<Box<()>> = &Foo::FOO; } |