From 52ce11996bb437f46ebe046940ab53d1e5781d4b Mon Sep 17 00:00:00 2001 From: Mara Bos Date: Tue, 19 Oct 2021 14:37:27 +0200 Subject: [PATCH 1/7] Implement RFC 3184 - thread local cell methods. --- library/std/src/lib.rs | 1 + library/std/src/thread/local.rs | 357 +++++++++++++++++++++++++++++++- 2 files changed, 348 insertions(+), 10 deletions(-) diff --git a/library/std/src/lib.rs b/library/std/src/lib.rs index 9fa9bf4371392..36e6032b5e4e5 100644 --- a/library/std/src/lib.rs +++ b/library/std/src/lib.rs @@ -249,6 +249,7 @@ #![feature(const_ip)] #![feature(const_ipv4)] #![feature(const_ipv6)] +#![feature(const_mut_refs)] #![feature(const_option)] #![feature(const_socketaddr)] #![feature(const_trait_impl)] diff --git a/library/std/src/thread/local.rs b/library/std/src/thread/local.rs index 2464a0e2e473c..03a402a5a47c8 100644 --- a/library/std/src/thread/local.rs +++ b/library/std/src/thread/local.rs @@ -8,6 +8,7 @@ mod tests; #[cfg(test)] mod dynamic_tests; +use crate::cell::{Cell, RefCell}; use crate::error::Error; use crate::fmt; @@ -108,7 +109,7 @@ pub struct LocalKey { // trivially devirtualizable by LLVM because the value of `inner` never // changes and the constant should be readonly within a crate. This mainly // only runs into problems when TLS statics are exported across crates. - inner: unsafe fn() -> Option<&'static T>, + inner: unsafe fn(Option<&mut Option>) -> Option<&'static T>, } #[stable(feature = "std_debug", since = "1.16.0")] @@ -178,7 +179,9 @@ macro_rules! __thread_local_inner { // used to generate the `LocalKey` value for const-initialized thread locals (@key $t:ty, const $init:expr) => {{ #[cfg_attr(not(windows), inline(always))] // see comments below - unsafe fn __getit() -> $crate::option::Option<&'static $t> { + unsafe fn __getit( + _init: $crate::option::Option<&mut $crate::option::Option<$t>>, + ) -> $crate::option::Option<&'static $t> { const INIT_EXPR: $t = $init; // wasm without atomics maps directly to `static mut`, and dtors @@ -260,7 +263,16 @@ macro_rules! __thread_local_inner { static __KEY: $crate::thread::__OsLocalKeyInner<$t> = $crate::thread::__OsLocalKeyInner::new(); #[allow(unused_unsafe)] - unsafe { __KEY.get(__init) } + unsafe { + __KEY.get(move || { + if let $crate::option::Option::Some(init) = _init { + if let $crate::option::Option::Some(value) = init.take() { + return value; + } + } + __init() + }) + } } } @@ -298,7 +310,9 @@ macro_rules! __thread_local_inner { // // The issue of "should enable on Windows sometimes" is #84933 #[cfg_attr(not(windows), inline(always))] - unsafe fn __getit() -> $crate::option::Option<&'static $t> { + unsafe fn __getit( + init: $crate::option::Option<&mut $crate::option::Option<$t>>, + ) -> $crate::option::Option<&'static $t> { #[cfg(all(target_family = "wasm", not(target_feature = "atomics")))] static __KEY: $crate::thread::__StaticLocalKeyInner<$t> = $crate::thread::__StaticLocalKeyInner::new(); @@ -322,7 +336,16 @@ macro_rules! __thread_local_inner { // raise warning for missing/extraneous unsafe blocks anymore. // See https://github.com/rust-lang/rust/issues/74838. #[allow(unused_unsafe)] - unsafe { __KEY.get(__init) } + unsafe { + __KEY.get(move || { + if let $crate::option::Option::Some(init) = init { + if let $crate::option::Option::Some(value) = init.take() { + return value; + } + } + __init() + }) + } } unsafe { @@ -367,7 +390,9 @@ impl LocalKey { issue = "none" )] #[rustc_const_unstable(feature = "thread_local_internals", issue = "none")] - pub const unsafe fn new(inner: unsafe fn() -> Option<&'static T>) -> LocalKey { + pub const unsafe fn new( + inner: unsafe fn(Option<&mut Option>) -> Option<&'static T>, + ) -> LocalKey { LocalKey { inner } } @@ -409,10 +434,322 @@ impl LocalKey { F: FnOnce(&T) -> R, { unsafe { - let thread_local = (self.inner)().ok_or(AccessError)?; + let thread_local = (self.inner)(None).ok_or(AccessError)?; Ok(f(thread_local)) } } + + 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( + "cannot access a Thread Local Storage value \ + during or after destruction", + ); + f(init, reference) + } + } +} + +impl LocalKey> { + /// Sets or initializes the contained value. + /// + /// Unlike the other methods, this will *not* run the lazy initializer of + /// the thread local. Instead, it will be directly initialized with the + /// given value if it wasn't initialized yet. + /// + /// # Panics + /// + /// Panics if the key currently has its destructor running, + /// and it **may** panic if the destructor has previously been run for this thread. + /// + /// # Examples + /// + /// ``` + /// #![feature(local_key_cell_methods)] + /// use std::cell::Cell; + /// + /// thread_local! { + /// static X: Cell = panic!("!"); + /// } + /// + /// // Calling X.get() here would result in a panic. + /// + /// X.set(123); // But X.set() is fine, as it skips the initializer above. + /// + /// assert_eq!(X.get(), 123); + /// ``` + #[unstable(feature = "local_key_cell_methods", issue = "none")] + pub fn set(&'static self, value: T) { + self.initialize_with(Cell::new(value), |init, cell| { + if let Some(init) = init { + cell.set(init.into_inner()); + } + }); + } + + /// Returns a copy of the contained value. + /// + /// This will lazily initialize the value if this thread has not referenced + /// this key yet. + /// + /// # Panics + /// + /// Panics if the key currently has its destructor running, + /// and it **may** panic if the destructor has previously been run for this thread. + /// + /// # Examples + /// + /// ``` + /// #![feature(local_key_cell_methods)] + /// use std::cell::Cell; + /// + /// thread_local! { + /// static X: Cell = Cell::new(1); + /// } + /// + /// assert_eq!(X.get(), 1); + /// ``` + #[unstable(feature = "local_key_cell_methods", issue = "none")] + pub fn get(&'static self) -> T + where + T: Copy, + { + self.with(|cell| cell.get()) + } + + /// Takes the contained value, leaving `Default::default()` in its place. + /// + /// This will lazily initialize the value if this thread has not referenced + /// this key yet. + /// + /// # Panics + /// + /// Panics if the key currently has its destructor running, + /// and it **may** panic if the destructor has previously been run for this thread. + /// + /// # Examples + /// + /// ``` + /// #![feature(local_key_cell_methods)] + /// use std::cell::Cell; + /// + /// thread_local! { + /// static X: Cell> = Cell::new(Some(1)); + /// } + /// + /// assert_eq!(X.take(), Some(1)); + /// assert_eq!(X.take(), None); + /// ``` + #[unstable(feature = "local_key_cell_methods", issue = "none")] + pub fn take(&'static self) -> T + where + T: Default, + { + self.with(|cell| cell.take()) + } + + /// Replaces the contained value, returning the old value. + /// + /// This will lazily initialize the value if this thread has not referenced + /// this key yet. + /// + /// # Panics + /// + /// Panics if the key currently has its destructor running, + /// and it **may** panic if the destructor has previously been run for this thread. + /// + /// # Examples + /// + /// ``` + /// #![feature(local_key_cell_methods)] + /// use std::cell::Cell; + /// + /// thread_local! { + /// static X: Cell = Cell::new(1); + /// } + /// + /// assert_eq!(X.replace(2), 1); + /// assert_eq!(X.replace(3), 2); + /// ``` + #[unstable(feature = "local_key_cell_methods", issue = "none")] + pub fn replace(&'static self, value: T) -> T { + self.with(|cell| cell.replace(value)) + } +} + +impl LocalKey> { + /// Acquires a reference to the contained value. + /// + /// This will lazily initialize the value if this thread has not referenced + /// this key yet. + /// + /// # Panics + /// + /// Panics if the value is currently borrowed. + /// + /// Panics if the key currently has its destructor running, + /// and it **may** panic if the destructor has previously been run for this thread. + /// + /// # Example + /// + /// ``` + /// #![feature(local_key_cell_methods)] + /// use std::cell::RefCell; + /// + /// thread_local! { + /// static X: RefCell> = RefCell::new(Vec::new()); + /// } + /// + /// X.with_ref(|v| assert!(v.is_empty())); + /// ``` + #[unstable(feature = "local_key_cell_methods", issue = "none")] + pub fn with_ref(&'static self, f: F) -> R + where + F: FnOnce(&T) -> R, + { + self.with(|cell| f(&mut cell.borrow())) + } + + /// Acquires a mutable reference to the contained value. + /// + /// This will lazily initialize the value if this thread has not referenced + /// this key yet. + /// + /// # Panics + /// + /// Panics if the value is currently borrowed. + /// + /// Panics if the key currently has its destructor running, + /// and it **may** panic if the destructor has previously been run for this thread. + /// + /// # Example + /// + /// ``` + /// #![feature(local_key_cell_methods)] + /// use std::cell::RefCell; + /// + /// thread_local! { + /// static X: RefCell> = RefCell::new(Vec::new()); + /// } + /// + /// X.with_mut(|v| v.push(1)); + /// + /// X.with_ref(|v| assert_eq!(*v, vec![1])); + /// ``` + #[unstable(feature = "local_key_cell_methods", issue = "none")] + pub fn with_mut(&'static self, f: F) -> R + where + F: FnOnce(&mut T) -> R, + { + self.with(|cell| f(&mut cell.borrow_mut())) + } + + /// Sets or initializes the contained value. + /// + /// Unlike the other methods, this will *not* run the lazy initializer of + /// the thread local. Instead, it will be directly initialized with the + /// given value if it wasn't initialized yet. + /// + /// # Panics + /// + /// Panics if the key currently has its destructor running, + /// and it **may** panic if the destructor has previously been run for this thread. + /// + /// # Examples + /// + /// ``` + /// #![feature(local_key_cell_methods)] + /// use std::cell::RefCell; + /// + /// thread_local! { + /// static X: RefCell> = panic!("!"); + /// } + /// + /// // Calling X.with() here would result in a panic. + /// + /// X.set(vec![1, 2, 3]); // But X.set() is fine, as it skips the initializer above. + /// + /// X.with_ref(|v| assert_eq!(*v, vec![1, 2, 3])); + /// ``` + #[unstable(feature = "local_key_cell_methods", issue = "none")] + pub fn set(&'static self, value: T) { + self.initialize_with(RefCell::new(value), |init, cell| { + if let Some(init) = init { + cell.replace(init.into_inner()); + } + }); + } + + /// Takes the contained value, leaving `Default::default()` in its place. + /// + /// This will lazily initialize the value if this thread has not referenced + /// this key yet. + /// + /// # Panics + /// + /// Panics if the value is currently borrowed. + /// + /// Panics if the key currently has its destructor running, + /// and it **may** panic if the destructor has previously been run for this thread. + /// + /// # Examples + /// + /// ``` + /// #![feature(local_key_cell_methods)] + /// use std::cell::RefCell; + /// + /// thread_local! { + /// static X: RefCell> = RefCell::new(Vec::new()); + /// } + /// + /// X.with_mut(|v| v.push(1)); + /// + /// let a = X.take(); + /// + /// assert_eq!(a, vec![1]); + /// + /// X.with_ref(|v| assert!(v.is_empty())); + /// ``` + #[unstable(feature = "local_key_cell_methods", issue = "none")] + pub fn take(&'static self) -> T + where + T: Default, + { + self.with(|cell| cell.take()) + } + + /// Replaces the contained value, returning the old value. + /// + /// # Panics + /// + /// Panics if the value is currently borrowed. + /// + /// Panics if the key currently has its destructor running, + /// and it **may** panic if the destructor has previously been run for this thread. + /// + /// # Examples + /// + /// ``` + /// #![feature(local_key_cell_methods)] + /// use std::cell::RefCell; + /// + /// thread_local! { + /// static X: RefCell> = RefCell::new(Vec::new()); + /// } + /// + /// let prev = X.replace(vec![1, 2, 3]); + /// assert!(prev.is_empty()); + /// + /// X.with_ref(|v| assert_eq!(*v, vec![1, 2, 3])); + /// ``` + #[unstable(feature = "local_key_cell_methods", issue = "none")] + pub fn replace(&'static self, value: T) -> T { + self.with(|cell| cell.replace(value)) + } } mod lazy { @@ -518,7 +855,7 @@ pub mod statik { Key { inner: LazyKeyInner::new() } } - pub unsafe fn get(&self, init: fn() -> T) -> Option<&'static T> { + pub unsafe fn get(&self, init: impl FnOnce() -> T) -> Option<&'static T> { // SAFETY: The caller must ensure no reference is ever handed out to // the inner cell nor mutable reference to the Option inside said // cell. This make it safe to hand a reference, though the lifetime @@ -707,7 +1044,7 @@ pub mod os { /// It is a requirement for the caller to ensure that no mutable /// reference is active when this method is called. - pub unsafe fn get(&'static self, init: fn() -> T) -> Option<&'static T> { + pub unsafe fn get(&'static self, init: impl FnOnce() -> T) -> Option<&'static T> { // SAFETY: See the documentation for this method. let ptr = unsafe { self.os.get() as *mut Value }; if ptr as usize > 1 { @@ -725,7 +1062,7 @@ pub mod os { // `try_initialize` is only called once per os thread local variable, // except in corner cases where thread_local dtors reference other // thread_local's, or it is being recursively initialized. - unsafe fn try_initialize(&'static self, init: fn() -> T) -> Option<&'static T> { + unsafe fn try_initialize(&'static self, init: impl FnOnce() -> T) -> Option<&'static T> { // SAFETY: No mutable references are ever handed out meaning getting // the value is ok. let ptr = unsafe { self.os.get() as *mut Value }; From 88a693c4f44b3e267fec108a5572f1e534d4e795 Mon Sep 17 00:00:00 2001 From: Mara Bos Date: Mon, 20 Dec 2021 13:38:07 +0100 Subject: [PATCH 2/7] Rename LocalKey's with_{ref,mut} to with_borrow{,_mut}. --- library/std/src/thread/local.rs | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/library/std/src/thread/local.rs b/library/std/src/thread/local.rs index 03a402a5a47c8..b73c9270929e9 100644 --- a/library/std/src/thread/local.rs +++ b/library/std/src/thread/local.rs @@ -604,10 +604,10 @@ impl LocalKey> { /// static X: RefCell> = RefCell::new(Vec::new()); /// } /// - /// X.with_ref(|v| assert!(v.is_empty())); + /// X.with_borrow(|v| assert!(v.is_empty())); /// ``` #[unstable(feature = "local_key_cell_methods", issue = "none")] - pub fn with_ref(&'static self, f: F) -> R + pub fn with_borrow(&'static self, f: F) -> R where F: FnOnce(&T) -> R, { @@ -636,12 +636,12 @@ impl LocalKey> { /// static X: RefCell> = RefCell::new(Vec::new()); /// } /// - /// X.with_mut(|v| v.push(1)); + /// X.with_borrow_mut(|v| v.push(1)); /// - /// X.with_ref(|v| assert_eq!(*v, vec![1])); + /// X.with_borrow(|v| assert_eq!(*v, vec![1])); /// ``` #[unstable(feature = "local_key_cell_methods", issue = "none")] - pub fn with_mut(&'static self, f: F) -> R + pub fn with_borrow_mut(&'static self, f: F) -> R where F: FnOnce(&mut T) -> R, { @@ -673,7 +673,7 @@ impl LocalKey> { /// /// X.set(vec![1, 2, 3]); // But X.set() is fine, as it skips the initializer above. /// - /// X.with_ref(|v| assert_eq!(*v, vec![1, 2, 3])); + /// X.with_borrow(|v| assert_eq!(*v, vec![1, 2, 3])); /// ``` #[unstable(feature = "local_key_cell_methods", issue = "none")] pub fn set(&'static self, value: T) { @@ -706,13 +706,13 @@ impl LocalKey> { /// static X: RefCell> = RefCell::new(Vec::new()); /// } /// - /// X.with_mut(|v| v.push(1)); + /// X.with_borrow_mut(|v| v.push(1)); /// /// let a = X.take(); /// /// assert_eq!(a, vec![1]); /// - /// X.with_ref(|v| assert!(v.is_empty())); + /// X.with_borrow(|v| assert!(v.is_empty())); /// ``` #[unstable(feature = "local_key_cell_methods", issue = "none")] pub fn take(&'static self) -> T @@ -744,7 +744,7 @@ impl LocalKey> { /// let prev = X.replace(vec![1, 2, 3]); /// assert!(prev.is_empty()); /// - /// X.with_ref(|v| assert_eq!(*v, vec![1, 2, 3])); + /// X.with_borrow(|v| assert_eq!(*v, vec![1, 2, 3])); /// ``` #[unstable(feature = "local_key_cell_methods", issue = "none")] pub fn replace(&'static self, value: T) -> T { From 93c409d6e2924cdc936f3f834fc257f913b5911c Mon Sep 17 00:00:00 2001 From: Mara Bos Date: Mon, 20 Dec 2021 13:49:10 +0100 Subject: [PATCH 3/7] Add tracking issue number for local_key_cell_methods. --- library/std/src/thread/local.rs | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/library/std/src/thread/local.rs b/library/std/src/thread/local.rs index b73c9270929e9..3e94c3d277d59 100644 --- a/library/std/src/thread/local.rs +++ b/library/std/src/thread/local.rs @@ -482,7 +482,7 @@ impl LocalKey> { /// /// assert_eq!(X.get(), 123); /// ``` - #[unstable(feature = "local_key_cell_methods", issue = "none")] + #[unstable(feature = "local_key_cell_methods", issue = "92122")] pub fn set(&'static self, value: T) { self.initialize_with(Cell::new(value), |init, cell| { if let Some(init) = init { @@ -513,7 +513,7 @@ impl LocalKey> { /// /// assert_eq!(X.get(), 1); /// ``` - #[unstable(feature = "local_key_cell_methods", issue = "none")] + #[unstable(feature = "local_key_cell_methods", issue = "92122")] pub fn get(&'static self) -> T where T: Copy, @@ -544,7 +544,7 @@ impl LocalKey> { /// assert_eq!(X.take(), Some(1)); /// assert_eq!(X.take(), None); /// ``` - #[unstable(feature = "local_key_cell_methods", issue = "none")] + #[unstable(feature = "local_key_cell_methods", issue = "92122")] pub fn take(&'static self) -> T where T: Default, @@ -575,7 +575,7 @@ impl LocalKey> { /// assert_eq!(X.replace(2), 1); /// assert_eq!(X.replace(3), 2); /// ``` - #[unstable(feature = "local_key_cell_methods", issue = "none")] + #[unstable(feature = "local_key_cell_methods", issue = "92122")] pub fn replace(&'static self, value: T) -> T { self.with(|cell| cell.replace(value)) } @@ -606,7 +606,7 @@ impl LocalKey> { /// /// X.with_borrow(|v| assert!(v.is_empty())); /// ``` - #[unstable(feature = "local_key_cell_methods", issue = "none")] + #[unstable(feature = "local_key_cell_methods", issue = "92122")] pub fn with_borrow(&'static self, f: F) -> R where F: FnOnce(&T) -> R, @@ -640,7 +640,7 @@ impl LocalKey> { /// /// X.with_borrow(|v| assert_eq!(*v, vec![1])); /// ``` - #[unstable(feature = "local_key_cell_methods", issue = "none")] + #[unstable(feature = "local_key_cell_methods", issue = "92122")] pub fn with_borrow_mut(&'static self, f: F) -> R where F: FnOnce(&mut T) -> R, @@ -675,7 +675,7 @@ impl LocalKey> { /// /// X.with_borrow(|v| assert_eq!(*v, vec![1, 2, 3])); /// ``` - #[unstable(feature = "local_key_cell_methods", issue = "none")] + #[unstable(feature = "local_key_cell_methods", issue = "92122")] pub fn set(&'static self, value: T) { self.initialize_with(RefCell::new(value), |init, cell| { if let Some(init) = init { @@ -714,7 +714,7 @@ impl LocalKey> { /// /// X.with_borrow(|v| assert!(v.is_empty())); /// ``` - #[unstable(feature = "local_key_cell_methods", issue = "none")] + #[unstable(feature = "local_key_cell_methods", issue = "92122")] pub fn take(&'static self) -> T where T: Default, @@ -746,7 +746,7 @@ impl LocalKey> { /// /// X.with_borrow(|v| assert_eq!(*v, vec![1, 2, 3])); /// ``` - #[unstable(feature = "local_key_cell_methods", issue = "none")] + #[unstable(feature = "local_key_cell_methods", issue = "92122")] pub fn replace(&'static self, value: T) -> T { self.with(|cell| cell.replace(value)) } From 36c904594ee0d3a311f720827025c293d071a006 Mon Sep 17 00:00:00 2001 From: Mara Bos Date: Mon, 20 Dec 2021 19:20:56 +0100 Subject: [PATCH 4/7] Add debug asserts in thread local cell set methods. --- library/std/src/thread/local.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/library/std/src/thread/local.rs b/library/std/src/thread/local.rs index 3e94c3d277d59..f36a06efbbd92 100644 --- a/library/std/src/thread/local.rs +++ b/library/std/src/thread/local.rs @@ -268,6 +268,8 @@ macro_rules! __thread_local_inner { if let $crate::option::Option::Some(init) = _init { if let $crate::option::Option::Some(value) = init.take() { return value; + } else if $crate::cfg!(debug_assertions) { + unreachable!("missing initial value"); } } __init() @@ -341,6 +343,8 @@ macro_rules! __thread_local_inner { if let $crate::option::Option::Some(init) = init { if let $crate::option::Option::Some(value) = init.take() { return value; + } else if $crate::cfg!(debug_assertions) { + unreachable!("missing default value"); } } __init() From c68c384b889943d0f1d08dbd8944e09a15f6929a Mon Sep 17 00:00:00 2001 From: Mara Bos Date: Mon, 20 Dec 2021 19:21:36 +0100 Subject: [PATCH 5/7] Update documentation in thread/local.rs. --- library/std/src/thread/local.rs | 32 ++++++++++++++++++++++++++------ 1 file changed, 26 insertions(+), 6 deletions(-) diff --git a/library/std/src/thread/local.rs b/library/std/src/thread/local.rs index f36a06efbbd92..8063242157367 100644 --- a/library/std/src/thread/local.rs +++ b/library/std/src/thread/local.rs @@ -443,6 +443,18 @@ impl LocalKey { } } + /// Acquires a reference to the value in this TLS key, initializing it with + /// `init` if it wasn't already initialized on this thread. + /// + /// If `init` was used to initialize the thread local variable, `None` is + /// passed as the first argument to `f`. If it was already initialized, + /// `Some(init)` is passed to `f`. + /// + /// # Panics + /// + /// This function will panic if the key currently has its destructor + /// running, and it **may** panic if the destructor has previously been run + /// for this thread. fn initialize_with(&'static self, init: T, f: F) -> R where F: FnOnce(Option, &T) -> R, @@ -488,9 +500,12 @@ impl LocalKey> { /// ``` #[unstable(feature = "local_key_cell_methods", issue = "92122")] pub fn set(&'static self, value: T) { - self.initialize_with(Cell::new(value), |init, cell| { - if let Some(init) = init { - cell.set(init.into_inner()); + self.initialize_with(Cell::new(value), |value, cell| { + if let Some(value) = value { + // The cell was already initialized, so `value` wasn't used to + // initialize it. So we overwrite the current value with the + // new one instead. + cell.set(value.into_inner()); } }); } @@ -593,7 +608,7 @@ impl LocalKey> { /// /// # Panics /// - /// Panics if the value is currently borrowed. + /// Panics if the value is currently mutably borrowed. /// /// Panics if the key currently has its destructor running, /// and it **may** panic if the destructor has previously been run for this thread. @@ -660,6 +675,8 @@ impl LocalKey> { /// /// # Panics /// + /// Panics if the value is currently borrowed. + /// /// Panics if the key currently has its destructor running, /// and it **may** panic if the destructor has previously been run for this thread. /// @@ -681,8 +698,11 @@ impl LocalKey> { /// ``` #[unstable(feature = "local_key_cell_methods", issue = "92122")] pub fn set(&'static self, value: T) { - self.initialize_with(RefCell::new(value), |init, cell| { - if let Some(init) = init { + self.initialize_with(RefCell::new(value), |value, cell| { + if let Some(value) = value { + // The cell was already initialized, so `value` wasn't used to + // initialize it. So we overwrite the current value with the + // new one instead. cell.replace(init.into_inner()); } }); From 3b9e214c406fd1a899d4324ad20ffb0de6823abb Mon Sep 17 00:00:00 2001 From: Mara Bos Date: Mon, 20 Dec 2021 19:21:53 +0100 Subject: [PATCH 6/7] Small fixes in thread local code. --- library/std/src/thread/local.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/library/std/src/thread/local.rs b/library/std/src/thread/local.rs index 8063242157367..a100444f04968 100644 --- a/library/std/src/thread/local.rs +++ b/library/std/src/thread/local.rs @@ -630,7 +630,7 @@ impl LocalKey> { where F: FnOnce(&T) -> R, { - self.with(|cell| f(&mut cell.borrow())) + self.with(|cell| f(&cell.borrow())) } /// Acquires a mutable reference to the contained value. @@ -703,7 +703,7 @@ impl LocalKey> { // The cell was already initialized, so `value` wasn't used to // initialize it. So we overwrite the current value with the // new one instead. - cell.replace(init.into_inner()); + *cell.borrow_mut() = value.into_inner(); } }); } From a6e7f26f5ae323b1b7180913fb54f7c7510b7f29 Mon Sep 17 00:00:00 2001 From: Mara Bos Date: Mon, 20 Dec 2021 19:24:40 +0100 Subject: [PATCH 7/7] Update tests. --- .../suggestions/missing-lifetime-specifier.rs | 2 + .../missing-lifetime-specifier.stderr | 169 ++++++++++++------ .../threads-sendsync/issue-43733.mir.stderr | 4 +- src/test/ui/threads-sendsync/issue-43733.rs | 4 +- .../threads-sendsync/issue-43733.thir.stderr | 4 +- 5 files changed, 120 insertions(+), 63 deletions(-) diff --git a/src/test/ui/suggestions/missing-lifetime-specifier.rs b/src/test/ui/suggestions/missing-lifetime-specifier.rs index 4aaac2d95d447..ce847c86bedde 100644 --- a/src/test/ui/suggestions/missing-lifetime-specifier.rs +++ b/src/test/ui/suggestions/missing-lifetime-specifier.rs @@ -45,6 +45,7 @@ thread_local! { //~| ERROR this union takes 2 lifetime arguments but 1 lifetime argument was supplied //~| ERROR this union takes 2 lifetime arguments but 1 lifetime argument was supplied //~| ERROR this union takes 2 lifetime arguments but 1 lifetime argument was supplied + //~| ERROR this union takes 2 lifetime arguments but 1 lifetime argument was supplied } thread_local! { static f: RefCell>>>> = RefCell::new(HashMap::new()); @@ -52,6 +53,7 @@ thread_local! { //~| ERROR this trait takes 2 lifetime arguments but 1 lifetime argument was supplied //~| ERROR this trait takes 2 lifetime arguments but 1 lifetime argument was supplied //~| ERROR this trait takes 2 lifetime arguments but 1 lifetime argument was supplied + //~| ERROR this trait takes 2 lifetime arguments but 1 lifetime argument was supplied //~| ERROR missing lifetime //~| ERROR missing lifetime } diff --git a/src/test/ui/suggestions/missing-lifetime-specifier.stderr b/src/test/ui/suggestions/missing-lifetime-specifier.stderr index bf546384124c0..b04ea1c9158df 100644 --- a/src/test/ui/suggestions/missing-lifetime-specifier.stderr +++ b/src/test/ui/suggestions/missing-lifetime-specifier.stderr @@ -13,14 +13,15 @@ LL | static a: RefCell>>>> = RefC error[E0106]: missing lifetime specifiers --> $DIR/missing-lifetime-specifier.rs:18:44 | -LL | static a: RefCell>>> = RefCell::new(HashMap::new()); - | ^^^ expected 2 lifetime parameters - | - = help: this function's return type contains a borrowed value, but there is no value for it to be borrowed from -help: consider using the `'static` lifetime - | -LL | static a: RefCell>>>> = RefCell::new(HashMap::new()); - | ~~~~~~~~~~~~~~~~~~~~~ +LL | / thread_local! { +LL | | static a: RefCell>>> = RefCell::new(HashMap::new()); + | | ^^^ expected 2 lifetime parameters +LL | | +LL | | +LL | | } + | |_- + | + = help: this function's return type contains a borrowed value, but the signature does not say which one of `init`'s 3 lifetimes it is borrowed from error[E0106]: missing lifetime specifier --> $DIR/missing-lifetime-specifier.rs:23:44 @@ -49,26 +50,32 @@ LL | static b: RefCell>>>> = Ref error[E0106]: missing lifetime specifier --> $DIR/missing-lifetime-specifier.rs:23:44 | -LL | static b: RefCell>>> = RefCell::new(HashMap::new()); - | ^ expected named lifetime parameter - | - = help: this function's return type contains a borrowed value, but there is no value for it to be borrowed from -help: consider using the `'static` lifetime - | -LL | static b: RefCell>>> = RefCell::new(HashMap::new()); - | ~~~~~~~~ +LL | / thread_local! { +LL | | static b: RefCell>>> = RefCell::new(HashMap::new()); + | | ^ expected named lifetime parameter +LL | | +LL | | +LL | | +LL | | +LL | | } + | |_- + | + = help: this function's return type contains a borrowed value, but the signature does not say which one of `init`'s 4 lifetimes it is borrowed from error[E0106]: missing lifetime specifiers --> $DIR/missing-lifetime-specifier.rs:23:45 | -LL | static b: RefCell>>> = RefCell::new(HashMap::new()); - | ^^^ expected 2 lifetime parameters - | - = help: this function's return type contains a borrowed value, but there is no value for it to be borrowed from -help: consider using the `'static` lifetime - | -LL | static b: RefCell>>>> = RefCell::new(HashMap::new()); - | ~~~~~~~~~~~~~~~~~~~~~ +LL | / thread_local! { +LL | | static b: RefCell>>> = RefCell::new(HashMap::new()); + | | ^^^ expected 2 lifetime parameters +LL | | +LL | | +LL | | +LL | | +LL | | } + | |_- + | + = help: this function's return type contains a borrowed value, but the signature does not say which one of `init`'s 4 lifetimes it is borrowed from error[E0106]: missing lifetime specifiers --> $DIR/missing-lifetime-specifier.rs:30:48 @@ -85,14 +92,15 @@ LL | static c: RefCell>>>> = error[E0106]: missing lifetime specifiers --> $DIR/missing-lifetime-specifier.rs:30:48 | -LL | static c: RefCell>>>> = RefCell::new(HashMap::new()); - | ^ expected 2 lifetime parameters - | - = help: this function's return type contains a borrowed value, but there is no value for it to be borrowed from -help: consider using the `'static` lifetime +LL | / thread_local! { +LL | | static c: RefCell>>>> = RefCell::new(HashMap::new()); + | | ^ expected 2 lifetime parameters +LL | | +LL | | +LL | | } + | |_- | -LL | static c: RefCell>>>> = RefCell::new(HashMap::new()); - | +++++++++++++++++ + = help: this function's return type contains a borrowed value, but the signature does not say which one of `init`'s 3 lifetimes it is borrowed from error[E0106]: missing lifetime specifier --> $DIR/missing-lifetime-specifier.rs:35:44 @@ -121,26 +129,50 @@ LL | static d: RefCell>>>> error[E0106]: missing lifetime specifier --> $DIR/missing-lifetime-specifier.rs:35:44 | -LL | static d: RefCell>>>> = RefCell::new(HashMap::new()); - | ^ expected named lifetime parameter - | - = help: this function's return type contains a borrowed value, but there is no value for it to be borrowed from -help: consider using the `'static` lifetime - | -LL | static d: RefCell>>>> = RefCell::new(HashMap::new()); - | ~~~~~~~~ +LL | / thread_local! { +LL | | static d: RefCell>>>> = RefCell::new(HashMap::new()); + | | ^ expected named lifetime parameter +LL | | +LL | | +LL | | +LL | | +LL | | } + | |_- + | + = help: this function's return type contains a borrowed value, but the signature does not say which one of `init`'s 4 lifetimes it is borrowed from error[E0106]: missing lifetime specifiers --> $DIR/missing-lifetime-specifier.rs:35:49 | -LL | static d: RefCell>>>> = RefCell::new(HashMap::new()); - | ^ expected 2 lifetime parameters +LL | / thread_local! { +LL | | static d: RefCell>>>> = RefCell::new(HashMap::new()); + | | ^ expected 2 lifetime parameters +LL | | +LL | | +LL | | +LL | | +LL | | } + | |_- + | + = help: this function's return type contains a borrowed value, but the signature does not say which one of `init`'s 4 lifetimes it is borrowed from + +error[E0107]: this union takes 2 lifetime arguments but 1 lifetime argument was supplied + --> $DIR/missing-lifetime-specifier.rs:43:44 | - = help: this function's return type contains a borrowed value, but there is no value for it to be borrowed from -help: consider using the `'static` lifetime +LL | static e: RefCell>>>> = RefCell::new(HashMap::new()); + | ^^^ ------- supplied 1 lifetime argument + | | + | expected 2 lifetime arguments | -LL | static d: RefCell>>>> = RefCell::new(HashMap::new()); - | +++++++++++++++++ +note: union defined here, with 2 lifetime parameters: `'t`, `'k` + --> $DIR/missing-lifetime-specifier.rs:11:11 + | +LL | pub union Qux<'t, 'k, I> { + | ^^^ -- -- +help: add missing lifetime argument + | +LL | static e: RefCell>>>> = RefCell::new(HashMap::new()); + | ++++ error[E0107]: this union takes 2 lifetime arguments but 1 lifetime argument was supplied --> $DIR/missing-lifetime-specifier.rs:43:44 @@ -157,7 +189,7 @@ LL | pub union Qux<'t, 'k, I> { | ^^^ -- -- help: add missing lifetime argument | -LL | static e: RefCell>>>> = RefCell::new(HashMap::new()); +LL | static e: RefCell>>>> = RefCell::new(HashMap::new()); | ++++ error[E0107]: this union takes 2 lifetime arguments but 1 lifetime argument was supplied @@ -215,7 +247,7 @@ LL | static e: RefCell>>>> = RefC | ++++ error[E0107]: this trait takes 2 lifetime arguments but 1 lifetime argument was supplied - --> $DIR/missing-lifetime-specifier.rs:50:45 + --> $DIR/missing-lifetime-specifier.rs:51:45 | LL | static f: RefCell>>>> = RefCell::new(HashMap::new()); | ^^^ ------- supplied 1 lifetime argument @@ -233,7 +265,7 @@ LL | static f: RefCell>>>> = Ref | ++++ error[E0106]: missing lifetime specifier - --> $DIR/missing-lifetime-specifier.rs:50:44 + --> $DIR/missing-lifetime-specifier.rs:51:44 | LL | static f: RefCell>>>> = RefCell::new(HashMap::new()); | ^ expected named lifetime parameter @@ -245,7 +277,7 @@ LL | static f: RefCell>>>> = | ~~~~~~~~ error[E0107]: this trait takes 2 lifetime arguments but 1 lifetime argument was supplied - --> $DIR/missing-lifetime-specifier.rs:50:45 + --> $DIR/missing-lifetime-specifier.rs:51:45 | LL | static f: RefCell>>>> = RefCell::new(HashMap::new()); | ^^^ ------- supplied 1 lifetime argument @@ -263,19 +295,40 @@ LL | static f: RefCell>>>> = Ref | ++++ error[E0106]: missing lifetime specifier - --> $DIR/missing-lifetime-specifier.rs:50:44 + --> $DIR/missing-lifetime-specifier.rs:51:44 + | +LL | / thread_local! { +LL | | static f: RefCell>>>> = RefCell::new(HashMap::new()); + | | ^ expected named lifetime parameter +LL | | +LL | | +... | +LL | | +LL | | } + | |_- + | + = help: this function's return type contains a borrowed value, but the signature does not say which one of `init`'s 3 lifetimes it is borrowed from + +error[E0107]: this trait takes 2 lifetime arguments but 1 lifetime argument was supplied + --> $DIR/missing-lifetime-specifier.rs:51:45 | LL | static f: RefCell>>>> = RefCell::new(HashMap::new()); - | ^ expected named lifetime parameter + | ^^^ ------- supplied 1 lifetime argument + | | + | expected 2 lifetime arguments | - = help: this function's return type contains a borrowed value, but there is no value for it to be borrowed from -help: consider using the `'static` lifetime +note: trait defined here, with 2 lifetime parameters: `'t`, `'k` + --> $DIR/missing-lifetime-specifier.rs:15:7 | -LL | static f: RefCell>>>> = RefCell::new(HashMap::new()); - | ~~~~~~~~ +LL | trait Tar<'t, 'k, I> {} + | ^^^ -- -- +help: add missing lifetime argument + | +LL | static f: RefCell>>>> = RefCell::new(HashMap::new()); + | ++++ error[E0107]: this trait takes 2 lifetime arguments but 1 lifetime argument was supplied - --> $DIR/missing-lifetime-specifier.rs:50:45 + --> $DIR/missing-lifetime-specifier.rs:51:45 | LL | static f: RefCell>>>> = RefCell::new(HashMap::new()); | ^^^ ------- supplied 1 lifetime argument @@ -293,7 +346,7 @@ LL | static f: RefCell>>>> = Ref | ++++ error[E0107]: this trait takes 2 lifetime arguments but 1 lifetime argument was supplied - --> $DIR/missing-lifetime-specifier.rs:50:45 + --> $DIR/missing-lifetime-specifier.rs:51:45 | LL | static f: RefCell>>>> = RefCell::new(HashMap::new()); | ^^^ ------- supplied 1 lifetime argument @@ -310,7 +363,7 @@ help: add missing lifetime argument LL | static f: RefCell>>>> = RefCell::new(HashMap::new()); | ++++ -error: aborting due to 22 previous errors +error: aborting due to 24 previous errors Some errors have detailed explanations: E0106, E0107. For more information about an error, try `rustc --explain E0106`. diff --git a/src/test/ui/threads-sendsync/issue-43733.mir.stderr b/src/test/ui/threads-sendsync/issue-43733.mir.stderr index 0f4b5936dd015..8dc0e75f1af22 100644 --- a/src/test/ui/threads-sendsync/issue-43733.mir.stderr +++ b/src/test/ui/threads-sendsync/issue-43733.mir.stderr @@ -1,5 +1,5 @@ error[E0133]: call to unsafe function is unsafe and requires unsafe function or block - --> $DIR/issue-43733.rs:17:5 + --> $DIR/issue-43733.rs:19:5 | LL | __KEY.get(Default::default) | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ call to unsafe function @@ -7,7 +7,7 @@ LL | __KEY.get(Default::default) = note: consult the function's documentation for information on how to avoid undefined behavior error[E0133]: call to unsafe function is unsafe and requires unsafe function or block - --> $DIR/issue-43733.rs:20:42 + --> $DIR/issue-43733.rs:22:42 | LL | static FOO: std::thread::LocalKey = std::thread::LocalKey::new(__getit); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ call to unsafe function diff --git a/src/test/ui/threads-sendsync/issue-43733.rs b/src/test/ui/threads-sendsync/issue-43733.rs index 5434140cd6128..9926ed09bb4a8 100644 --- a/src/test/ui/threads-sendsync/issue-43733.rs +++ b/src/test/ui/threads-sendsync/issue-43733.rs @@ -4,6 +4,8 @@ #![feature(thread_local)] #![feature(cfg_target_thread_local, thread_local_internals)] +use std::cell::RefCell; + type Foo = std::cell::RefCell; #[cfg(target_thread_local)] @@ -13,7 +15,7 @@ static __KEY: std::thread::__FastLocalKeyInner = std::thread::__FastLocalKe #[cfg(not(target_thread_local))] static __KEY: std::thread::__OsLocalKeyInner = std::thread::__OsLocalKeyInner::new(); -fn __getit() -> std::option::Option<&'static Foo> { +fn __getit(_: Option<&mut Option>>) -> std::option::Option<&'static Foo> { __KEY.get(Default::default) //~ ERROR call to unsafe function is unsafe } diff --git a/src/test/ui/threads-sendsync/issue-43733.thir.stderr b/src/test/ui/threads-sendsync/issue-43733.thir.stderr index 0f4b5936dd015..8dc0e75f1af22 100644 --- a/src/test/ui/threads-sendsync/issue-43733.thir.stderr +++ b/src/test/ui/threads-sendsync/issue-43733.thir.stderr @@ -1,5 +1,5 @@ error[E0133]: call to unsafe function is unsafe and requires unsafe function or block - --> $DIR/issue-43733.rs:17:5 + --> $DIR/issue-43733.rs:19:5 | LL | __KEY.get(Default::default) | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ call to unsafe function @@ -7,7 +7,7 @@ LL | __KEY.get(Default::default) = note: consult the function's documentation for information on how to avoid undefined behavior error[E0133]: call to unsafe function is unsafe and requires unsafe function or block - --> $DIR/issue-43733.rs:20:42 + --> $DIR/issue-43733.rs:22:42 | LL | static FOO: std::thread::LocalKey = std::thread::LocalKey::new(__getit); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ call to unsafe function