From 14cd81e5ac4e06b2b0c80803f865a72edd1ac1b7 Mon Sep 17 00:00:00 2001 From: Daniel Sanchez Quiros Date: Thu, 1 Sep 2022 12:23:38 +0200 Subject: [PATCH 01/23] Implemented try_insert_mut --- src/lib.rs | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/src/lib.rs b/src/lib.rs index 70f08de..c14c90b 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -526,6 +526,40 @@ pub mod unsync { }) } + /// Like [`set`](Self::set), but also returns a mutable reference to the final cell value. + /// + /// # Example + /// ``` + /// use once_cell::unsync::OnceCell; + /// + /// let mut cell = OnceCell::new(); + /// assert!(cell.get_mut().is_none()); + /// + /// assert_eq!(cell.try_insert_mut(92), Ok(&mut 92)); + /// assert_eq!(cell.try_insert_mut(62), Err((&mut 92, 62))); + /// + /// assert!(cell.get_mut().is_some()); + /// ``` + pub fn try_insert_mut(&mut self, value: T) -> Result<&mut T, (&mut T, T)> { + // Sadly the only way to avoid double referencing is to use `is_some` and unwrap later on. + // Once [Polonius](https://github.com/rust-lang/polonius) hits stable this should be better with a `match` or `if let` block. + if self.get().is_some() { + // Safety, we already check if it some. + Err((self.get_mut().unwrap(), value)) + } else { + let slot = unsafe { &mut *self.inner.get() }; + // This is the only place where we set the slot, no races + // due to reentrancy/concurrency are possible, and we've + // checked that slot is currently `None`, so this write + // maintains the `inner`'s invariant. + *slot = Some(value); + Ok(match &mut *slot { + Some(value) => value, + None => unsafe { hint::unreachable_unchecked() }, + }) + } + } + /// Gets the contents of the cell, initializing it with `f` /// if the cell was empty. /// From 7a1994916daf32bba559f7763eb039ec202a60db Mon Sep 17 00:00:00 2001 From: Daniel Sanchez Quiros Date: Thu, 1 Sep 2022 12:44:58 +0200 Subject: [PATCH 02/23] Implemented get_mut_or_init and get_mut_or_try_init --- src/lib.rs | 75 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 75 insertions(+) diff --git a/src/lib.rs b/src/lib.rs index c14c90b..78c402e 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -592,6 +592,38 @@ pub mod unsync { } } + /// Gets the contents of the cell as mutable, initializing it with `f` + /// if the cell was empty. + /// + /// # Panics + /// + /// If `f` panics, the panic is propagated to the caller, and the cell + /// remains uninitialized. + /// + /// It is an error to reentrantly initialize the cell from `f`. Doing + /// so results in a panic. + /// + /// # Example + /// ``` + /// use once_cell::unsync::OnceCell; + /// + /// let mut cell = OnceCell::new(); + /// let value = cell.get_mut_or_init(|| 92); + /// assert_eq!(value, &92); + /// let value = cell.get_mut_or_init(|| unreachable!()); + /// assert_eq!(value, &92); + /// ``` + pub fn get_mut_or_init(&mut self, f: F) -> &mut T + where + F: FnOnce() -> T, + { + enum Void {} + match self.get_mut_or_try_init(|| Ok::(f())) { + Ok(val) => val, + Err(void) => match void {}, + } + } + /// Gets the contents of the cell, initializing it with `f` if /// the cell was empty. If the cell was empty and `f` failed, an /// error is returned. @@ -633,6 +665,49 @@ pub mod unsync { Ok(self.get().unwrap()) } + /// Gets the contents of the cell as mutable, initializing it with `f` if + /// the cell was empty. If the cell was empty and `f` failed, an + /// error is returned. + /// + /// # Panics + /// + /// If `f` panics, the panic is propagated to the caller, and the cell + /// remains uninitialized. + /// + /// It is an error to reentrantly initialize the cell from `f`. Doing + /// so results in a panic. + /// + /// # Example + /// ``` + /// use once_cell::unsync::OnceCell; + /// + /// let mut cell = OnceCell::new(); + /// assert_eq!(cell.get_mut_or_try_init(|| Err(())), Err(())); + /// assert!(cell.get().is_none()); + /// let value = cell.get_mut_or_try_init(|| -> Result { + /// Ok(92) + /// }); + /// assert_eq!(value, Ok(&mut 92)); + /// assert_eq!(cell.get(), Some(&92)) + /// ``` + pub fn get_mut_or_try_init(&mut self, f: F) -> Result<&mut T, E> + where + F: FnOnce() -> Result, + { + let mut_ref = if self.get_mut().is_some() { + self.get_mut().unwrap() + } else { + let val = f()?; + // Note that *some* forms of reentrant initialization might lead to + // UB (see `reentrant_init` test). I believe that just removing this + // `assert`, while keeping `set/get` would be sound, but it seems + // better to panic, rather than to silently use an old value. + assert!(self.set(val).is_ok(), "reentrant init"); + self.get_mut().unwrap() + }; + Ok(mut_ref) + } + /// Takes the value out of this `OnceCell`, moving it back to an uninitialized state. /// /// Has no effect and returns `None` if the `OnceCell` hasn't been initialized. From d353abd855af0d4282afa86739fce5843b07a42a Mon Sep 17 00:00:00 2001 From: Daniel Sanchez Quiros Date: Thu, 1 Sep 2022 13:44:48 +0200 Subject: [PATCH 03/23] Implemented mut api for sync::OnceCell --- src/imp_pl.rs | 20 +++++++++++ src/lib.rs | 95 +++++++++++++++++++++++++++++++++++++++++++++++---- 2 files changed, 109 insertions(+), 6 deletions(-) diff --git a/src/imp_pl.rs b/src/imp_pl.rs index d80ca5e..5a87172 100644 --- a/src/imp_pl.rs +++ b/src/imp_pl.rs @@ -112,6 +112,26 @@ impl OnceCell { } } + /// Get a mutable reference to the underlying value, without checking if the cell + /// is initialized. + /// + /// # Safety + /// + /// Caller must ensure that the cell is in initialized state, and that + /// the contents are acquired by (synchronized to) this thread. + pub(crate) unsafe fn get_mut_unchecked(&self) -> &mut T { + debug_assert!(self.is_initialized()); + let slot: &mut Option = &mut *self.value.get(); + match slot { + Some(value) => value, + // This unsafe does improve performance, see `examples/bench`. + None => { + debug_assert!(false); + hint::unreachable_unchecked() + } + } + } + /// Gets the mutable reference to the underlying value. /// Returns `None` if the cell is empty. pub(crate) fn get_mut(&mut self) -> Option<&mut T> { diff --git a/src/lib.rs b/src/lib.rs index 78c402e..24a4c78 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -694,18 +694,15 @@ pub mod unsync { where F: FnOnce() -> Result, { - let mut_ref = if self.get_mut().is_some() { - self.get_mut().unwrap() - } else { + if self.get_mut().is_none() { let val = f()?; // Note that *some* forms of reentrant initialization might lead to // UB (see `reentrant_init` test). I believe that just removing this // `assert`, while keeping `set/get` would be sound, but it seems // better to panic, rather than to silently use an old value. assert!(self.set(val).is_ok(), "reentrant init"); - self.get_mut().unwrap() - }; - Ok(mut_ref) + } + Ok(self.get_mut().unwrap()) } /// Takes the value out of this `OnceCell`, moving it back to an uninitialized state. @@ -1062,6 +1059,17 @@ pub mod sync { self.0.get_unchecked() } + /// Get a mutable reference to the underlying value, without checking if the + /// cell is initialized. + /// + /// # Safety + /// + /// Caller must ensure that the cell is in initialized state, and that + /// the contents are acquired by (synchronized to) this thread. + pub unsafe fn get_mut_unchecked(&self) -> &mut T { + self.0.get_mut_unchecked() + } + /// Sets the contents of this cell to `value`. /// /// Returns `Ok(())` if the cell was empty and `Err(value)` if it was @@ -1152,6 +1160,42 @@ pub mod sync { Err(void) => match void {}, } } + /// Gets the contents of the cell as mutable, initializing it with `f` if the cell + /// was empty. + /// + /// Only a single thread may call `get_mut_or_init`, as the method expects a `&mut self`, + /// that guarantees that only a single thread can hold the reference. + /// + /// # Panics + /// + /// If `f` panics, the panic is propagated to the caller, and the cell + /// remains uninitialized. + /// + /// It is an error to reentrantly initialize the cell from `f`. The + /// exact outcome is unspecified. Current implementation deadlocks, but + /// this may be changed to a panic in the future. + /// + /// # Example + /// ``` + /// use once_cell::sync::OnceCell; + /// + /// let mut cell = OnceCell::new(); + /// let value = cell.get_mut_or_init(|| 92); + /// assert_eq!(value, &mut 92); + /// let mut value = cell.get_mut_or_init(|| unreachable!()); + /// *value += 1; + /// assert_eq!(value, &mut 93); + /// ``` + pub fn get_mut_or_init(&mut self, f: F) -> &mut T + where + F: FnOnce() -> T, + { + enum Void {} + match self.get_mut_or_try_init(|| Ok::(f())) { + Ok(val) => val, + Err(void) => match void {}, + } + } /// Gets the contents of the cell, initializing it with `f` if /// the cell was empty. If the cell was empty and `f` failed, an @@ -1194,6 +1238,45 @@ pub mod sync { Ok(unsafe { self.get_unchecked() }) } + /// Gets the contents of the cell, initializing it with `f` if + /// the cell was empty. If the cell was empty and `f` failed, an + /// error is returned. + /// + /// # Panics + /// + /// If `f` panics, the panic is propagated to the caller, and + /// the cell remains uninitialized. + /// + /// It is an error to reentrantly initialize the cell from `f`. + /// The exact outcome is unspecified. Current implementation + /// deadlocks, but this may be changed to a panic in the future. + /// + /// # Example + /// ``` + /// use once_cell::sync::OnceCell; + /// + /// let mut cell = OnceCell::new(); + /// assert_eq!(cell.get_mut_or_try_init(|| Err(())), Err(())); + /// assert!(cell.get().is_none()); + /// let mut value = cell.get_mut_or_try_init(|| -> Result { + /// Ok(92) + /// }); + /// *value += 1; + /// assert_eq!(value, Ok(&mut 93)); + /// assert_eq!(cell.get(), Some(&93)) + /// ``` + pub fn get_mut_or_try_init(&mut self, f: F) -> Result<&mut T, E> + where + F: FnOnce() -> Result, + { + if self.get_mut().is_none() { + self.0.initialize(f)?; + } + // Safe b/c value is initialized. + debug_assert!(self.0.is_initialized()); + Ok(unsafe { self.get_mut_unchecked() }) + } + /// Takes the value out of this `OnceCell`, moving it back to an uninitialized state. /// /// Has no effect and returns `None` if the `OnceCell` hasn't been initialized. From a7db7ad8f28a4f24ba53649ae429ffff16e420dc Mon Sep 17 00:00:00 2001 From: Daniel Sanchez Quiros Date: Thu, 1 Sep 2022 14:06:58 +0200 Subject: [PATCH 04/23] Add OnceCell extension mut API tests to it.rs --- tests/it.rs | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/tests/it.rs b/tests/it.rs index 476f14b..6a170ad 100644 --- a/tests/it.rs +++ b/tests/it.rs @@ -15,6 +15,12 @@ mod unsync { c.get_or_init(|| panic!("Kabom!")); assert_eq!(c.get(), Some(&92)); + + let mut c = OnceCell::new(); + assert!(c.get().is_none()); + let mut_ref = c.get_mut_or_init(|| 92); + *mut_ref += 1; + assert_eq!(c.get(), Some(&93)); } #[test] @@ -261,6 +267,15 @@ mod sync { } } + #[test] + fn once_cell_get_mut_unchecked() { + let c = OnceCell::new(); + c.set(92).unwrap(); + unsafe { + assert_eq!(c.get_mut_unchecked(), &mut 92); + } + } + #[test] fn once_cell_drop() { static DROP_CNT: AtomicUsize = AtomicUsize::new(0); @@ -319,6 +334,20 @@ mod sync { assert_eq!(cell.get(), Some(&"hello".to_string())); } + #[test] + fn get_mut_or_try_init() { + let mut cell: OnceCell = OnceCell::new(); + assert!(cell.get().is_none()); + + assert_eq!(cell.get_mut_or_try_init(|| Err(())), Err(())); + + assert_eq!( + cell.get_mut_or_try_init(|| Ok::<_, ()>("hello".to_string())), + Ok(&mut "hello".to_string()) + ); + assert_eq!(cell.get(), Some(&"hello".to_string())); + } + #[test] fn wait() { let cell: OnceCell = OnceCell::new(); From ef1d848bfce73c28912cbc8663b40a9e93e8751b Mon Sep 17 00:00:00 2001 From: Daniel Sanchez Quiros Date: Thu, 1 Sep 2022 14:07:12 +0200 Subject: [PATCH 05/23] Add changelog notes --- CHANGELOG.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index ffb5b1d..0b484c6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,13 @@ - +## 1.14.0 + +- Add extension to `async` and `sync` `OnceCell` mut API: + - `get_mut_or_init` + - `get_mut_or_try_init` + + ## 1.13.1 - Make implementation compliant with [strict provenance](https://github.com/rust-lang/rust/issues/95228). From fcdf1542f0264b1454c9c4525bdffe15f1780052 Mon Sep 17 00:00:00 2001 From: Daniel Sanchez Quiros Date: Thu, 1 Sep 2022 11:28:54 +0200 Subject: [PATCH 06/23] Implement force_mut and get_mut for unsync::Lazy --- src/lib.rs | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/src/lib.rs b/src/lib.rs index 24a4c78..208be88 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -848,6 +848,25 @@ pub mod unsync { }) } + /// Forces the evaluation of this lazy value and returns a mutable reference to + /// the result. + /// + /// This is equivalent to the `DerefMut` impl, but is explicit. + /// + /// # Example + /// ``` + /// use once_cell::unsync::Lazy; + /// + /// let mut lazy = Lazy::new(|| 92); + /// + /// assert_eq!(Lazy::force_mut(&mut lazy), &92); + /// assert_eq!(*lazy, 92); + /// ``` + pub fn force_mut(this: &mut Lazy) -> &mut T { + Self::force(this); + this.cell.get_mut().unwrap_or_else(|| unreachable!()) + } + /// Gets the reference to the result of this lazy value if /// it was initialized, otherwise returns `None`. /// @@ -864,6 +883,23 @@ pub mod unsync { pub fn get(this: &Lazy) -> Option<&T> { this.cell.get() } + + /// Gets the mutable reference to the result of this lazy value if + /// it was initialized, otherwise returns `None`. + /// + /// # Example + /// ``` + /// use once_cell::unsync::Lazy; + /// + /// let mut lazy = Lazy::new(|| 92); + /// + /// assert_eq!(Lazy::get_mut(&mut lazy), None); + /// assert_eq!(*lazy, 92); + /// assert_eq!(Lazy::get_mut(&mut lazy), Some(&mut 92)); + /// ``` + pub fn get_mut(this: &mut Lazy) -> Option<&mut T> { + this.cell.get_mut() + } } impl T> Deref for Lazy { From 5c75c22ad694da47b3d2e428a2b0f4ff8f8cf65e Mon Sep 17 00:00:00 2001 From: Daniel Sanchez Quiros Date: Thu, 1 Sep 2022 13:13:52 +0200 Subject: [PATCH 07/23] Added force_mut and get_mut tests to it.rs --- tests/it.rs | 38 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/tests/it.rs b/tests/it.rs index 6a170ad..d1f1920 100644 --- a/tests/it.rs +++ b/tests/it.rs @@ -143,6 +143,44 @@ mod unsync { assert_eq!(called.get(), 1); } + #[test] + fn lazy_force_mut() { + let called = Cell::new(0); + let mut x = Lazy::new(|| { + called.set(called.get() + 1); + 92 + }); + assert_eq!(called.get(), 0); + Lazy::force_mut(&mut x); + assert_eq!(called.get(), 1); + + let y = *x - 30; + assert_eq!(y, 62); + assert_eq!(called.get(), 1); + + *x /= 2; + assert_eq!(*x, 46); + assert_eq!(called.get(), 1); + } + #[test] + fn lazy_get_mut() { + let called = Cell::new(0); + let mut x: Lazy = Lazy::new(|| { + called.set(called.get() + 1); + 92 + }); + + assert_eq!(called.get(), 0); + assert_eq!(*x, 92); + + let mut_ref: &mut u32 = Lazy::get_mut(&mut x).unwrap(); + assert_eq!(called.get(), 1); + + *mut_ref /= 2; + assert_eq!(*x, 46); + assert_eq!(called.get(), 1); + } + #[test] fn lazy_default() { static CALLED: AtomicUsize = AtomicUsize::new(0); From e4f38c195be4e5a7f25d4916adb9dc5ff95a2300 Mon Sep 17 00:00:00 2001 From: Daniel Sanchez Quiros Date: Thu, 1 Sep 2022 14:16:58 +0200 Subject: [PATCH 08/23] Added sync::Lazy version of `force_mut` and `get_mut` --- src/lib.rs | 36 +++++++++++++++++++++++++++++++++++- 1 file changed, 35 insertions(+), 1 deletion(-) diff --git a/src/lib.rs b/src/lib.rs index 208be88..be36e40 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -864,7 +864,7 @@ pub mod unsync { /// ``` pub fn force_mut(this: &mut Lazy) -> &mut T { Self::force(this); - this.cell.get_mut().unwrap_or_else(|| unreachable!()) + Self::get_mut(this).unwrap_or_else(|| unreachable!()) } /// Gets the reference to the result of this lazy value if @@ -1460,6 +1460,23 @@ pub mod sync { }) } + /// Forces the evaluation of this lazy value and + /// returns a mutable reference to the result. This is equivalent + /// to the `Deref` impl, but is explicit. + /// + /// # Example + /// ``` + /// use once_cell::sync::Lazy; + /// + /// let mut lazy = Lazy::new(|| 92); + /// + /// assert_eq!(Lazy::force_mut(&mut lazy), &mut 92); + /// ``` + pub fn force_mut(this: &mut Lazy) -> &mut T { + Self::force(this); + Self::get_mut(this).unwrap_or_else(|| unreachable!()) + } + /// Gets the reference to the result of this lazy value if /// it was initialized, otherwise returns `None`. /// @@ -1476,6 +1493,23 @@ pub mod sync { pub fn get(this: &Lazy) -> Option<&T> { this.cell.get() } + + /// Gets the reference to the result of this lazy value if + /// it was initialized, otherwise returns `None`. + /// + /// # Example + /// ``` + /// use once_cell::sync::Lazy; + /// + /// let mut lazy = Lazy::new(|| 92); + /// + /// assert_eq!(Lazy::get_mut(&mut lazy), None); + /// assert_eq!(&*lazy, &92); + /// assert_eq!(Lazy::get(&mut lazy), Some(&mut 92)); + /// ``` + pub fn get_mut(this: &mut Lazy) -> Option<&mut T> { + this.cell.get_mut() + } } impl T> Deref for Lazy { From 2b0da3ea37f171cf388da099cf4ee0193e8e1ef7 Mon Sep 17 00:00:00 2001 From: Daniel Sanchez Quiros Date: Thu, 1 Sep 2022 14:17:09 +0200 Subject: [PATCH 09/23] Added changelog notes --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0b484c6..aa8df1d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,9 @@ - Add extension to `async` and `sync` `OnceCell` mut API: - `get_mut_or_init` - `get_mut_or_try_init` +- Add extension to `async` and `sync` `Lazy` mut API: + - `force_mut` + - `get_mut` ## 1.13.1 From eef42a7468ad8283794b8f32df0f81c9c1c1092e Mon Sep 17 00:00:00 2001 From: Daniel Sanchez Quiros Date: Thu, 1 Sep 2022 14:17:31 +0200 Subject: [PATCH 10/23] Modify cargo version --- Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index 1b87237..b7ebb35 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "once_cell" -version = "1.13.1" +version = "1.14.0" authors = ["Aleksey Kladov "] license = "MIT OR Apache-2.0" edition = "2018" From 59ea68ebe9de91979510c2aad6b0379573a663ad Mon Sep 17 00:00:00 2001 From: Daniel Sanchez Quiros Date: Thu, 1 Sep 2022 15:03:41 +0200 Subject: [PATCH 11/23] Added try_insert_mut for sync::OnceCell --- src/lib.rs | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/src/lib.rs b/src/lib.rs index be36e40..bfc09f9 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1160,6 +1160,30 @@ pub mod sync { } } + /// Like [`set`](Self::set), but also returns a mutable reference to the final cell value. + /// + /// # Example + /// + /// ``` + /// use once_cell::unsync::OnceCell; + /// + /// let mut cell = OnceCell::new(); + /// assert!(cell.get().is_none()); + /// + /// assert_eq!(cell.try_insert_mut(92), Ok(&mut 92)); + /// assert_eq!(cell.try_insert_mut(62), Err((&mut 92, 62))); + /// + /// assert!(cell.get().is_some()); + /// ``` + pub fn try_insert_mut(&mut self, value: T) -> Result<&mut T, (&mut T, T)> { + let mut value = Some(value); + let res = self.get_mut_or_init(|| unsafe { take_unchecked(&mut value) }); + match value { + None => Ok(res), + Some(value) => Err((res, value)), + } + } + /// Gets the contents of the cell, initializing it with `f` if the cell /// was empty. /// From 61346b3743b86cb851746426ce4e81d32e3ff613 Mon Sep 17 00:00:00 2001 From: Daniel Sanchez Quiros Date: Thu, 1 Sep 2022 15:55:49 +0200 Subject: [PATCH 12/23] Use &mut for get_mut_unchecked --- src/imp_pl.rs | 2 +- src/lib.rs | 4 ++-- tests/it.rs | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/imp_pl.rs b/src/imp_pl.rs index 5a87172..d1caa48 100644 --- a/src/imp_pl.rs +++ b/src/imp_pl.rs @@ -119,7 +119,7 @@ impl OnceCell { /// /// Caller must ensure that the cell is in initialized state, and that /// the contents are acquired by (synchronized to) this thread. - pub(crate) unsafe fn get_mut_unchecked(&self) -> &mut T { + pub(crate) unsafe fn get_mut_unchecked(&mut self) -> &mut T { debug_assert!(self.is_initialized()); let slot: &mut Option = &mut *self.value.get(); match slot { diff --git a/src/lib.rs b/src/lib.rs index bfc09f9..0f0135d 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1102,7 +1102,7 @@ pub mod sync { /// /// Caller must ensure that the cell is in initialized state, and that /// the contents are acquired by (synchronized to) this thread. - pub unsafe fn get_mut_unchecked(&self) -> &mut T { + pub unsafe fn get_mut_unchecked(&mut self) -> &mut T { self.0.get_mut_unchecked() } @@ -1334,7 +1334,7 @@ pub mod sync { } // Safe b/c value is initialized. debug_assert!(self.0.is_initialized()); - Ok(unsafe { self.get_mut_unchecked() }) + Ok(self.get_mut().unwrap_or_else(|| unreachable!())) } /// Takes the value out of this `OnceCell`, moving it back to an uninitialized state. diff --git a/tests/it.rs b/tests/it.rs index d1f1920..6fe98f8 100644 --- a/tests/it.rs +++ b/tests/it.rs @@ -307,7 +307,7 @@ mod sync { #[test] fn once_cell_get_mut_unchecked() { - let c = OnceCell::new(); + let mut c = OnceCell::new(); c.set(92).unwrap(); unsafe { assert_eq!(c.get_mut_unchecked(), &mut 92); From f8d5f76f019bb4f0dba5967e2b4eef007e3a0b21 Mon Sep 17 00:00:00 2001 From: Daniel Sanchez Quiros Date: Thu, 1 Sep 2022 16:10:24 +0200 Subject: [PATCH 13/23] Implemented get_mut_unchecked to imp_std --- src/imp_std.rs | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/src/imp_std.rs b/src/imp_std.rs index 4d5b5fd..59a219b 100644 --- a/src/imp_std.rs +++ b/src/imp_std.rs @@ -122,6 +122,26 @@ impl OnceCell { } } + /// Get a mutable reference to the underlying value, without checking if the cell + /// is initialized. + /// + /// # Safety + /// + /// Caller must ensure that the cell is in initialized state, and that + /// the contents are acquired by (synchronized to) this thread. + pub(crate) unsafe fn get_mut_unchecked(&mut self) -> &mut T { + debug_assert!(self.is_initialized()); + let slot: &mut Option = &mut *self.value.get_mut(); + match slot { + Some(value) => value, + // This unsafe does improve performance, see `examples/bench`. + None => { + debug_assert!(false); + unreachable_unchecked() + } + } + } + /// Gets the mutable reference to the underlying value. /// Returns `None` if the cell is empty. pub(crate) fn get_mut(&mut self) -> Option<&mut T> { From e68da06608c8f32e641e351caa47ef58e5068d72 Mon Sep 17 00:00:00 2001 From: Daniel Sanchez Quiros Date: Thu, 1 Sep 2022 16:10:33 +0200 Subject: [PATCH 14/23] Fix tests --- src/lib.rs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 0f0135d..639c9f0 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1321,9 +1321,8 @@ pub mod sync { /// let mut value = cell.get_mut_or_try_init(|| -> Result { /// Ok(92) /// }); - /// *value += 1; - /// assert_eq!(value, Ok(&mut 93)); - /// assert_eq!(cell.get(), Some(&93)) + /// assert_eq!(value, Ok(&mut 92)); + /// assert_eq!(cell.get(), Some(&92)) /// ``` pub fn get_mut_or_try_init(&mut self, f: F) -> Result<&mut T, E> where @@ -1529,7 +1528,7 @@ pub mod sync { /// /// assert_eq!(Lazy::get_mut(&mut lazy), None); /// assert_eq!(&*lazy, &92); - /// assert_eq!(Lazy::get(&mut lazy), Some(&mut 92)); + /// assert_eq!(Lazy::get_mut(&mut lazy), Some(&mut 92)); /// ``` pub fn get_mut(this: &mut Lazy) -> Option<&mut T> { this.cell.get_mut() From 2ff6d0e916dc7923ed7cefa2ccce01e422e0d9a9 Mon Sep 17 00:00:00 2001 From: Daniel Sanchez Quiros Date: Thu, 1 Sep 2022 16:13:38 +0200 Subject: [PATCH 15/23] Fix get_mut_unchecked, used get instead of get_mut --- src/imp_std.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/imp_std.rs b/src/imp_std.rs index 59a219b..4a307ec 100644 --- a/src/imp_std.rs +++ b/src/imp_std.rs @@ -131,7 +131,7 @@ impl OnceCell { /// the contents are acquired by (synchronized to) this thread. pub(crate) unsafe fn get_mut_unchecked(&mut self) -> &mut T { debug_assert!(self.is_initialized()); - let slot: &mut Option = &mut *self.value.get_mut(); + let slot: &mut Option = &mut *self.value.get(); match slot { Some(value) => value, // This unsafe does improve performance, see `examples/bench`. From 8b191996f4e15bdd73b4a72579a7191ceae547c5 Mon Sep 17 00:00:00 2001 From: Daniel Sanchez Quiros Date: Fri, 2 Sep 2022 07:18:16 +0200 Subject: [PATCH 16/23] Remove get_mut_unchecked --- src/imp_pl.rs | 20 -------------------- src/imp_std.rs | 20 -------------------- src/lib.rs | 11 ----------- tests/it.rs | 9 --------- 4 files changed, 60 deletions(-) diff --git a/src/imp_pl.rs b/src/imp_pl.rs index d1caa48..d80ca5e 100644 --- a/src/imp_pl.rs +++ b/src/imp_pl.rs @@ -112,26 +112,6 @@ impl OnceCell { } } - /// Get a mutable reference to the underlying value, without checking if the cell - /// is initialized. - /// - /// # Safety - /// - /// Caller must ensure that the cell is in initialized state, and that - /// the contents are acquired by (synchronized to) this thread. - pub(crate) unsafe fn get_mut_unchecked(&mut self) -> &mut T { - debug_assert!(self.is_initialized()); - let slot: &mut Option = &mut *self.value.get(); - match slot { - Some(value) => value, - // This unsafe does improve performance, see `examples/bench`. - None => { - debug_assert!(false); - hint::unreachable_unchecked() - } - } - } - /// Gets the mutable reference to the underlying value. /// Returns `None` if the cell is empty. pub(crate) fn get_mut(&mut self) -> Option<&mut T> { diff --git a/src/imp_std.rs b/src/imp_std.rs index 4a307ec..4d5b5fd 100644 --- a/src/imp_std.rs +++ b/src/imp_std.rs @@ -122,26 +122,6 @@ impl OnceCell { } } - /// Get a mutable reference to the underlying value, without checking if the cell - /// is initialized. - /// - /// # Safety - /// - /// Caller must ensure that the cell is in initialized state, and that - /// the contents are acquired by (synchronized to) this thread. - pub(crate) unsafe fn get_mut_unchecked(&mut self) -> &mut T { - debug_assert!(self.is_initialized()); - let slot: &mut Option = &mut *self.value.get(); - match slot { - Some(value) => value, - // This unsafe does improve performance, see `examples/bench`. - None => { - debug_assert!(false); - unreachable_unchecked() - } - } - } - /// Gets the mutable reference to the underlying value. /// Returns `None` if the cell is empty. pub(crate) fn get_mut(&mut self) -> Option<&mut T> { diff --git a/src/lib.rs b/src/lib.rs index 639c9f0..db853eb 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1095,17 +1095,6 @@ pub mod sync { self.0.get_unchecked() } - /// Get a mutable reference to the underlying value, without checking if the - /// cell is initialized. - /// - /// # Safety - /// - /// Caller must ensure that the cell is in initialized state, and that - /// the contents are acquired by (synchronized to) this thread. - pub unsafe fn get_mut_unchecked(&mut self) -> &mut T { - self.0.get_mut_unchecked() - } - /// Sets the contents of this cell to `value`. /// /// Returns `Ok(())` if the cell was empty and `Err(value)` if it was diff --git a/tests/it.rs b/tests/it.rs index 6fe98f8..039da61 100644 --- a/tests/it.rs +++ b/tests/it.rs @@ -305,15 +305,6 @@ mod sync { } } - #[test] - fn once_cell_get_mut_unchecked() { - let mut c = OnceCell::new(); - c.set(92).unwrap(); - unsafe { - assert_eq!(c.get_mut_unchecked(), &mut 92); - } - } - #[test] fn once_cell_drop() { static DROP_CNT: AtomicUsize = AtomicUsize::new(0); From 180305a60861811e42ad021c2777b817d59a0f80 Mon Sep 17 00:00:00 2001 From: Daniel Sanchez Quiros Date: Fri, 2 Sep 2022 07:21:15 +0200 Subject: [PATCH 17/23] Remove oncecell mut extensions --- src/lib.rs | 204 ----------------------------------------------------- 1 file changed, 204 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index db853eb..dcd64c9 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -526,40 +526,6 @@ pub mod unsync { }) } - /// Like [`set`](Self::set), but also returns a mutable reference to the final cell value. - /// - /// # Example - /// ``` - /// use once_cell::unsync::OnceCell; - /// - /// let mut cell = OnceCell::new(); - /// assert!(cell.get_mut().is_none()); - /// - /// assert_eq!(cell.try_insert_mut(92), Ok(&mut 92)); - /// assert_eq!(cell.try_insert_mut(62), Err((&mut 92, 62))); - /// - /// assert!(cell.get_mut().is_some()); - /// ``` - pub fn try_insert_mut(&mut self, value: T) -> Result<&mut T, (&mut T, T)> { - // Sadly the only way to avoid double referencing is to use `is_some` and unwrap later on. - // Once [Polonius](https://github.com/rust-lang/polonius) hits stable this should be better with a `match` or `if let` block. - if self.get().is_some() { - // Safety, we already check if it some. - Err((self.get_mut().unwrap(), value)) - } else { - let slot = unsafe { &mut *self.inner.get() }; - // This is the only place where we set the slot, no races - // due to reentrancy/concurrency are possible, and we've - // checked that slot is currently `None`, so this write - // maintains the `inner`'s invariant. - *slot = Some(value); - Ok(match &mut *slot { - Some(value) => value, - None => unsafe { hint::unreachable_unchecked() }, - }) - } - } - /// Gets the contents of the cell, initializing it with `f` /// if the cell was empty. /// @@ -592,38 +558,6 @@ pub mod unsync { } } - /// Gets the contents of the cell as mutable, initializing it with `f` - /// if the cell was empty. - /// - /// # Panics - /// - /// If `f` panics, the panic is propagated to the caller, and the cell - /// remains uninitialized. - /// - /// It is an error to reentrantly initialize the cell from `f`. Doing - /// so results in a panic. - /// - /// # Example - /// ``` - /// use once_cell::unsync::OnceCell; - /// - /// let mut cell = OnceCell::new(); - /// let value = cell.get_mut_or_init(|| 92); - /// assert_eq!(value, &92); - /// let value = cell.get_mut_or_init(|| unreachable!()); - /// assert_eq!(value, &92); - /// ``` - pub fn get_mut_or_init(&mut self, f: F) -> &mut T - where - F: FnOnce() -> T, - { - enum Void {} - match self.get_mut_or_try_init(|| Ok::(f())) { - Ok(val) => val, - Err(void) => match void {}, - } - } - /// Gets the contents of the cell, initializing it with `f` if /// the cell was empty. If the cell was empty and `f` failed, an /// error is returned. @@ -665,46 +599,6 @@ pub mod unsync { Ok(self.get().unwrap()) } - /// Gets the contents of the cell as mutable, initializing it with `f` if - /// the cell was empty. If the cell was empty and `f` failed, an - /// error is returned. - /// - /// # Panics - /// - /// If `f` panics, the panic is propagated to the caller, and the cell - /// remains uninitialized. - /// - /// It is an error to reentrantly initialize the cell from `f`. Doing - /// so results in a panic. - /// - /// # Example - /// ``` - /// use once_cell::unsync::OnceCell; - /// - /// let mut cell = OnceCell::new(); - /// assert_eq!(cell.get_mut_or_try_init(|| Err(())), Err(())); - /// assert!(cell.get().is_none()); - /// let value = cell.get_mut_or_try_init(|| -> Result { - /// Ok(92) - /// }); - /// assert_eq!(value, Ok(&mut 92)); - /// assert_eq!(cell.get(), Some(&92)) - /// ``` - pub fn get_mut_or_try_init(&mut self, f: F) -> Result<&mut T, E> - where - F: FnOnce() -> Result, - { - if self.get_mut().is_none() { - let val = f()?; - // Note that *some* forms of reentrant initialization might lead to - // UB (see `reentrant_init` test). I believe that just removing this - // `assert`, while keeping `set/get` would be sound, but it seems - // better to panic, rather than to silently use an old value. - assert!(self.set(val).is_ok(), "reentrant init"); - } - Ok(self.get_mut().unwrap()) - } - /// Takes the value out of this `OnceCell`, moving it back to an uninitialized state. /// /// Has no effect and returns `None` if the `OnceCell` hasn't been initialized. @@ -1149,30 +1043,6 @@ pub mod sync { } } - /// Like [`set`](Self::set), but also returns a mutable reference to the final cell value. - /// - /// # Example - /// - /// ``` - /// use once_cell::unsync::OnceCell; - /// - /// let mut cell = OnceCell::new(); - /// assert!(cell.get().is_none()); - /// - /// assert_eq!(cell.try_insert_mut(92), Ok(&mut 92)); - /// assert_eq!(cell.try_insert_mut(62), Err((&mut 92, 62))); - /// - /// assert!(cell.get().is_some()); - /// ``` - pub fn try_insert_mut(&mut self, value: T) -> Result<&mut T, (&mut T, T)> { - let mut value = Some(value); - let res = self.get_mut_or_init(|| unsafe { take_unchecked(&mut value) }); - match value { - None => Ok(res), - Some(value) => Err((res, value)), - } - } - /// Gets the contents of the cell, initializing it with `f` if the cell /// was empty. /// @@ -1209,42 +1079,6 @@ pub mod sync { Err(void) => match void {}, } } - /// Gets the contents of the cell as mutable, initializing it with `f` if the cell - /// was empty. - /// - /// Only a single thread may call `get_mut_or_init`, as the method expects a `&mut self`, - /// that guarantees that only a single thread can hold the reference. - /// - /// # Panics - /// - /// If `f` panics, the panic is propagated to the caller, and the cell - /// remains uninitialized. - /// - /// It is an error to reentrantly initialize the cell from `f`. The - /// exact outcome is unspecified. Current implementation deadlocks, but - /// this may be changed to a panic in the future. - /// - /// # Example - /// ``` - /// use once_cell::sync::OnceCell; - /// - /// let mut cell = OnceCell::new(); - /// let value = cell.get_mut_or_init(|| 92); - /// assert_eq!(value, &mut 92); - /// let mut value = cell.get_mut_or_init(|| unreachable!()); - /// *value += 1; - /// assert_eq!(value, &mut 93); - /// ``` - pub fn get_mut_or_init(&mut self, f: F) -> &mut T - where - F: FnOnce() -> T, - { - enum Void {} - match self.get_mut_or_try_init(|| Ok::(f())) { - Ok(val) => val, - Err(void) => match void {}, - } - } /// Gets the contents of the cell, initializing it with `f` if /// the cell was empty. If the cell was empty and `f` failed, an @@ -1287,44 +1121,6 @@ pub mod sync { Ok(unsafe { self.get_unchecked() }) } - /// Gets the contents of the cell, initializing it with `f` if - /// the cell was empty. If the cell was empty and `f` failed, an - /// error is returned. - /// - /// # Panics - /// - /// If `f` panics, the panic is propagated to the caller, and - /// the cell remains uninitialized. - /// - /// It is an error to reentrantly initialize the cell from `f`. - /// The exact outcome is unspecified. Current implementation - /// deadlocks, but this may be changed to a panic in the future. - /// - /// # Example - /// ``` - /// use once_cell::sync::OnceCell; - /// - /// let mut cell = OnceCell::new(); - /// assert_eq!(cell.get_mut_or_try_init(|| Err(())), Err(())); - /// assert!(cell.get().is_none()); - /// let mut value = cell.get_mut_or_try_init(|| -> Result { - /// Ok(92) - /// }); - /// assert_eq!(value, Ok(&mut 92)); - /// assert_eq!(cell.get(), Some(&92)) - /// ``` - pub fn get_mut_or_try_init(&mut self, f: F) -> Result<&mut T, E> - where - F: FnOnce() -> Result, - { - if self.get_mut().is_none() { - self.0.initialize(f)?; - } - // Safe b/c value is initialized. - debug_assert!(self.0.is_initialized()); - Ok(self.get_mut().unwrap_or_else(|| unreachable!())) - } - /// Takes the value out of this `OnceCell`, moving it back to an uninitialized state. /// /// Has no effect and returns `None` if the `OnceCell` hasn't been initialized. From 8f6ca60a6341345016cd4332d5d6f8e026116759 Mon Sep 17 00:00:00 2001 From: Daniel Sanchez Quiros Date: Fri, 2 Sep 2022 07:22:18 +0200 Subject: [PATCH 18/23] Remove oncecell mut extensions tests --- tests/it.rs | 20 -------------------- 1 file changed, 20 deletions(-) diff --git a/tests/it.rs b/tests/it.rs index 039da61..b501304 100644 --- a/tests/it.rs +++ b/tests/it.rs @@ -15,12 +15,6 @@ mod unsync { c.get_or_init(|| panic!("Kabom!")); assert_eq!(c.get(), Some(&92)); - - let mut c = OnceCell::new(); - assert!(c.get().is_none()); - let mut_ref = c.get_mut_or_init(|| 92); - *mut_ref += 1; - assert_eq!(c.get(), Some(&93)); } #[test] @@ -363,20 +357,6 @@ mod sync { assert_eq!(cell.get(), Some(&"hello".to_string())); } - #[test] - fn get_mut_or_try_init() { - let mut cell: OnceCell = OnceCell::new(); - assert!(cell.get().is_none()); - - assert_eq!(cell.get_mut_or_try_init(|| Err(())), Err(())); - - assert_eq!( - cell.get_mut_or_try_init(|| Ok::<_, ()>("hello".to_string())), - Ok(&mut "hello".to_string()) - ); - assert_eq!(cell.get(), Some(&"hello".to_string())); - } - #[test] fn wait() { let cell: OnceCell = OnceCell::new(); From a9fb2d33ff5be954379c09a82ce0cd9821ed08f8 Mon Sep 17 00:00:00 2001 From: Daniel Sanchez Quiros Date: Fri, 2 Sep 2022 07:22:55 +0200 Subject: [PATCH 19/23] Remove changelog oncecell notes --- CHANGELOG.md | 3 --- 1 file changed, 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index aa8df1d..c8dc27a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,9 +6,6 @@ ## 1.14.0 -- Add extension to `async` and `sync` `OnceCell` mut API: - - `get_mut_or_init` - - `get_mut_or_try_init` - Add extension to `async` and `sync` `Lazy` mut API: - `force_mut` - `get_mut` From d3e28a5da71ec11d6294cf1c52a393d80c4cfc0a Mon Sep 17 00:00:00 2001 From: Daniel Sanchez Quiros Date: Fri, 2 Sep 2022 07:25:34 +0200 Subject: [PATCH 20/23] Formatting tests missing line --- tests/it.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/it.rs b/tests/it.rs index b501304..bf745e4 100644 --- a/tests/it.rs +++ b/tests/it.rs @@ -156,6 +156,7 @@ mod unsync { assert_eq!(*x, 46); assert_eq!(called.get(), 1); } + #[test] fn lazy_get_mut() { let called = Cell::new(0); From cca53a4887318613b969ce80859b925a45f05d15 Mon Sep 17 00:00:00 2001 From: Daniel Sanchez Quiros Date: Fri, 2 Sep 2022 07:36:24 +0200 Subject: [PATCH 21/23] Use force_mut return mut ref in test --- tests/it.rs | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/tests/it.rs b/tests/it.rs index bf745e4..85c17a2 100644 --- a/tests/it.rs +++ b/tests/it.rs @@ -145,14 +145,10 @@ mod unsync { 92 }); assert_eq!(called.get(), 0); - Lazy::force_mut(&mut x); + let mut v = Lazy::force_mut(&mut x); assert_eq!(called.get(), 1); - let y = *x - 30; - assert_eq!(y, 62); - assert_eq!(called.get(), 1); - - *x /= 2; + *v /= 2; assert_eq!(*x, 46); assert_eq!(called.get(), 1); } From 02f8b9f8473f812de0585acf41e216ad4ac6dc49 Mon Sep 17 00:00:00 2001 From: Daniel Sanchez Quiros Date: Fri, 2 Sep 2022 07:41:19 +0200 Subject: [PATCH 22/23] Fix unnecesary mut --- tests/it.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/it.rs b/tests/it.rs index 85c17a2..410b93b 100644 --- a/tests/it.rs +++ b/tests/it.rs @@ -145,7 +145,7 @@ mod unsync { 92 }); assert_eq!(called.get(), 0); - let mut v = Lazy::force_mut(&mut x); + let v = Lazy::force_mut(&mut x); assert_eq!(called.get(), 1); *v /= 2; From ceff838c1a9ad23216e743dde997ddabeaea775a Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Fri, 2 Sep 2022 12:48:03 +0100 Subject: [PATCH 23/23] Update CHANGELOG.md --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c8dc27a..f0795e1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,7 +6,7 @@ ## 1.14.0 -- Add extension to `async` and `sync` `Lazy` mut API: +- Add extension to `unsync` and `sync` `Lazy` mut API: - `force_mut` - `get_mut`