Skip to content

Commit

Permalink
auto merge of #6038 : alexcrichton/rust/more-unsafe, r=pcwalton
Browse files Browse the repository at this point in the history
Because unsafe functions are never warned about, then all `unsafe` blocks in unsafe functions should definitely be warned about (no need to be redundant). This fixes this case, adds tests, cleans up remaining cases, and then fixes a few other import warnings being spit out.
  • Loading branch information
bors committed Apr 24, 2013
2 parents 8708e0c + 0c2ab66 commit 706096b
Show file tree
Hide file tree
Showing 15 changed files with 160 additions and 144 deletions.
102 changes: 50 additions & 52 deletions src/libcore/gc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -231,66 +231,64 @@ unsafe fn walk_gc_roots(mem: Memory, sentinel: **Word, visitor: Visitor) {
// the stack.
let mut reached_sentinel = ptr::is_null(sentinel);
for stackwalk::walk_stack |frame| {
unsafe {
let pc = last_ret;
let Segment {segment: next_segment, boundary: boundary} =
find_segment_for_frame(frame.fp, segment);
segment = next_segment;
// Each stack segment is bounded by a morestack frame. The
// morestack frame includes two return addresses, one for
// morestack itself, at the normal offset from the frame
// pointer, and then a second return address for the
// function prologue (which called morestack after
// determining that it had hit the end of the stack).
// Since morestack itself takes two parameters, the offset
// for this second return address is 3 greater than the
// return address for morestack.
let ret_offset = if boundary { 4 } else { 1 };
last_ret = *ptr::offset(frame.fp, ret_offset) as *Word;

if ptr::is_null(pc) {
loop;
}
let pc = last_ret;
let Segment {segment: next_segment, boundary: boundary} =
find_segment_for_frame(frame.fp, segment);
segment = next_segment;
// Each stack segment is bounded by a morestack frame. The
// morestack frame includes two return addresses, one for
// morestack itself, at the normal offset from the frame
// pointer, and then a second return address for the
// function prologue (which called morestack after
// determining that it had hit the end of the stack).
// Since morestack itself takes two parameters, the offset
// for this second return address is 3 greater than the
// return address for morestack.
let ret_offset = if boundary { 4 } else { 1 };
last_ret = *ptr::offset(frame.fp, ret_offset) as *Word;

if ptr::is_null(pc) {
loop;
}

let mut delay_reached_sentinel = reached_sentinel;
let sp = is_safe_point(pc);
match sp {
Some(sp_info) => {
for walk_safe_point(frame.fp, sp_info) |root, tydesc| {
// Skip roots until we see the sentinel.
if !reached_sentinel {
if root == sentinel {
delay_reached_sentinel = true;
}
loop;
let mut delay_reached_sentinel = reached_sentinel;
let sp = is_safe_point(pc);
match sp {
Some(sp_info) => {
for walk_safe_point(frame.fp, sp_info) |root, tydesc| {
// Skip roots until we see the sentinel.
if !reached_sentinel {
if root == sentinel {
delay_reached_sentinel = true;
}
loop;
}

// Skip null pointers, which can occur when a
// unique pointer has already been freed.
if ptr::is_null(*root) {
loop;
}
// Skip null pointers, which can occur when a
// unique pointer has already been freed.
if ptr::is_null(*root) {
loop;
}

if ptr::is_null(tydesc) {
// Root is a generic box.
let refcount = **root;
if mem | task_local_heap != 0 && refcount != -1 {
if !visitor(root, tydesc) { return; }
} else if mem | exchange_heap != 0 && refcount == -1 {
if !visitor(root, tydesc) { return; }
}
} else {
// Root is a non-immediate.
if mem | stack != 0 {
if !visitor(root, tydesc) { return; }
}
if ptr::is_null(tydesc) {
// Root is a generic box.
let refcount = **root;
if mem | task_local_heap != 0 && refcount != -1 {
if !visitor(root, tydesc) { return; }
} else if mem | exchange_heap != 0 && refcount == -1 {
if !visitor(root, tydesc) { return; }
}
} else {
// Root is a non-immediate.
if mem | stack != 0 {
if !visitor(root, tydesc) { return; }
}
}
}
None => ()
}
reached_sentinel = delay_reached_sentinel;
}
None => ()
}
reached_sentinel = delay_reached_sentinel;
}
}

Expand Down
4 changes: 1 addition & 3 deletions src/libcore/pipes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -156,9 +156,7 @@ pub impl PacketHeader {
unsafe fn unblock(&self) {
let old_task = swap_task(&mut self.blocked_task, ptr::null());
if !old_task.is_null() {
unsafe {
rustrt::rust_task_deref(old_task)
}
rustrt::rust_task_deref(old_task)
}
match swap_state_acq(&mut self.state, Empty) {
Empty | Blocked => (),
Expand Down
6 changes: 2 additions & 4 deletions src/libcore/rt/sched/local_sched.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,10 +80,8 @@ pub unsafe fn unsafe_borrow() -> &mut Scheduler {
}

pub unsafe fn unsafe_borrow_io() -> &mut IoFactoryObject {
unsafe {
let sched = unsafe_borrow();
return sched.event_loop.io().unwrap();
}
let sched = unsafe_borrow();
return sched.event_loop.io().unwrap();
}

fn tls_key() -> tls::Key {
Expand Down
3 changes: 2 additions & 1 deletion src/libcore/rt/uvio.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,15 @@
use option::*;
use result::*;

use super::io::net::ip::{IpAddr, Ipv4}; // n.b. Ipv4 is used only in tests
use super::io::net::ip::IpAddr;
use super::uv::*;
use super::rtio::*;
use ops::Drop;
use cell::{Cell, empty_cell};
use cast::transmute;
use super::sched::{Scheduler, local_sched};

#[cfg(test)] use super::io::net::ip::Ipv4;
#[cfg(test)] use super::sched::Task;
#[cfg(test)] use unstable::run_in_bare_thread;
#[cfg(test)] use uint;
Expand Down
4 changes: 2 additions & 2 deletions src/libcore/rt/uvll.rs
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ pub enum uv_req_type {

pub unsafe fn malloc_handle(handle: uv_handle_type) -> *c_void {
assert!(handle != UV_UNKNOWN_HANDLE && handle != UV_HANDLE_TYPE_MAX);
let size = unsafe { rust_uv_handle_size(handle as uint) };
let size = rust_uv_handle_size(handle as uint);
let p = malloc(size);
assert!(p.is_not_null());
return p;
Expand All @@ -110,7 +110,7 @@ pub unsafe fn free_handle(v: *c_void) {

pub unsafe fn malloc_req(req: uv_req_type) -> *c_void {
assert!(req != UV_UNKNOWN_REQ && req != UV_REQ_TYPE_MAX);
let size = unsafe { rust_uv_req_size(req as uint) };
let size = rust_uv_req_size(req as uint);
let p = malloc(size);
assert!(p.is_not_null());
return p;
Expand Down
1 change: 1 addition & 0 deletions src/libcore/str/ascii.rs
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,7 @@ impl ToStrConsume for ~[Ascii] {
}
}

#[cfg(test)]
mod tests {
use super::*;

Expand Down
20 changes: 9 additions & 11 deletions src/libcore/unstable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -262,18 +262,16 @@ pub impl<T:Owned> Exclusive<T> {
// the exclusive. Supporting that is a work in progress.
#[inline(always)]
unsafe fn with<U>(&self, f: &fn(x: &mut T) -> U) -> U {
unsafe {
let rec = get_shared_mutable_state(&self.x);
do (*rec).lock.lock {
if (*rec).failed {
fail!(
~"Poisoned exclusive - another task failed inside!");
}
(*rec).failed = true;
let result = f(&mut (*rec).data);
(*rec).failed = false;
result
let rec = get_shared_mutable_state(&self.x);
do (*rec).lock.lock {
if (*rec).failed {
fail!(
~"Poisoned exclusive - another task failed inside!");
}
(*rec).failed = true;
let result = f(&mut (*rec).data);
(*rec).failed = false;
result
}
}

Expand Down
4 changes: 2 additions & 2 deletions src/libcore/unstable/weak_task.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,11 +43,11 @@ pub unsafe fn weaken_task(f: &fn(Port<ShutdownMsg>)) {
let task = get_task_id();
// Expect the weak task service to be alive
assert!(service.try_send(RegisterWeakTask(task, shutdown_chan)));
unsafe { rust_dec_kernel_live_count(); }
rust_dec_kernel_live_count();
do (|| {
f(shutdown_port.take())
}).finally || {
unsafe { rust_inc_kernel_live_count(); }
rust_inc_kernel_live_count();
// Service my have already exited
service.send(UnregisterWeakTask(task));
}
Expand Down
2 changes: 1 addition & 1 deletion src/librustc/back/link.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ use core::char;
use core::hash::Streaming;
use core::hash;
use core::io::WriterUtil;
use core::libc::{c_int, c_uint, c_char};
use core::libc::{c_int, c_uint};
use core::os::consts::{macos, freebsd, linux, android, win32};
use core::os;
use core::ptr;
Expand Down
25 changes: 4 additions & 21 deletions src/librustc/middle/borrowck/check_loans.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
use core::prelude::*;

use middle::moves;
use middle::typeck::check::PurityState;
use middle::borrowck::{Loan, bckerr, BorrowckCtxt, inherent_mutability};
use middle::borrowck::{ReqMaps, root_map_key, save_and_restore_managed};
use middle::borrowck::{MoveError, MoveOk, MoveFromIllegalCmt};
Expand All @@ -41,11 +42,6 @@ use syntax::codemap::span;
use syntax::print::pprust;
use syntax::visit;

struct PurityState {
def: ast::node_id,
purity: ast::purity
}

struct CheckLoanCtxt {
bccx: @BorrowckCtxt,
req_maps: ReqMaps,
Expand Down Expand Up @@ -85,8 +81,7 @@ pub fn check_loans(bccx: @BorrowckCtxt,
bccx: bccx,
req_maps: req_maps,
reported: HashSet::new(),
declared_purity: @mut PurityState { purity: ast::impure_fn,
def: 0 },
declared_purity: @mut PurityState::function(ast::impure_fn, 0),
fn_args: @mut @~[]
};
let vt = visit::mk_vt(@visit::Visitor {visit_expr: check_loans_in_expr,
Expand Down Expand Up @@ -658,9 +653,7 @@ fn check_loans_in_fn(fk: &visit::fn_kind,
debug!("purity on entry=%?", copy self.declared_purity);
do save_and_restore_managed(self.declared_purity) {
do save_and_restore_managed(self.fn_args) {
self.declared_purity = @mut PurityState {
purity: declared_purity, def: src
};
self.declared_purity = @mut PurityState::function(declared_purity, src);
match *fk {
visit::fk_anon(*) |
Expand Down Expand Up @@ -810,17 +803,7 @@ fn check_loans_in_block(blk: &ast::blk,
do save_and_restore_managed(self.declared_purity) {
self.check_for_conflicting_loans(blk.node.id);

match blk.node.rules {
ast::default_blk => {
}
ast::unsafe_blk => {
*self.declared_purity = PurityState {
purity: ast::unsafe_fn,
def: blk.node.id,
};
}
}

*self.declared_purity = self.declared_purity.recurse(blk);
visit::visit_block(blk, self, vt);
}
}
Expand Down
8 changes: 3 additions & 5 deletions src/librustc/middle/trans/base.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ use core::hash;
use core::hashmap::{HashMap, HashSet};
use core::int;
use core::io;
use core::libc::{c_uint, c_ulonglong};
use core::libc::c_uint;
use core::uint;
use std::time;
use syntax::ast::ident;
Expand Down Expand Up @@ -2628,13 +2628,11 @@ pub fn get_item_val(ccx: @CrateContext, id: ast::node_id) -> ValueRef {
let class_ty = ty::lookup_item_type(tcx, parent_id).ty;
// This code shouldn't be reached if the class is generic
assert!(!ty::type_has_params(class_ty));
let lldty = unsafe {
T_fn(~[
let lldty = T_fn(~[
T_ptr(T_i8()),
T_ptr(type_of(ccx, class_ty))
],
T_nil())
};
T_nil());
let s = get_dtor_symbol(ccx, /*bad*/copy *pt, dt.node.id, None);
/* Make the declaration for the dtor */
Expand Down
2 changes: 1 addition & 1 deletion src/librustc/middle/ty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -514,7 +514,7 @@ pub struct substs {
}

mod primitives {
use super::{sty, t_box_};
use super::t_box_;

use syntax::ast;

Expand Down
Loading

0 comments on commit 706096b

Please sign in to comment.