Skip to content

Commit

Permalink
Rollup merge of rust-lang#52239 - CAD97:patch-1, r=alexcrichton
Browse files Browse the repository at this point in the history
Remove sync::Once::call_once 'static bound

See https://internals.rust-lang.org/t/sync-once-per-instance/7918 for more context.

Suggested r is @alexcrichton, the one who added the `'static` bound back in 2014. I don't want to officially r? though, if the system would even let me. I'd rather let the system choose the appropriate member since it knows more than I do.

`git blame` history for `sync::Once::call_once`'s signature:

- [std: Second pass stabilization of sync](rust-lang@f3a7ec7) (Dec 2014)

    ```diff
    -    pub fn doit<F>(&'static self, f: F) where F: FnOnce() {
    +    #[stable]
    +    pub fn call_once<F>(&'static self, f: F) where F: FnOnce() {
    ```

- [libstd: use unboxed closures](rust-lang@cdbb3ca) (Dec 2014)

    ```diff
    -    pub fn doit(&'static self, f: ||) {
    +    pub fn doit<F>(&'static self, f: F) where F: FnOnce() {
    ```

- [std: Rewrite the `sync` module](rust-lang@71d4e77) (Nov 2014)

    ```diff
    -    pub fn doit(&self, f: ||) {
    +    pub fn doit(&'static self, f: ||) {
    ```

    > ```text
    >  The second layer is the layer provided by `std::sync` which is intended to be
    >  the thinnest possible layer on top of `sys_common` which is entirely safe to
    >  use. There are a few concerns which need to be addressed when making these
    >  system primitives safe:
    >
    >    * Once used, the OS primitives can never be **moved**. This means that they
    >      essentially need to have a stable address. The static primitives use
    >      `&'static self` to enforce this, and the non-static primitives all use a
    >      `Box` to provide this guarantee.
    > ```

The author of this diff is @alexcrichton. `sync::Once` now contains only a pointer to (privately hidden) `Waiter`s, which are all stack-allocated. The `'static` bound to `sync::Once` is thus unnecessary to guarantee that any OS primitives are non-relocatable.

As the `'static` bound is not required for `sync::Once`'s operation, removing it is strictly more useful. As an example, it allows attaching a one-time operation to instances rather than only to global singletons.
  • Loading branch information
Mark-Simulacrum committed Jul 11, 2018
2 parents 2d49909 + 0f3f292 commit b41105b
Showing 1 changed file with 6 additions and 6 deletions.
12 changes: 6 additions & 6 deletions src/libstd/sync/once.rs
Expand Up @@ -149,9 +149,9 @@ struct Waiter {

// Helper struct used to clean up after a closure call with a `Drop`
// implementation to also run on panic.
struct Finish {
struct Finish<'a> {
panicked: bool,
me: &'static Once,
me: &'a Once,
}

impl Once {
Expand Down Expand Up @@ -218,7 +218,7 @@ impl Once {
///
/// [poison]: struct.Mutex.html#poisoning
#[stable(feature = "rust1", since = "1.0.0")]
pub fn call_once<F>(&'static self, f: F) where F: FnOnce() {
pub fn call_once<F>(&self, f: F) where F: FnOnce() {
// Fast path, just see if we've completed initialization.
if self.state.load(Ordering::SeqCst) == COMPLETE {
return
Expand Down Expand Up @@ -275,7 +275,7 @@ impl Once {
/// INIT.call_once(|| {});
/// ```
#[unstable(feature = "once_poison", issue = "33577")]
pub fn call_once_force<F>(&'static self, f: F) where F: FnOnce(&OnceState) {
pub fn call_once_force<F>(&self, f: F) where F: FnOnce(&OnceState) {
// same as above, just with a different parameter to `call_inner`.
if self.state.load(Ordering::SeqCst) == COMPLETE {
return
Expand All @@ -299,7 +299,7 @@ impl Once {
// currently no way to take an `FnOnce` and call it via virtual dispatch
// without some allocation overhead.
#[cold]
fn call_inner(&'static self,
fn call_inner(&self,
ignore_poisoning: bool,
init: &mut FnMut(bool)) {
let mut state = self.state.load(Ordering::SeqCst);
Expand Down Expand Up @@ -390,7 +390,7 @@ impl fmt::Debug for Once {
}
}

impl Drop for Finish {
impl<'a> Drop for Finish<'a> {
fn drop(&mut self) {
// Swap out our state with however we finished. We should only ever see
// an old state which was RUNNING.
Expand Down

0 comments on commit b41105b

Please sign in to comment.