From 2aec2fe3b8315741432a941f58b087d1311d9a44 Mon Sep 17 00:00:00 2001 From: Trevor Gross Date: Mon, 8 Apr 2024 17:43:24 -0400 Subject: [PATCH 1/3] Update thread local docs with idiomatic cell type use The `thread_local!` examples use `RefCell` for `Copy` types. Update examples to have one `Copy` and one non-`Copy` type using `Cell` and `RefCell`, respectively. --- library/std/src/thread/local.rs | 34 +++++++++++++++++---------------- 1 file changed, 18 insertions(+), 16 deletions(-) diff --git a/library/std/src/thread/local.rs b/library/std/src/thread/local.rs index fbb882e640b..d53abf50eb5 100644 --- a/library/std/src/thread/local.rs +++ b/library/std/src/thread/local.rs @@ -53,25 +53,25 @@ /// # Examples /// /// ``` -/// use std::cell::RefCell; +/// use std::cell::Cell; /// use std::thread; /// -/// thread_local!(static FOO: RefCell = RefCell::new(1)); +/// thread_local!(static FOO: Cell = Cell::new(1)); /// -/// FOO.with_borrow(|v| assert_eq!(*v, 1)); -/// FOO.with_borrow_mut(|v| *v = 2); +/// assert_eq!(FOO.get(), 1); +/// FOO.set(2); /// /// // each thread starts out with the initial value of 1 /// let t = thread::spawn(move|| { -/// FOO.with_borrow(|v| assert_eq!(*v, 1)); -/// FOO.with_borrow_mut(|v| *v = 3); +/// assert_eq!(FOO.get(), 1); +/// FOO.set(3); /// }); /// /// // wait for the thread to complete and bail out on panic /// t.join().unwrap(); /// /// // we retain our original value of 2 despite the child thread -/// FOO.with_borrow(|v| assert_eq!(*v, 2)); +/// assert_eq!(FOO.get(), 2); /// ``` /// /// # Platform-specific behavior @@ -141,15 +141,16 @@ fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { /// Publicity and attributes for each static are allowed. Example: /// /// ``` -/// use std::cell::RefCell; -/// thread_local! { -/// pub static FOO: RefCell = RefCell::new(1); +/// use std::cell::{Cell, RefCell}; /// -/// static BAR: RefCell = RefCell::new(1.0); +/// thread_local! { +/// pub static FOO: Cell = Cell::new(1); +/// +/// static BAR: RefCell> = RefCell::new(vec![1.0, 2.0]); /// } /// -/// FOO.with_borrow(|v| assert_eq!(*v, 1)); -/// BAR.with_borrow(|v| assert_eq!(*v, 1.0)); +/// assert_eq!(FOO.get(), 1); +/// BAR.with_borrow(|v| assert_eq!(v[1], 2.0)); /// ``` /// /// Note that only shared references (`&T`) to the inner data may be obtained, so a @@ -164,12 +165,13 @@ fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { /// track any additional state. /// /// ``` -/// use std::cell::Cell; +/// use std::cell::RefCell; +/// /// thread_local! { -/// pub static FOO: Cell = const { Cell::new(1) }; +/// pub static FOO: RefCell> = const { RefCell::new(Vec::new()) }; /// } /// -/// assert_eq!(FOO.get(), 1); +/// FOO.with_borrow(|v| assert_eq!(v.len(), 0)); /// ``` /// /// See [`LocalKey` documentation][`std::thread::LocalKey`] for more From 6e68a2f475593f15c9bb3fdbe9b53633a5aa0659 Mon Sep 17 00:00:00 2001 From: Trevor Gross Date: Mon, 8 Apr 2024 17:47:09 -0400 Subject: [PATCH 2/3] Add `SAFETY` comments to the thread local implementation Reduce `unsafe` block scope and add `SAFETY` comments. --- library/std/src/thread/local.rs | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/library/std/src/thread/local.rs b/library/std/src/thread/local.rs index d53abf50eb5..a4acd6c3d06 100644 --- a/library/std/src/thread/local.rs +++ b/library/std/src/thread/local.rs @@ -281,10 +281,9 @@ pub fn try_with(&'static self, f: F) -> Result where F: FnOnce(&T) -> R, { - unsafe { - let thread_local = (self.inner)(None).ok_or(AccessError)?; - Ok(f(thread_local)) - } + // SAFETY: `inner` is safe to call within the lifetime of the thread + let thread_local = unsafe { (self.inner)(None).ok_or(AccessError)? }; + Ok(f(thread_local)) } /// Acquires a reference to the value in this TLS key, initializing it with @@ -303,14 +302,17 @@ fn initialize_with(&'static self, init: T, f: F) -> R where F: FnOnce(Option, &T) -> R, { - unsafe { - let mut init = Some(init); - let reference = (self.inner)(Some(&mut init)).expect( + let mut init = Some(init); + + // SAFETY: `inner` is safe to call within the lifetime of the thread + let reference = unsafe { + (self.inner)(Some(&mut init)).expect( "cannot access a Thread Local Storage value \ during or after destruction", - ); - f(init, reference) - } + ) + }; + + f(init, reference) } } From 313085f7258ef86cf190aa6c234a42843995fdbc Mon Sep 17 00:00:00 2001 From: Trevor Gross Date: Mon, 8 Apr 2024 17:48:07 -0400 Subject: [PATCH 3/3] Change method calls to using the method directly This is in accordance with Clippy's redundant_closure_for_method_calls. --- library/std/src/thread/local.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/library/std/src/thread/local.rs b/library/std/src/thread/local.rs index a4acd6c3d06..c1b4440e560 100644 --- a/library/std/src/thread/local.rs +++ b/library/std/src/thread/local.rs @@ -381,7 +381,7 @@ pub fn get(&'static self) -> T where T: Copy, { - self.with(|cell| cell.get()) + self.with(Cell::get) } /// Takes the contained value, leaving `Default::default()` in its place. @@ -411,7 +411,7 @@ pub fn take(&'static self) -> T where T: Default, { - self.with(|cell| cell.take()) + self.with(Cell::take) } /// Replaces the contained value, returning the old value. @@ -582,7 +582,7 @@ pub fn take(&'static self) -> T where T: Default, { - self.with(|cell| cell.take()) + self.with(RefCell::take) } /// Replaces the contained value, returning the old value.