Don't allow static items to have types with destructors

src/librustc/driver/driver.rs

@@ -301,6 +301,9 @@ pub fn phase_3_run_analysis_passes(sess: Session,
     // passes are timed inside typeck
     let (method_map, vtable_map) = typeck::check_crate(ty_cx, trait_map, krate);
 
+    time(time_passes, "check static items", (), |_|
+         middle::check_static::check_crate(ty_cx, krate));
+
     // These next two const passes can probably be merged
     time(time_passes, "const marking", (), |_|
          middle::const_eval::process_crate(krate, ty_cx));

src/librustc/lib.rs

@@ -69,6 +69,7 @@ pub mod middle {
     pub mod check_loop;
     pub mod check_match;
     pub mod check_const;
+    pub mod check_static;
     pub mod lint;
     pub mod borrowck;
     pub mod dataflow;

src/librustc/middle/borrowck/gather_loans/gather_moves.rs

@@ -104,7 +104,7 @@ fn check_is_legal_to_move_from(bccx: &BorrowckCtxt,
         mc::cat_deref(_, _, mc::BorrowedPtr(..)) |
         mc::cat_deref(_, _, mc::GcPtr) |
         mc::cat_deref(_, _, mc::UnsafePtr(..)) |
-        mc::cat_upvar(..) |
+        mc::cat_upvar(..) | mc::cat_static_item |
         mc::cat_copied_upvar(mc::CopiedUpvar { onceness: ast::Many, .. }) => {
             bccx.span_err(
                 cmt0.span,
@@ -120,19 +120,6 @@ fn check_is_legal_to_move_from(bccx: &BorrowckCtxt,
             true
         }
 
-        // It seems strange to allow a move out of a static item,
-        // but what happens in practice is that you have a
-        // reference to a constant with a type that should be
-        // moved, like `None::<~int>`.  The type of this constant
-        // is technically `Option<~int>`, which moves, but we know
-        // that the content of static items will never actually
-        // contain allocated pointers, so we can just memcpy it.
-        // Since static items can never have allocated memory,
-        // this is ok. For now anyhow.
-        mc::cat_static_item => {
-            true
-        }
-
         mc::cat_rvalue(..) |
         mc::cat_local(..) |
         mc::cat_arg(..) => {

src/librustc/middle/check_static.rs

@@ -0,0 +1,159 @@
+// 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.
+
+// Verifies that the types and values of static items
+// are safe. The rules enforced by this module are:
+//
+// - For each *mutable* static item, it checks that its **type**:
+//     - doesn't have a destructor
+//     - doesn't own an owned pointer
+//
+// - For each *immutable* static item, it checks that its **value**:
+//       - doesn't own owned, managed pointers
+//       - doesn't contain a struct literal or a call to an enum variant / struct constructor where
+//           - the type of the struct/enum is not freeze
+//           - the type of the struct/enum has a dtor
+
+use middle::ty;
+
+use syntax::ast;
+use syntax::codemap::Span;
+use syntax::visit::Visitor;
+use syntax::visit;
+use syntax::print::pprust;
+
+
+fn safe_type_for_static_mut(cx: ty::ctxt, e: &ast::Expr) -> Option<~str> {
+    let node_ty = ty::node_id_to_type(cx, e.id);
+    let tcontents = ty::type_contents(cx, node_ty);
+    debug!("safe_type_for_static_mut(dtor={}, managed={}, owned={})",
+           tcontents.has_dtor(), tcontents.owns_managed(), tcontents.owns_owned())
+
+    let suffix = if tcontents.has_dtor() {
+        "destructors"
+    } else if tcontents.owns_managed() {
+        "managed pointers"
+    } else if tcontents.owns_owned() {
+        "owned pointers"
+    } else {
+        return None;
+    };
+
+    Some(format!("mutable static items are not allowed to have {}", suffix))
+}
+
+struct CheckStaticVisitor {
+    tcx: ty::ctxt,
+}
+
+pub fn check_crate(tcx: ty::ctxt, krate: &ast::Crate) {
+    visit::walk_crate(&mut CheckStaticVisitor { tcx: tcx }, krate, false)
+}
+
+impl CheckStaticVisitor {
+
+    fn report_error(&self, span: Span, result: Option<~str>) -> bool {
+        match result {
+            None => { false }
+            Some(msg) => {
+                self.tcx.sess.span_err(span, msg);
+                true
+            }
+        }
+    }
+}
+
+impl Visitor<bool> for CheckStaticVisitor {
+
+    fn visit_item(&mut self, i: &ast::Item, _is_const: bool) {
+        debug!("visit_item(item={})", pprust::item_to_str(i));
+        match i.node {
+            ast::ItemStatic(_, mutability, expr) => {
+                match mutability {
+                    ast::MutImmutable => {
+                        self.visit_expr(expr, true);
+                    }
+                    ast::MutMutable => {
+                        self.report_error(expr.span, safe_type_for_static_mut(self.tcx, expr));
+                    }
+                }
+            }
+            _ => { visit::walk_item(self, i, false) }
+        }
+    }
+
+    /// This method is used to enforce the constraints on
+    /// immutable static items. It walks through the *value*
+    /// of the item walking down the expression and evaluating
+    /// every nested expression. if the expression is not part
+    /// of a static item, this method does nothing but walking
+    /// down through it.
+    fn visit_expr(&mut self, e: &ast::Expr, is_const: bool) {
+        debug!("visit_expr(expr={})", pprust::expr_to_str(e));
+
+        if !is_const {
+            return visit::walk_expr(self, e, is_const);
+        }
+
+        match e.node {
+            ast::ExprField(..) | ast::ExprVec(..) |
+            ast::ExprBlock(..) | ast::ExprTup(..) |
+            ast::ExprVstore(_, ast::ExprVstoreSlice) => {
+                visit::walk_expr(self, e, is_const);
+            }
+            ast::ExprUnary(ast::UnBox, _) => {
+                self.tcx.sess.span_err(e.span,
+                                   "static items are not allowed to have managed pointers");
+            }
+            ast::ExprBox(..) |
+            ast::ExprUnary(ast::UnUniq, _) |
+            ast::ExprVstore(_, ast::ExprVstoreUniq) => {
+                self.tcx.sess.span_err(e.span,
+                                   "static items are not allowed to have owned pointers");
+            }
+            ast::ExprProc(..) => {
+                self.report_error(e.span,
+                                  Some(~"immutable static items must be `Freeze`"));
+                return;
+            }
+            ast::ExprAddrOf(mutability, _) => {
+                match mutability {
+                    ast::MutMutable => {
+                        self.report_error(e.span,
+                                  Some(~"immutable static items must be `Freeze`"));
+                        return;
+                    }
+                    _ => {}
+                }
+            }
+            _ => {
+                let node_ty = ty::node_id_to_type(self.tcx, e.id);
+
+                match ty::get(node_ty).sty {
+                    ty::ty_struct(did, _) |
+                    ty::ty_enum(did, _) => {
+                        if ty::has_dtor(self.tcx, did) {
+                            self.report_error(e.span,
+                                     Some(~"static items are not allowed to have destructors"));
+                            return;
+                        }
+                        if Some(did) == self.tcx.lang_items.no_freeze_bound() {
+                            self.report_error(e.span,
+                                              Some(~"immutable static items must be `Freeze`"));
+                            return;
+                        }
+                    }
+                    _ => {}
+                }
+                visit::walk_expr(self, e, is_const);
+            }
+        }
+    }
+}

src/librustc/middle/ty.rs

@@ -1950,6 +1950,10 @@ impl TypeContents {
         self.intersects(TC::OwnsManaged)
     }
 
+    pub fn owns_owned(&self) -> bool {
+        self.intersects(TC::OwnsOwned)
+    }
+
     pub fn is_freezable(&self, _: ctxt) -> bool {
         !self.intersects(TC::Nonfreezable)
     }
@@ -2012,6 +2016,10 @@ impl TypeContents {
     pub fn inverse(&self) -> TypeContents {
         TypeContents { bits: !self.bits }
     }
+
+    pub fn has_dtor(&self) -> bool {
+        self.intersects(TC::OwnsDtor)
+    }
 }
 
 impl ops::BitOr<TypeContents,TypeContents> for TypeContents {
@@ -2038,6 +2046,10 @@ impl fmt::Show for TypeContents {
     }
 }
 
+pub fn type_has_dtor(cx: ctxt, t: ty::t) -> bool {
+    type_contents(cx, t).has_dtor()
+}
+
 pub fn type_is_static(cx: ctxt, t: ty::t) -> bool {
     type_contents(cx, t).is_static(cx)
 }
@@ -2624,61 +2636,6 @@ pub fn type_is_machine(ty: t) -> bool {
     }
 }
 
-// Whether a type is Plain Old Data -- meaning it does not contain pointers
-// that the cycle collector might care about.
-pub fn type_is_pod(cx: ctxt, ty: t) -> bool {
-    let mut result = true;
-    match get(ty).sty {
-      // Scalar types
-      ty_nil | ty_bot | ty_bool | ty_char | ty_int(_) | ty_float(_) | ty_uint(_) |
-      ty_ptr(_) | ty_bare_fn(_) => result = true,
-      // Boxed types
-      ty_box(_) | ty_uniq(_) | ty_closure(_) |
-      ty_str(vstore_uniq) |
-      ty_vec(_, vstore_uniq) |
-      ty_trait(_, _, _, _, _) | ty_rptr(_,_) => result = false,
-      // Structural types
-      ty_enum(did, ref substs) => {
-        let variants = enum_variants(cx, did);
-        for variant in (*variants).iter() {
-            // FIXME(pcwalton): This is an inefficient way to do this. Don't
-            // synthesize a tuple!
-            //
-            // Perform any type parameter substitutions.
-            let tup_ty = mk_tup(cx, variant.args.clone());
-            let tup_ty = subst(cx, substs, tup_ty);
-            if !type_is_pod(cx, tup_ty) { result = false; }
-        }
-      }
-      ty_tup(ref elts) => {
-        for elt in elts.iter() { if !type_is_pod(cx, *elt) { result = false; } }
-      }
-      ty_str(vstore_fixed(_)) => result = true,
-      ty_vec(ref mt, vstore_fixed(_)) | ty_unboxed_vec(ref mt) => {
-        result = type_is_pod(cx, mt.ty);
-      }
-      ty_param(_) => result = false,
-      ty_struct(did, ref substs) => {
-        let fields = lookup_struct_fields(cx, did);
-        result = fields.iter().all(|f| {
-            let fty = ty::lookup_item_type(cx, f.id);
-            let sty = subst(cx, substs, fty.ty);
-            type_is_pod(cx, sty)
-        });
-      }
-
-      ty_str(vstore_slice(..)) | ty_vec(_, vstore_slice(..)) => {
-        result = false;
-      }
-
-      ty_infer(..) | ty_self(..) | ty_err => {
-        cx.sess.bug("non concrete type in type_is_pod");
-      }
-    }
-
-    return result;
-}
-
 pub fn type_is_enum(ty: t) -> bool {
     match get(ty).sty {
       ty_enum(_, _) => return true,

src/librustc/middle/typeck/coherence.rs

@@ -725,6 +725,7 @@ impl CoherenceChecker {
 
             let self_type = self.get_self_type_for_implementation(*impl_info);
             match ty::get(self_type.ty).sty {
+                ty::ty_enum(type_def_id, _) |
                 ty::ty_struct(type_def_id, _) => {
                     let mut destructor_for_type = tcx.destructor_for_type
                                                      .borrow_mut();

src/test/compile-fail/borrowck-move-out-of-static-item.rs

@@ -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.
+
+// Ensure that moves out of static items is forbidden
+
+use std::kinds::marker;
+
+struct Foo {
+    foo: int,
+    nopod: marker::NoPod
+}
+
+static BAR: Foo = Foo{foo: 5, nopod: marker::NoPod};
+
+
+fn test(f: Foo) {
+    let _f = Foo{foo: 4, ..f};
+}
+
+fn main() {
+    test(BAR); //~ ERROR cannot move out of static item
+}