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

16 files changed, 288 insertions, 136 deletions

diff --git a/src/libextra/arc.rs b/src/libextra/arc.rs
index 4baa1168b3b..e120f3dd033 100644
--- a/src/libextra/arc.rs
+++ b/src/libextra/arc.rs
@@ -198,13 +198,15 @@ pub impl<T:Owned> MutexARC<T> {
      */
     #[inline(always)]
     unsafe fn access<U>(&self, blk: &fn(x: &mut T) -> U) -> U {
-        let state = self.x.get();
-        // Borrowck would complain about this if the function were
-        // not already unsafe. See borrow_rwlock, far below.
-        do (&(*state).lock).lock {
-            check_poison(true, (*state).failed);
-            let _z = PoisonOnFail(&mut (*state).failed);
-            blk(&mut (*state).data)
+        unsafe {
+            let state = self.x.get();
+            // Borrowck would complain about this if the function were
+            // not already unsafe. See borrow_rwlock, far below.
+            do (&(*state).lock).lock {
+                check_poison(true, (*state).failed);
+                let _z = PoisonOnFail(&mut (*state).failed);
+                blk(&mut (*state).data)
+            }
         }
     }
 
@@ -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)
diff --git a/src/libextra/sync.rs b/src/libextra/sync.rs
index 48f34fdf46a..beea3b6f52b 100644
--- a/src/libextra/sync.rs
+++ b/src/libextra/sync.rs
@@ -100,30 +100,34 @@ fn new_sem_and_signal(count: int, num_condvars: uint)
 #[doc(hidden)]
 pub impl<Q:Owned> Sem<Q> {
     fn acquire(&self) {
-        let mut waiter_nobe = None;
-        do (**self).with |state| {
-            state.count -= 1;
-            if state.count < 0 {
-                // Create waiter nobe.
-                let (WaitEnd, SignalEnd) = comm::oneshot();
-                // Tell outer scope we need to block.
-                waiter_nobe = Some(WaitEnd);
-                // Enqueue ourself.
-                state.waiters.tail.send(SignalEnd);
+        unsafe {
+            let mut waiter_nobe = None;
+            do (**self).with |state| {
+                state.count -= 1;
+                if state.count < 0 {
+                    // Create waiter nobe.
+                    let (WaitEnd, SignalEnd) = comm::oneshot();
+                    // Tell outer scope we need to block.
+                    waiter_nobe = Some(WaitEnd);
+                    // Enqueue ourself.
+                    state.waiters.tail.send(SignalEnd);
+                }
+            }
+            // Uncomment if you wish to test for sem races. Not valgrind-friendly.
+            /* for 1000.times { task::yield(); } */
+            // Need to wait outside the exclusive.
+            if waiter_nobe.is_some() {
+                let _ = comm::recv_one(waiter_nobe.unwrap());
             }
-        }
-        // Uncomment if you wish to test for sem races. Not valgrind-friendly.
-        /* for 1000.times { task::yield(); } */
-        // Need to wait outside the exclusive.
-        if waiter_nobe.is_some() {
-            let _ = comm::recv_one(waiter_nobe.unwrap());
         }
     }
     fn release(&self) {
-        do (**self).with |state| {
-            state.count += 1;
-            if state.count <= 0 {
-                signal_waitqueue(&state.waiters);
+        unsafe {
+            do (**self).with |state| {
+                state.count += 1;
+                if state.count <= 0 {
+                    signal_waitqueue(&state.waiters);
+                }
             }
         }
     }
@@ -283,17 +287,19 @@ pub impl<'self> Condvar<'self> {
 
     /// As signal, but with a specified condvar_id. See wait_on.
     fn signal_on(&self, condvar_id: uint) -> bool {
-        let mut out_of_bounds = None;
-        let mut result = false;
-        do (**self.sem).with |state| {
-            if condvar_id < state.blocked.len() {
-                result = signal_waitqueue(&state.blocked[condvar_id]);
-            } else {
-                out_of_bounds = Some(state.blocked.len());
+        unsafe {
+            let mut out_of_bounds = None;
+            let mut result = false;
+            do (**self.sem).with |state| {
+                if condvar_id < state.blocked.len() {
+                    result = signal_waitqueue(&state.blocked[condvar_id]);
+                } else {
+                    out_of_bounds = Some(state.blocked.len());
+                }
+            }
+            do check_cvar_bounds(out_of_bounds, condvar_id, "cond.signal_on()") {
+                result
             }
-        }
-        do check_cvar_bounds(out_of_bounds, condvar_id, "cond.signal_on()") {
-            result
         }
     }
 
@@ -304,20 +310,22 @@ pub impl<'self> Condvar<'self> {
     fn broadcast_on(&self, condvar_id: uint) -> uint {
         let mut out_of_bounds = None;
         let mut queue = None;
-        do (**self.sem).with |state| {
-            if condvar_id < state.blocked.len() {
-                // To avoid :broadcast_heavy, we make a new waitqueue,
-                // swap it out with the old one, and broadcast on the
-                // old one outside of the little-lock.
-                queue = Some(util::replace(&mut state.blocked[condvar_id],
-                                           new_waitqueue()));
-            } else {
-                out_of_bounds = Some(state.blocked.len());
+        unsafe {
+            do (**self.sem).with |state| {
+                if condvar_id < state.blocked.len() {
+                    // To avoid :broadcast_heavy, we make a new waitqueue,
+                    // swap it out with the old one, and broadcast on the
+                    // old one outside of the little-lock.
+                    queue = Some(util::replace(&mut state.blocked[condvar_id],
+                                               new_waitqueue()));
+                } else {
+                    out_of_bounds = Some(state.blocked.len());
+                }
+            }
+            do check_cvar_bounds(out_of_bounds, condvar_id, "cond.signal_on()") {
+                let queue = queue.swap_unwrap();
+                broadcast_waitqueue(&queue)
             }
-        }
-        do check_cvar_bounds(out_of_bounds, condvar_id, "cond.signal_on()") {
-            let queue = queue.swap_unwrap();
-            broadcast_waitqueue(&queue)
         }
     }
 }
diff --git a/src/librustc/driver/driver.rs b/src/librustc/driver/driver.rs
index 2d2170278ad..b56699927e4 100644
--- a/src/librustc/driver/driver.rs
+++ b/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));
 
diff --git a/src/librustc/middle/effect.rs b/src/librustc/middle/effect.rs
new file mode 100644
index 00000000000..d2f0a5580ef
--- /dev/null
+++ b/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)
+}
+
diff --git a/src/librustc/middle/typeck/check/mod.rs b/src/librustc/middle/typeck/check/mod.rs
index e700b8760fd..f25d451363b 100644
--- a/src/librustc/middle/typeck/check/mod.rs
+++ b/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) => {
diff --git a/src/librustc/rustc.rc b/src/librustc/rustc.rc
index bdab72c4d73..019ca2a0ed5 100644
--- a/src/librustc/rustc.rc
+++ b/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 {
diff --git a/src/libstd/comm.rs b/src/libstd/comm.rs
index 59eb915c239..adc2c21580b 100644
--- a/src/libstd/comm.rs
+++ b/src/libstd/comm.rs
@@ -236,20 +236,24 @@ impl<T: Owned> SharedChan<T> {
 
 impl<T: Owned> GenericChan<T> for SharedChan<T> {
     fn send(&self, x: T) {
-        let mut xx = Some(x);
-        do self.ch.with_imm |chan| {
-            let x = replace(&mut xx, None);
-            chan.send(x.unwrap())
+        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 {
-        let mut xx = Some(x);
-        do self.ch.with_imm |chan| {
-            let x = replace(&mut xx, None);
-            chan.try_send(x.unwrap())
+        unsafe {
+            let mut xx = Some(x);
+            do self.ch.with_imm |chan| {
+                let x = replace(&mut xx, None);
+                chan.try_send(x.unwrap())
+            }
         }
     }
 }
diff --git a/src/libstd/os.rs b/src/libstd/os.rs
index 15c68efc7cc..9c8c2dbe4d7 100644
--- a/src/libstd/os.rs
+++ b/src/libstd/os.rs
@@ -861,20 +861,18 @@ pub fn change_dir_locked(p: &Path, action: &fn()) -> bool {
 
     fn key(_: Exclusive<()>) { }
 
-    let result = unsafe {
-        global_data_clone_create(key, || {
-            ~exclusive(())
-        })
-    };
+    unsafe {
+        let result = global_data_clone_create(key, || { ~exclusive(()) });
 
-    do result.with_imm() |_| {
-        let old_dir = os::getcwd();
-        if change_dir(p) {
-            action();
-            change_dir(&old_dir)
-        }
-        else {
-            false
+        do result.with_imm() |_| {
+            let old_dir = os::getcwd();
+            if change_dir(p) {
+                action();
+                change_dir(&old_dir)
+            }
+            else {
+                false
+            }
         }
     }
 }
diff --git a/src/libstd/rt/message_queue.rs b/src/libstd/rt/message_queue.rs
index eaab9288ac8..fb1a5334646 100644
--- a/src/libstd/rt/message_queue.rs
+++ b/src/libstd/rt/message_queue.rs
@@ -29,16 +29,20 @@ impl<T: Owned> MessageQueue<T> {
     }
 
     pub fn push(&mut self, value: T) {
-        let value = Cell(value);
-        self.queue.with(|q| q.push(value.take()) );
+        unsafe {
+            let value = Cell(value);
+            self.queue.with(|q| q.push(value.take()) );
+        }
     }
 
     pub fn pop(&mut self) -> Option<T> {
-        do self.queue.with |q| {
-            if !q.is_empty() {
-                Some(q.shift())
-            } else {
-                None
+        unsafe {
+            do self.queue.with |q| {
+                if !q.is_empty() {
+                    Some(q.shift())
+                } else {
+                    None
+                }
             }
         }
     }
diff --git a/src/libstd/rt/work_queue.rs b/src/libstd/rt/work_queue.rs
index e9eb663392b..4671a45aaea 100644
--- a/src/libstd/rt/work_queue.rs
+++ b/src/libstd/rt/work_queue.rs
@@ -29,32 +29,40 @@ pub impl<T: Owned> WorkQueue<T> {
     }
 
     fn push(&mut self, value: T) {
-        let value = Cell(value);
-        self.queue.with(|q| q.unshift(value.take()) );
+        unsafe {
+            let value = Cell(value);
+            self.queue.with(|q| q.unshift(value.take()) );
+        }
     }
 
     fn pop(&mut self) -> Option<T> {
-        do self.queue.with |q| {
-            if !q.is_empty() {
-                Some(q.shift())
-            } else {
-                None
+        unsafe {
+            do self.queue.with |q| {
+                if !q.is_empty() {
+                    Some(q.shift())
+                } else {
+                    None
+                }
             }
         }
     }
 
     fn steal(&mut self) -> Option<T> {
-        do self.queue.with |q| {
-            if !q.is_empty() {
-                Some(q.pop())
-            } else {
-                None
+        unsafe {
+            do self.queue.with |q| {
+                if !q.is_empty() {
+                    Some(q.pop())
+                } else {
+                    None
+                }
             }
         }
     }
 
     fn is_empty(&self) -> bool {
-        self.queue.with_imm(|q| q.is_empty() )
+        unsafe {
+            self.queue.with_imm(|q| q.is_empty() )
+        }
     }
 }
 
diff --git a/src/libstd/task/spawn.rs b/src/libstd/task/spawn.rs
index 5941221821a..56b1fb43ff1 100644
--- a/src/libstd/task/spawn.rs
+++ b/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 {
-    x.with(blk)
+    unsafe {
+        x.with(blk)
+    }
 }
 
 #[inline(always)]
 fn access_ancestors<U>(x: &Exclusive<AncestorNode>,
                        blk: &fn(x: &mut AncestorNode) -> U) -> U {
-    x.with(blk)
+    unsafe {
+        x.with(blk)
+    }
 }
 
 // Iterates over an ancestor list.
diff --git a/src/test/compile-fail/foreign-unsafe-fn-called.rs b/src/test/compile-fail/foreign-unsafe-fn-called.rs
index ed8b8088ee4..2f5258aa7f2 100644
--- a/src/test/compile-fail/foreign-unsafe-fn-called.rs
+++ b/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
 }
diff --git a/src/test/compile-fail/foreign-unsafe-fn.rs b/src/test/compile-fail/foreign-unsafe-fn.rs
index 3633267d02c..fce269ab517 100644
--- a/src/test/compile-fail/foreign-unsafe-fn.rs
+++ b/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
 }
diff --git a/src/test/compile-fail/forget-init-unsafe.rs b/src/test/compile-fail/forget-init-unsafe.rs
index 25ab28b5fc0..9ad7a178fb1 100644
--- a/src/test/compile-fail/forget-init-unsafe.rs
+++ b/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
-    forget(stuff);             //~ ERROR access to unsafe function requires unsafe
-}
\ No newline at end of file
+    let stuff = init::<int>(); //~ ERROR call to unsafe function requires unsafe
+    forget(stuff);             //~ ERROR call to unsafe function requires unsafe
+}
diff --git a/src/test/compile-fail/unsafe-fn-called-from-safe.rs b/src/test/compile-fail/unsafe-fn-called-from-safe.rs
index 864dffea8bf..2ea0f5a4ec9 100644
--- a/src/test/compile-fail/unsafe-fn-called-from-safe.rs
+++ b/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
 }
diff --git a/src/test/compile-fail/unsafe-fn-used-as-value.rs b/src/test/compile-fail/unsafe-fn-used-as-value.rs
index cbfb60f0a32..b5565b4821b 100644
--- a/src/test/compile-fail/unsafe-fn-used-as-value.rs
+++ b/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
-    x();
+    let x = f;
+    x();    //~ ERROR call to unsafe function requires unsafe function or block
 }