librustc: Redo the unsafe checker and make unsafe methods not callable from safe code

src/libextra/arc.rs

@@ -198,6 +198,7 @@ pub impl<T:Owned> MutexARC<T> {
      */
     #[inline(always)]
     unsafe fn access<U>(&self, blk: &fn(x: &mut T) -> U) -> U {
+        unsafe {
             let state = self.x.get();
             // Borrowck would complain about this if the function were
             // not already unsafe. See borrow_rwlock, far below.
@@ -207,6 +208,7 @@ pub impl<T:Owned> MutexARC<T> {
                 blk(&mut (*state).data)
             }
         }
+    }
 
     /// As access(), but with a condvar, as sync::mutex.lock_cond().
     #[inline(always)]
@@ -356,8 +358,8 @@ pub impl<T:Const + Owned> RWARC<T> {
      * access modes, this will not poison the ARC.
      */
     fn read<U>(&self, blk: &fn(x: &T) -> U) -> U {
-        let state = self.x.get();
         unsafe {
+            let state = self.x.get();
             do (*state).lock.read {
                 check_poison(false, (*state).failed);
                 blk(&(*state).data)

src/libextra/sync.rs

@@ -100,6 +100,7 @@ fn new_sem_and_signal(count: int, num_condvars: uint)
 #[doc(hidden)]
 pub impl<Q:Owned> Sem<Q> {
     fn acquire(&self) {
+        unsafe {
             let mut waiter_nobe = None;
             do (**self).with |state| {
                 state.count -= 1;
@@ -119,7 +120,9 @@ pub impl<Q:Owned> Sem<Q> {
                 let _ = comm::recv_one(waiter_nobe.unwrap());
             }
         }
+    }
     fn release(&self) {
+        unsafe {
             do (**self).with |state| {
                 state.count += 1;
                 if state.count <= 0 {
@@ -127,6 +130,7 @@ pub impl<Q:Owned> Sem<Q> {
                 }
             }
         }
+    }
 }
 // FIXME(#3154) move both copies of this into Sem<Q>, and unify the 2 structs
 #[doc(hidden)]
@@ -283,6 +287,7 @@ pub impl<'self> Condvar<'self> {
 
     /// As signal, but with a specified condvar_id. See wait_on.
     fn signal_on(&self, condvar_id: uint) -> bool {
+        unsafe {
             let mut out_of_bounds = None;
             let mut result = false;
             do (**self.sem).with |state| {
@@ -296,6 +301,7 @@ pub impl<'self> Condvar<'self> {
                 result
             }
         }
+    }
 
     /// Wake up all blocked tasks. Returns the number of tasks woken.
     fn broadcast(&self) -> uint { self.broadcast_on(0) }
@@ -304,6 +310,7 @@ pub impl<'self> Condvar<'self> {
     fn broadcast_on(&self, condvar_id: uint) -> uint {
         let mut out_of_bounds = None;
         let mut queue = None;
+        unsafe {
             do (**self.sem).with |state| {
                 if condvar_id < state.blocked.len() {
                     // To avoid :broadcast_heavy, we make a new waitqueue,
@@ -320,6 +327,7 @@ pub impl<'self> Condvar<'self> {
                 broadcast_waitqueue(&queue)
             }
         }
+    }
 }
 
 // Checks whether a condvar ID was out of bounds, and fails if so, or does

src/librustc/driver/driver.rs

@@ -262,6 +262,9 @@ pub fn compile_rest(sess: Session,
         time(time_passes, ~"privacy checking", ||
              middle::privacy::check_crate(ty_cx, &method_map, crate));
 
+        time(time_passes, ~"effect checking", ||
+             middle::effect::check_crate(ty_cx, method_map, crate));
+
         time(time_passes, ~"loop checking", ||
              middle::check_loop::check_crate(ty_cx, crate));
 

src/librustc/middle/effect.rs

@@ -0,0 +1,154 @@
+// Copyright 2012-2013 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.
+
+//! Enforces the Rust effect system. Currently there is just one effect,
+/// `unsafe`.
+
+use middle::ty::{ty_bare_fn, ty_closure, ty_ptr};
+use middle::ty;
+use middle::typeck::method_map;
+use util::ppaux;
+
+use syntax::ast::{deref, expr_call, expr_inline_asm, expr_method_call};
+use syntax::ast::{expr_unary, node_id, unsafe_blk, unsafe_fn};
+use syntax::ast;
+use syntax::codemap::span;
+use syntax::visit::{fk_item_fn, fk_method};
+use syntax::visit;
+
+#[deriving(Eq)]
+enum UnsafeContext {
+    SafeContext,
+    UnsafeFn,
+    UnsafeBlock(node_id),
+}
+
+struct Context {
+    /// The method map.
+    method_map: method_map,
+    /// Whether we're in an unsafe context.
+    unsafe_context: UnsafeContext,
+}
+
+fn type_is_unsafe_function(ty: ty::t) -> bool {
+    match ty::get(ty).sty {
+        ty_bare_fn(ref f) => f.purity == unsafe_fn,
+        ty_closure(ref f) => f.purity == unsafe_fn,
+        _ => false,
+    }
+}
+
+pub fn check_crate(tcx: ty::ctxt,
+                   method_map: method_map,
+                   crate: @ast::crate) {
+    let context = @mut Context {
+        method_map: method_map,
+        unsafe_context: SafeContext,
+    };
+
+    let require_unsafe: @fn(span: span,
+                            description: &str) = |span, description| {
+        match context.unsafe_context {
+            SafeContext => {
+                // Report an error.
+                tcx.sess.span_err(span,
+                                  fmt!("%s requires unsafe function or block",
+                                       description))
+            }
+            UnsafeBlock(block_id) => {
+                // OK, but record this.
+                debug!("effect: recording unsafe block as used: %?", block_id);
+                let _ = tcx.used_unsafe.insert(block_id);
+            }
+            UnsafeFn => {}
+        }
+    };
+
+    let visitor = visit::mk_vt(@visit::Visitor {
+        visit_fn: |fn_kind, fn_decl, block, span, node_id, _, visitor| {
+            let is_unsafe_fn = match *fn_kind {
+                fk_item_fn(_, _, purity, _) => purity == unsafe_fn,
+                fk_method(_, _, method) => method.purity == unsafe_fn,
+                _ => false,
+            };
+
+            let old_unsafe_context = context.unsafe_context;
+            if is_unsafe_fn {
+                context.unsafe_context = UnsafeFn
+            }
+
+            visit::visit_fn(fn_kind,
+                            fn_decl,
+                            block,
+                            span,
+                            node_id,
+                            (),
+                            visitor);
+
+            context.unsafe_context = old_unsafe_context
+        },
+
+        visit_block: |block, _, visitor| {
+            let old_unsafe_context = context.unsafe_context;
+            if block.node.rules == unsafe_blk {
+                context.unsafe_context = UnsafeBlock(block.node.id)
+            }
+
+            visit::visit_block(block, (), visitor);
+
+            context.unsafe_context = old_unsafe_context
+        },
+
+        visit_expr: |expr, _, visitor| {
+            match expr.node {
+                expr_method_call(*) => {
+                    let base_type = ty::node_id_to_type(tcx, expr.callee_id);
+                    debug!("effect: method call case, base type is %s",
+                           ppaux::ty_to_str(tcx, base_type));
+                    if type_is_unsafe_function(base_type) {
+                        require_unsafe(expr.span,
+                                       "invocation of unsafe method")
+                    }
+                }
+                expr_call(base, _, _) => {
+                    let base_type = ty::node_id_to_type(tcx, base.id);
+                    debug!("effect: call case, base type is %s",
+                           ppaux::ty_to_str(tcx, base_type));
+                    if type_is_unsafe_function(base_type) {
+                        require_unsafe(expr.span, "call to unsafe function")
+                    }
+                }
+                expr_unary(deref, base) => {
+                    let base_type = ty::node_id_to_type(tcx, base.id);
+                    debug!("effect: unary case, base type is %s",
+                           ppaux::ty_to_str(tcx, base_type));
+                    match ty::get(base_type).sty {
+                        ty_ptr(_) => {
+                            require_unsafe(expr.span,
+                                           "dereference of unsafe pointer")
+                        }
+                        _ => {}
+                    }
+                }
+                expr_inline_asm(*) => {
+                    require_unsafe(expr.span, "use of inline assembly")
+                }
+                _ => {}
+            }
+
+            visit::visit_expr(expr, (), visitor)
+        },
+
+        .. *visit::default_visitor()
+    });
+
+    visit::visit_crate(crate, (), visitor)
+}
+

src/librustc/middle/typeck/check/mod.rs

@@ -891,21 +891,6 @@ pub impl FnCtxt {
         infer::mk_subr(self.infcx(), a_is_expected, span, sub, sup)
     }
 
-    fn require_unsafe(&self, sp: span, op: ~str) {
-        match self.ps.purity {
-          ast::unsafe_fn => {
-            // ok, but flag that we used the source of unsafeness
-            debug!("flagging %? as a used unsafe source", self.ps);
-            self.tcx().used_unsafe.insert(self.ps.def);
-          }
-          _ => {
-            self.ccx.tcx.sess.span_err(
-                sp,
-                fmt!("%s requires unsafe function or block", op));
-          }
-        }
-    }
-
     fn with_region_lb<R>(@mut self, lb: ast::node_id, f: &fn() -> R) -> R {
         let old_region_lb = self.region_lb;
         self.region_lb = lb;
@@ -2285,16 +2270,6 @@ pub fn check_expr_with_unifier(fcx: @mut FnCtxt,
                 }
                 ast::deref => {
                     let sty = structure_of(fcx, expr.span, oprnd_t);
-                    match sty {
-                        // deref'ing an unsafe pointer requires that we be in
-                        // an unsafe context
-                        ty::ty_ptr(*) => {
-                            fcx.require_unsafe(
-                                expr.span,
-                                ~"dereference of unsafe pointer");
-                        }
-                        _ => { /*ok*/ }
-                    }
                     let operand_ty = ty::deref_sty(tcx, &sty, true);
                     match operand_ty {
                         Some(mt) => {
@@ -2392,8 +2367,6 @@ pub fn check_expr_with_unifier(fcx: @mut FnCtxt,
         fcx.write_ty(id, ty_param_bounds_and_ty.ty);
       }
       ast::expr_inline_asm(ref ia) => {
-          fcx.require_unsafe(expr.span, ~"use of inline assembly");
-
           for ia.inputs.each |&(_, in)| {
               check_expr(fcx, in);
           }
@@ -3223,13 +3196,6 @@ pub fn ty_param_bounds_and_ty_for_def(fcx: @mut FnCtxt,
         };
       }
 
-      ast::def_fn(id, ast::unsafe_fn) |
-      ast::def_static_method(id, _, ast::unsafe_fn) => {
-        // Unsafe functions can only be touched in an unsafe context
-        fcx.require_unsafe(sp, ~"access to unsafe function");
-        return ty::lookup_item_type(fcx.ccx.tcx, id);
-      }
-
       ast::def_fn(id, _) | ast::def_static_method(id, _, _) |
       ast::def_const(id) | ast::def_variant(_, id) |
       ast::def_struct(id) => {

src/librustc/rustc.rc

@@ -109,6 +109,7 @@ pub mod middle {
     pub mod privacy;
     pub mod moves;
     pub mod entry;
+    pub mod effect;
 }
 
 pub mod front {

src/libstd/comm.rs

@@ -236,22 +236,26 @@ impl<T: Owned> SharedChan<T> {
 
 impl<T: Owned> GenericChan<T> for SharedChan<T> {
     fn send(&self, x: T) {
+        unsafe {
             let mut xx = Some(x);
             do self.ch.with_imm |chan| {
                 let x = replace(&mut xx, None);
                 chan.send(x.unwrap())
             }
         }
+    }
 }
 
 impl<T: Owned> GenericSmartChan<T> for SharedChan<T> {
     fn try_send(&self, x: T) -> bool {
+        unsafe {
             let mut xx = Some(x);
             do self.ch.with_imm |chan| {
                 let x = replace(&mut xx, None);
                 chan.try_send(x.unwrap())
             }
         }
+    }
 }
 
 impl<T: Owned> ::clone::Clone for SharedChan<T> {

src/libstd/os.rs

@@ -861,11 +861,8 @@ pub fn change_dir_locked(p: &Path, action: &fn()) -> bool {
 
     fn key(_: Exclusive<()>) { }
 
-    let result = unsafe {
+    unsafe {
-        global_data_clone_create(key, || {
+        let result = global_data_clone_create(key, || { ~exclusive(()) });
-            ~exclusive(())
-        })
-    };
 
         do result.with_imm() |_| {
             let old_dir = os::getcwd();
@@ -877,6 +874,7 @@ pub fn change_dir_locked(p: &Path, action: &fn()) -> bool {
                 false
             }
         }
+    }
 }
 
 /// Copies a file from one location to another

src/libstd/rt/message_queue.rs

@@ -29,11 +29,14 @@ impl<T: Owned> MessageQueue<T> {
     }
 
     pub fn push(&mut self, value: T) {
+        unsafe {
             let value = Cell(value);
             self.queue.with(|q| q.push(value.take()) );
         }
+    }
 
     pub fn pop(&mut self) -> Option<T> {
+        unsafe {
             do self.queue.with |q| {
                 if !q.is_empty() {
                     Some(q.shift())
@@ -42,6 +45,7 @@ impl<T: Owned> MessageQueue<T> {
                 }
             }
         }
+    }
 }
 
 impl<T> Clone for MessageQueue<T> {

src/libstd/rt/work_queue.rs

@@ -29,11 +29,14 @@ pub impl<T: Owned> WorkQueue<T> {
     }
 
     fn push(&mut self, value: T) {
+        unsafe {
             let value = Cell(value);
             self.queue.with(|q| q.unshift(value.take()) );
         }
+    }
 
     fn pop(&mut self) -> Option<T> {
+        unsafe {
             do self.queue.with |q| {
                 if !q.is_empty() {
                     Some(q.shift())
@@ -42,8 +45,10 @@ pub impl<T: Owned> WorkQueue<T> {
                 }
             }
         }
+    }
 
     fn steal(&mut self) -> Option<T> {
+        unsafe {
             do self.queue.with |q| {
                 if !q.is_empty() {
                     Some(q.pop())
@@ -52,10 +57,13 @@ pub impl<T: Owned> WorkQueue<T> {
                 }
             }
         }
+    }
 
     fn is_empty(&self) -> bool {
+        unsafe {
             self.queue.with_imm(|q| q.is_empty() )
         }
+    }
 }
 
 impl<T> Clone for WorkQueue<T> {

src/libstd/task/spawn.rs

@@ -159,13 +159,17 @@ struct AncestorList(Option<Exclusive<AncestorNode>>);
 // Accessors for taskgroup arcs and ancestor arcs that wrap the unsafety.
 #[inline(always)]
 fn access_group<U>(x: &TaskGroupArc, blk: &fn(TaskGroupInner) -> U) -> U {
+    unsafe {
         x.with(blk)
+    }
 }
 
 #[inline(always)]
 fn access_ancestors<U>(x: &Exclusive<AncestorNode>,
                        blk: &fn(x: &mut AncestorNode) -> U) -> U {
+    unsafe {
         x.with(blk)
+    }
 }
 
 // Iterates over an ancestor list.

src/test/compile-fail/foreign-unsafe-fn-called.rs

@@ -19,5 +19,5 @@ mod test {
 
 fn main() {
     test::free();
-    //~^ ERROR access to unsafe function requires unsafe function or block
+    //~^ ERROR call to unsafe function requires unsafe function or block
 }

src/test/compile-fail/foreign-unsafe-fn.rs

@@ -19,5 +19,5 @@ mod test {
 
 fn main() {
     let x = test::free;
-    //~^ ERROR access to unsafe function requires unsafe function or block
+    //~^ ERROR call to unsafe function requires unsafe function or block
 }

src/test/compile-fail/forget-init-unsafe.rs

@@ -12,6 +12,6 @@ use std::unstable::intrinsics::{init, forget};
 
 // Test that the `forget` and `init` intrinsics are really unsafe
 pub fn main() {
-    let stuff = init::<int>(); //~ ERROR access to unsafe function requires unsafe
+    let stuff = init::<int>(); //~ ERROR call to unsafe function requires unsafe
-    forget(stuff);             //~ ERROR access to unsafe function requires unsafe
+    forget(stuff);             //~ ERROR call to unsafe function requires unsafe
 }

src/test/compile-fail/unsafe-fn-called-from-safe.rs

@@ -13,5 +13,5 @@
 unsafe fn f() { return; }
 
 fn main() {
-    f(); //~ ERROR access to unsafe function requires unsafe function or block
+    f(); //~ ERROR call to unsafe function requires unsafe function or block
 }

src/test/compile-fail/unsafe-fn-used-as-value.rs

@@ -13,6 +13,6 @@
 unsafe fn f() { return; }
 
 fn main() {
-    let x = f; //~ ERROR access to unsafe function requires unsafe function or block
+    let x = f;
-    x();
+    x();    //~ ERROR call to unsafe function requires unsafe function or block
 }