From 4bfb7e400ea14c942f93e3666d0c85acee52ea5d Mon Sep 17 00:00:00 2001 From: Huon Wilson Date: Wed, 29 Jan 2014 23:02:51 +1100 Subject: [PATCH] std: replace the finally closure with a finally macro. This adds a `finally!` macro similar to D's scope statement. It runs a user specified piece of code in the destructor of a stack variable to allow guaranteeing that it runs, even on failure. --- src/libextra/sync.rs | 52 +++++++-------- src/libstd/finally.rs | 102 ++++++++++++++++++++++++++++++ src/libstd/io/extensions.rs | 9 ++- src/libstd/io/mod.rs | 28 ++++----- src/libstd/lib.rs | 3 + src/libstd/os.rs | 10 ++- src/libstd/rt/args.rs | 26 +++----- src/libstd/rt/task.rs | 10 +-- src/libstd/unstable/finally.rs | 111 --------------------------------- src/libstd/unstable/mod.rs | 1 - src/libstd/vec.rs | 18 +++--- src/test/run-pass/glob-std.rs | 1 - 12 files changed, 176 insertions(+), 195 deletions(-) create mode 100644 src/libstd/finally.rs delete mode 100644 src/libstd/unstable/finally.rs diff --git a/src/libextra/sync.rs b/src/libextra/sync.rs index 26a555a646c58..9214749d22119 100644 --- a/src/libextra/sync.rs +++ b/src/libextra/sync.rs @@ -22,12 +22,22 @@ use std::comm; use std::unstable::sync::Exclusive; use std::sync::arc::UnsafeArc; use std::sync::atomics; -use std::unstable::finally::Finally; use std::util; use std::util::NonCopyable; use arc::MutexArc; +#[cfg(stage0)] // SNAP b6400f9 +macro_rules! finally { + ($e: expr) => { + // this `let` has to be free-standing, so that it's directly + // in the scope of the callee of `finally!`, to get the dtor to + // run at the right time. + let _guard = ::std::finally::FinallyGuard::new(|| $e); + } +} + + /**************************************************************************** * Internals ****************************************************************************/ @@ -140,12 +150,9 @@ impl Sem { } pub fn access(&self, blk: || -> U) -> U { - (|| { - self.acquire(); - blk() - }).finally(|| { - self.release(); - }) + self.acquire(); + finally!(self.release()); + blk() } } @@ -233,15 +240,13 @@ impl<'a> Condvar<'a> { // Unconditionally "block". (Might not actually block if a // signaller already sent -- I mean 'unconditionally' in contrast // with acquire().) - (|| { - let _ = WaitEnd.take_unwrap().recv(); - }).finally(|| { + finally!( // Reacquire the condvar. match self.order { Just(lock) => lock.access(|| self.sem.acquire()), Nothing => self.sem.acquire(), - } - }) + }); + let _ = WaitEnd.take_unwrap().recv(); }) } @@ -497,9 +502,8 @@ impl RWLock { state.read_mode = true; } }); - (|| { - blk() - }).finally(|| { + + finally!({ let state = &mut *self.state.get(); assert!(state.read_mode); let old_count = state.read_count.fetch_sub(1, atomics::Release); @@ -512,7 +516,8 @@ impl RWLock { // this access MUST NOT go inside the exclusive access. (&self.access_lock).release(); } - }) + }); + blk() } } @@ -600,9 +605,7 @@ impl RWLock { (&self.order_lock).acquire(); (&self.access_lock).acquire(); (&self.order_lock).release(); - (|| { - blk(RWLockWriteMode { lock: self, token: NonCopyable }) - }).finally(|| { + finally!({ let writer_or_last_reader; // Check if we're releasing from read mode or from write mode. let state = unsafe { &mut *self.state.get() }; @@ -627,7 +630,8 @@ impl RWLock { // Nobody left inside; release the "reader cloud" lock. (&self.access_lock).release(); } - }) + }); + blk(RWLockWriteMode { lock: self, token: NonCopyable }) } /// To be called inside of the write_downgrade block. @@ -1011,7 +1015,6 @@ mod tests { #[ignore(reason = "linked failure")] #[test] fn test_mutex_killed_broadcast() { - use std::unstable::finally::Finally; let m = Mutex::new(); let m2 = m.clone(); @@ -1027,13 +1030,12 @@ mod tests { task::spawn(proc() { // linked mi.lock_cond(|cond| { c.send(()); // tell sibling to go ahead - (|| { - cond.wait(); // block forever - }).finally(|| { + finally!({ error!("task unwinding and sending"); c.send(()); error!("task unwinding and done sending"); - }) + }); + cond.wait(); // block forever }) }); } diff --git a/src/libstd/finally.rs b/src/libstd/finally.rs new file mode 100644 index 0000000000000..5b6a00c5a484e --- /dev/null +++ b/src/libstd/finally.rs @@ -0,0 +1,102 @@ +// 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 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +//! Guarantee that a piece of code is always run. + +#[macro_escape]; + +use ops::Drop; + +#[macro_export] +macro_rules! finally { + ($e: expr) => { + // this `let` has to be free-standing, so that it's directly + // in the finally of the callee of `finally!`, to get the dtor to + // run at the right time. + let _guard = ::std::finally::FinallyGuard::new(|| $e); + } +} + +/// Runs a user-provided function on destruction. +/// +/// One can use this to guarantee that a piece of code is always run, +/// even when the task fails. The `finally!` macro provides a convenient +/// interface, by taking an expression that is wrapped into the +/// appropriate closure. +/// +/// # Example +/// +/// ```rust +/// { +/// finally!(println!("bye")); +/// +/// println!("hi"); +/// } // `hi` then `bye` +/// +/// { +/// finally!(println!("bye")); +/// +/// fail!("oops"); +/// } // always prints `bye` +/// ``` +pub struct FinallyGuard<'a> { + priv f: 'a || +} +impl<'a> FinallyGuard<'a> { + /// Create a new `FinallyGuard`. + pub fn new(f: 'a ||) -> FinallyGuard<'a> { + FinallyGuard { f: f } + } +} + +#[unsafe_destructor] +impl<'a> Drop for FinallyGuard<'a> { + fn drop(&mut self) { + (self.f)() + } +} + +#[cfg(test)] +mod test { + use finally::FinallyGuard; + use comm::Chan; + use task::task; + + #[test] + fn test_no_fail() { + let mut ran = false; + { + finally!(ran = true); + assert!(!ran); + } + assert!(ran) + } + + + #[test] + fn test_fail() { + let mut t = task(); + let completion = t.future_result(); + + let (p, c) = Chan::new(); + + t.spawn(proc() { + finally!(c.send("guarded")); + c.send("unguarded"); + fail!() + }); + + // wait for the task to complete + completion.recv(); + + assert_eq!(p.recv(), "unguarded"); + assert_eq!(p.recv(), "guarded"); + } +} diff --git a/src/libstd/io/extensions.rs b/src/libstd/io/extensions.rs index 26e0a0d09adb9..673c0bed1c979 100644 --- a/src/libstd/io/extensions.rs +++ b/src/libstd/io/extensions.rs @@ -139,7 +139,6 @@ pub fn u64_from_be_bytes(data: &[u8], #[cfg(test)] mod test { - use unstable::finally::Finally; use prelude::*; use io::{MemReader, MemWriter}; use io::{io_error, placeholder_error}; @@ -371,13 +370,13 @@ mod test { }; // FIXME (#7049): Figure out some other way to do this. //let buf = RefCell::new(~[8, 9]); - (|| { - //reader.push_bytes(buf.borrow_mut().get(), 4); - }).finally(|| { + finally!( // NB: Using rtassert here to trigger abort on failure since this is a should_fail test // FIXME: #7049 This fails because buf is still borrowed //rtassert!(buf.borrow().get() == ~[8, 9, 10]); - }) + ); + + //reader.push_bytes(buf.borrow_mut().get(), 4); } #[test] diff --git a/src/libstd/io/mod.rs b/src/libstd/io/mod.rs index cadcbdd51f57b..ce8a115d749a9 100644 --- a/src/libstd/io/mod.rs +++ b/src/libstd/io/mod.rs @@ -301,7 +301,6 @@ use str; use str::{StrSlice, OwnedStr}; use to_str::ToStr; use uint; -use unstable::finally::Finally; use vec::{OwnedVector, MutableVector, ImmutableVector, OwnedCloneableVector}; use vec; @@ -544,22 +543,21 @@ pub trait Reader { buf.reserve_additional(len); buf.set_len(start_len + len); - - (|| { - while total_read < len { - let len = buf.len(); - let slice = buf.mut_slice(start_len + total_read, len); - match self.read(slice) { - Some(nread) => { - total_read += nread; - } - None => { - io_error::cond.raise(standard_error(EndOfFile)); - break; - } + finally!(buf.set_len(start_len + total_read)); + + while total_read < len { + let len = buf.len(); + let slice = buf.mut_slice(start_len + total_read, len); + match self.read(slice) { + Some(nread) => { + total_read += nread; + } + None => { + io_error::cond.raise(standard_error(EndOfFile)); + break; } } - }).finally(|| buf.set_len(start_len + total_read)) + } } } diff --git a/src/libstd/lib.rs b/src/libstd/lib.rs index d7a7011319ae9..315817f75c9ab 100644 --- a/src/libstd/lib.rs +++ b/src/libstd/lib.rs @@ -112,6 +112,8 @@ pub mod bool; pub mod char; pub mod tuple; +pub mod scope; + pub mod vec; pub mod vec_ng; pub mod at_vec; @@ -226,6 +228,7 @@ mod std { pub use option; pub use os; pub use rt; + pub use scope; pub use str; pub use to_bytes; pub use to_str; diff --git a/src/libstd/os.rs b/src/libstd/os.rs index 457d58ae46405..ffa5475835d69 100644 --- a/src/libstd/os.rs +++ b/src/libstd/os.rs @@ -40,7 +40,6 @@ use prelude::*; use ptr; use str; use fmt; -use unstable::finally::Finally; use sync::atomics::{AtomicInt, INIT_ATOMIC_INT, SeqCst}; /// Delegates to the libc close() function, returning the same return value. @@ -133,15 +132,14 @@ Serialize access through a global lock. */ fn with_env_lock(f: || -> T) -> T { use unstable::mutex::{Mutex, MUTEX_INIT}; - use unstable::finally::Finally; static mut lock: Mutex = MUTEX_INIT; unsafe { - return (|| { - lock.lock(); - f() - }).finally(|| lock.unlock()); + lock.lock(); + finally!(lock.unlock()); + + f() } } diff --git a/src/libstd/rt/args.rs b/src/libstd/rt/args.rs index 526ad60bb2134..e44b73e91f846 100644 --- a/src/libstd/rt/args.rs +++ b/src/libstd/rt/args.rs @@ -70,7 +70,6 @@ mod imp { use ptr::RawPtr; use iter::Iterator; #[cfg(not(test))] use str; - use unstable::finally::Finally; use unstable::mutex::{Mutex, MUTEX_INIT}; use util; #[cfg(not(test))] use vec; @@ -114,16 +113,11 @@ mod imp { } fn with_lock(f: || -> T) -> T { - (|| { - unsafe { - lock.lock(); - f() - } - }).finally(|| { - unsafe { - lock.unlock(); - } - }) + unsafe { + lock.lock(); + finally!(lock.unlock()); + f() + } } fn get_global_ptr() -> *mut Option<~~[~str]> { @@ -142,7 +136,6 @@ mod imp { mod tests { use prelude::*; use super::*; - use unstable::finally::Finally; #[test] fn smoke_test() { @@ -156,14 +149,11 @@ mod imp { assert!(take() == Some(expected.clone())); assert!(take() == None); - (|| { - }).finally(|| { - // Restore the actual global state. - match saved_value { + // Restore the actual global state. + finally!(match saved_value { Some(ref args) => put(args.clone()), None => () - } - }) + }) } } } diff --git a/src/libstd/rt/task.rs b/src/libstd/rt/task.rs index 7c43e64f17b15..01e590bc9dbae 100644 --- a/src/libstd/rt/task.rs +++ b/src/libstd/rt/task.rs @@ -34,7 +34,6 @@ use send_str::SendStr; use sync::arc::UnsafeArc; use sync::atomics::{AtomicUint, SeqCst}; use task::{TaskResult, TaskOpts}; -use unstable::finally::Finally; /// The Task struct represents all state associated with a rust /// task. There are at this point two primary "subtypes" of task, @@ -117,8 +116,9 @@ impl Task { // client-specified code and catch any failures. let try_block = || { - // Run the task main function, then do some cleanup. - f.finally(|| { + // Register the clean up to run after we run the main task + // function. + finally!({ fn close_outputs() { let mut task = Local::borrow(None::); let logger = task.get().logger.take(); @@ -171,7 +171,9 @@ impl Task { // runtime-provided type which we have control over what the // destructor does. close_outputs(); - }) + }); + + f() }; unsafe { (*handle).unwinder.try(try_block); } diff --git a/src/libstd/unstable/finally.rs b/src/libstd/unstable/finally.rs deleted file mode 100644 index 6faf69d2bb98f..0000000000000 --- a/src/libstd/unstable/finally.rs +++ /dev/null @@ -1,111 +0,0 @@ -// Copyright 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 or the MIT license -// , at your -// option. This file may not be copied, modified, or distributed -// except according to those terms. - -/*! -The Finally trait provides a method, `finally` on -stack closures that emulates Java-style try/finally blocks. - -# Example - - ``` -(|| { - ... -}).finally(|| { - always_run_this(); -}) - ``` -*/ - -use ops::Drop; - -#[cfg(test)] use task::failing; - -pub trait Finally { - fn finally(&self, dtor: ||) -> T; -} - -macro_rules! finally_fn { - ($fnty:ty) => { - impl Finally for $fnty { - fn finally(&self, dtor: ||) -> T { - let _d = Finallyalizer { - dtor: dtor - }; - (*self)() - } - } - } -} - -impl<'a,T> Finally for 'a || -> T { - fn finally(&self, dtor: ||) -> T { - let _d = Finallyalizer { - dtor: dtor - }; - - (*self)() - } -} - -finally_fn!(extern "Rust" fn() -> T) - -struct Finallyalizer<'a> { - dtor: 'a || -} - -#[unsafe_destructor] -impl<'a> Drop for Finallyalizer<'a> { - #[inline] - fn drop(&mut self) { - (self.dtor)(); - } -} - -#[test] -fn test_success() { - let mut i = 0; - (|| { - i = 10; - }).finally(|| { - assert!(!failing()); - assert_eq!(i, 10); - i = 20; - }); - assert_eq!(i, 20); -} - -#[test] -#[should_fail] -fn test_fail() { - let mut i = 0; - (|| { - i = 10; - fail!(); - }).finally(|| { - assert!(failing()); - assert_eq!(i, 10); - }) -} - -#[test] -fn test_retval() { - let closure: || -> int = || 10; - let i = closure.finally(|| { }); - assert_eq!(i, 10); -} - -#[test] -fn test_compact() { - fn do_some_fallible_work() {} - fn but_always_run_this_function() { } - do_some_fallible_work.finally( - but_always_run_this_function); -} - diff --git a/src/libstd/unstable/mod.rs b/src/libstd/unstable/mod.rs index 87870ef033142..7cad953fec0f4 100644 --- a/src/libstd/unstable/mod.rs +++ b/src/libstd/unstable/mod.rs @@ -15,7 +15,6 @@ use libc::uintptr_t; pub mod dynamic_lib; -pub mod finally; pub mod intrinsics; pub mod simd; #[cfg(not(test))] diff --git a/src/libstd/vec.rs b/src/libstd/vec.rs index 467bcf075f60c..5d1e40cd23165 100644 --- a/src/libstd/vec.rs +++ b/src/libstd/vec.rs @@ -118,7 +118,6 @@ use rt::global_heap::{malloc_raw, realloc_raw, exchange_free}; use mem; use mem::size_of; use uint; -use unstable::finally::Finally; use unstable::intrinsics; use unstable::raw::{Repr, Slice, Vec}; use util; @@ -134,14 +133,15 @@ pub fn from_fn(n_elts: uint, op: |uint| -> T) -> ~[T] { let mut v = with_capacity(n_elts); let p = v.as_mut_ptr(); let mut i: uint = 0u; - (|| { + { + finally!(v.set_len(i)); + while i < n_elts { intrinsics::move_val_init(&mut(*ptr::mut_offset(p, i as int)), op(i)); i += 1u; } - }).finally(|| { - v.set_len(i); - }); + } + v } } @@ -161,14 +161,14 @@ pub fn from_elem(n_elts: uint, t: T) -> ~[T] { let mut v = with_capacity(n_elts); let p = v.as_mut_ptr(); let mut i = 0u; - (|| { + { + finally!(v.set_len(i)); + while i < n_elts { intrinsics::move_val_init(&mut(*ptr::mut_offset(p, i as int)), t.clone()); i += 1u; } - }).finally(|| { - v.set_len(i); - }); + } v } } diff --git a/src/test/run-pass/glob-std.rs b/src/test/run-pass/glob-std.rs index 9e724d86df5eb..bba276090e8e5 100644 --- a/src/test/run-pass/glob-std.rs +++ b/src/test/run-pass/glob-std.rs @@ -16,7 +16,6 @@ extern mod glob; use glob::glob; use extra::tempfile::TempDir; -use std::unstable::finally::Finally; use std::{os, unstable}; use std::io;