From fba5cfe482ef8fc60bbd102e22f63059ab15d1c7 Mon Sep 17 00:00:00 2001 From: Mara Bos Date: Wed, 26 Apr 2023 21:02:29 +0200 Subject: [PATCH 1/3] Restructure and rename thread local things in std. --- .../src/sys/common/thread_local/fast_local.rs | 255 +++++++++--------- .../std/src/sys/common/thread_local/mod.rs | 33 +-- .../src/sys/common/thread_local/os_local.rs | 211 +++++++-------- .../sys/common/thread_local/static_local.rs | 80 +++--- library/std/src/thread/local.rs | 8 +- library/std/src/thread/mod.rs | 7 +- 6 files changed, 283 insertions(+), 311 deletions(-) diff --git a/library/std/src/sys/common/thread_local/fast_local.rs b/library/std/src/sys/common/thread_local/fast_local.rs index e229eb16aa139..914e017f7ffaf 100644 --- a/library/std/src/sys/common/thread_local/fast_local.rs +++ b/library/std/src/sys/common/thread_local/fast_local.rs @@ -1,13 +1,14 @@ +use super::lazy::LazyKeyInner; +use crate::cell::Cell; +use crate::sys::thread_local_dtor::register_dtor; +use crate::{fmt, mem, panic}; + #[doc(hidden)] -#[macro_export] -#[allow_internal_unstable( - thread_local_internals, - cfg_target_thread_local, - thread_local, - libstd_thread_internals -)] +#[allow_internal_unstable(thread_local_internals, cfg_target_thread_local, thread_local)] #[allow_internal_unsafe] -macro_rules! __thread_local_inner { +#[unstable(feature = "thread_local_internals", issue = "none")] +#[rustc_macro_transparency = "semitransparent"] +pub macro thread_local_inner { // used to generate the `LocalKey` value for const-initialized thread locals (@key $t:ty, const $init:expr) => {{ #[cfg_attr(not(bootstrap), inline)] @@ -49,7 +50,7 @@ macro_rules! __thread_local_inner { // 0 == we haven't registered a destructor, so do // so now. 0 => { - $crate::thread::__LocalKeyInner::<$t>::register_dtor( + $crate::thread::local_impl::Key::<$t>::register_dtor( $crate::ptr::addr_of_mut!(VAL) as *mut $crate::primitive::u8, destroy, ); @@ -69,7 +70,7 @@ macro_rules! __thread_local_inner { unsafe { $crate::thread::LocalKey::new(__getit) } - }}; + }}, // used to generate the `LocalKey` value for `thread_local!` (@key $t:ty, $init:expr) => { @@ -82,8 +83,8 @@ macro_rules! __thread_local_inner { init: $crate::option::Option<&mut $crate::option::Option<$t>>, ) -> $crate::option::Option<&'static $t> { #[thread_local] - static __KEY: $crate::thread::__LocalKeyInner<$t> = - $crate::thread::__LocalKeyInner::<$t>::new(); + static __KEY: $crate::thread::local_impl::Key<$t> = + $crate::thread::local_impl::Key::<$t>::new(); // FIXME: remove the #[allow(...)] marker when macros don't // raise warning for missing/extraneous unsafe blocks anymore. @@ -107,148 +108,140 @@ macro_rules! __thread_local_inner { $crate::thread::LocalKey::new(__getit) } } - }; + }, ($(#[$attr:meta])* $vis:vis $name:ident, $t:ty, $($init:tt)*) => { $(#[$attr])* $vis const $name: $crate::thread::LocalKey<$t> = - $crate::__thread_local_inner!(@key $t, $($init)*); - } + $crate::thread::local_impl::thread_local_inner!(@key $t, $($init)*); + }, } -#[doc(hidden)] -pub mod fast { - use super::super::lazy::LazyKeyInner; - use crate::cell::Cell; - use crate::sys::thread_local_dtor::register_dtor; - use crate::{fmt, mem, panic}; - - #[derive(Copy, Clone)] - enum DtorState { - Unregistered, - Registered, - RunningOrHasRun, - } +#[derive(Copy, Clone)] +enum DtorState { + Unregistered, + Registered, + RunningOrHasRun, +} - // This data structure has been carefully constructed so that the fast path - // only contains one branch on x86. That optimization is necessary to avoid - // duplicated tls lookups on OSX. +// This data structure has been carefully constructed so that the fast path +// only contains one branch on x86. That optimization is necessary to avoid +// duplicated tls lookups on OSX. +// +// LLVM issue: https://bugs.llvm.org/show_bug.cgi?id=41722 +pub struct Key { + // If `LazyKeyInner::get` returns `None`, that indicates either: + // * The value has never been initialized + // * The value is being recursively initialized + // * The value has already been destroyed or is being destroyed + // To determine which kind of `None`, check `dtor_state`. // - // LLVM issue: https://bugs.llvm.org/show_bug.cgi?id=41722 - pub struct Key { - // If `LazyKeyInner::get` returns `None`, that indicates either: - // * The value has never been initialized - // * The value is being recursively initialized - // * The value has already been destroyed or is being destroyed - // To determine which kind of `None`, check `dtor_state`. - // - // This is very optimizer friendly for the fast path - initialized but - // not yet dropped. - inner: LazyKeyInner, + // This is very optimizer friendly for the fast path - initialized but + // not yet dropped. + inner: LazyKeyInner, - // Metadata to keep track of the state of the destructor. Remember that - // this variable is thread-local, not global. - dtor_state: Cell, - } + // Metadata to keep track of the state of the destructor. Remember that + // this variable is thread-local, not global. + dtor_state: Cell, +} - impl fmt::Debug for Key { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - f.debug_struct("Key").finish_non_exhaustive() - } +impl fmt::Debug for Key { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.debug_struct("Key").finish_non_exhaustive() } +} - impl Key { - pub const fn new() -> Key { - Key { inner: LazyKeyInner::new(), dtor_state: Cell::new(DtorState::Unregistered) } - } +impl Key { + pub const fn new() -> Key { + Key { inner: LazyKeyInner::new(), dtor_state: Cell::new(DtorState::Unregistered) } + } - // note that this is just a publicly-callable function only for the - // const-initialized form of thread locals, basically a way to call the - // free `register_dtor` function defined elsewhere in std. - pub unsafe fn register_dtor(a: *mut u8, dtor: unsafe extern "C" fn(*mut u8)) { - unsafe { - register_dtor(a, dtor); - } + // note that this is just a publicly-callable function only for the + // const-initialized form of thread locals, basically a way to call the + // free `register_dtor` function defined elsewhere in std. + pub unsafe fn register_dtor(a: *mut u8, dtor: unsafe extern "C" fn(*mut u8)) { + unsafe { + register_dtor(a, dtor); } + } - pub unsafe fn get T>(&self, init: F) -> Option<&'static T> { - // SAFETY: See the definitions of `LazyKeyInner::get` and - // `try_initialize` for more information. - // - // The caller must ensure no mutable references are ever active to - // the inner cell or the inner T when this is called. - // The `try_initialize` is dependant on the passed `init` function - // for this. - unsafe { - match self.inner.get() { - Some(val) => Some(val), - None => self.try_initialize(init), - } + pub unsafe fn get T>(&self, init: F) -> Option<&'static T> { + // SAFETY: See the definitions of `LazyKeyInner::get` and + // `try_initialize` for more information. + // + // The caller must ensure no mutable references are ever active to + // the inner cell or the inner T when this is called. + // The `try_initialize` is dependant on the passed `init` function + // for this. + unsafe { + match self.inner.get() { + Some(val) => Some(val), + None => self.try_initialize(init), } } + } - // `try_initialize` is only called once per fast thread local variable, - // except in corner cases where thread_local dtors reference other - // thread_local's, or it is being recursively initialized. - // - // Macos: Inlining this function can cause two `tlv_get_addr` calls to - // be performed for every call to `Key::get`. - // LLVM issue: https://bugs.llvm.org/show_bug.cgi?id=41722 - #[inline(never)] - unsafe fn try_initialize T>(&self, init: F) -> Option<&'static T> { + // `try_initialize` is only called once per fast thread local variable, + // except in corner cases where thread_local dtors reference other + // thread_local's, or it is being recursively initialized. + // + // Macos: Inlining this function can cause two `tlv_get_addr` calls to + // be performed for every call to `Key::get`. + // LLVM issue: https://bugs.llvm.org/show_bug.cgi?id=41722 + #[inline(never)] + unsafe fn try_initialize T>(&self, init: F) -> Option<&'static T> { + // SAFETY: See comment above (this function doc). + if !mem::needs_drop::() || unsafe { self.try_register_dtor() } { // SAFETY: See comment above (this function doc). - if !mem::needs_drop::() || unsafe { self.try_register_dtor() } { - // SAFETY: See comment above (this function doc). - Some(unsafe { self.inner.initialize(init) }) - } else { - None - } + Some(unsafe { self.inner.initialize(init) }) + } else { + None } + } - // `try_register_dtor` is only called once per fast 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_register_dtor(&self) -> bool { - match self.dtor_state.get() { - DtorState::Unregistered => { - // SAFETY: dtor registration happens before initialization. - // Passing `self` as a pointer while using `destroy_value` - // is safe because the function will build a pointer to a - // Key, which is the type of self and so find the correct - // size. - unsafe { register_dtor(self as *const _ as *mut u8, destroy_value::) }; - self.dtor_state.set(DtorState::Registered); - true - } - DtorState::Registered => { - // recursively initialized - true - } - DtorState::RunningOrHasRun => false, + // `try_register_dtor` is only called once per fast 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_register_dtor(&self) -> bool { + match self.dtor_state.get() { + DtorState::Unregistered => { + // SAFETY: dtor registration happens before initialization. + // Passing `self` as a pointer while using `destroy_value` + // is safe because the function will build a pointer to a + // Key, which is the type of self and so find the correct + // size. + unsafe { register_dtor(self as *const _ as *mut u8, destroy_value::) }; + self.dtor_state.set(DtorState::Registered); + true } + DtorState::Registered => { + // recursively initialized + true + } + DtorState::RunningOrHasRun => false, } } +} - unsafe extern "C" fn destroy_value(ptr: *mut u8) { - let ptr = ptr as *mut Key; +unsafe extern "C" fn destroy_value(ptr: *mut u8) { + let ptr = ptr as *mut Key; - // SAFETY: - // - // The pointer `ptr` has been built just above and comes from - // `try_register_dtor` where it is originally a Key coming from `self`, - // making it non-NUL and of the correct type. - // - // Right before we run the user destructor be sure to set the - // `Option` to `None`, and `dtor_state` to `RunningOrHasRun`. This - // causes future calls to `get` to run `try_initialize_drop` again, - // which will now fail, and return `None`. - // - // Wrap the call in a catch to ensure unwinding is caught in the event - // a panic takes place in a destructor. - if let Err(_) = panic::catch_unwind(panic::AssertUnwindSafe(|| unsafe { - let value = (*ptr).inner.take(); - (*ptr).dtor_state.set(DtorState::RunningOrHasRun); - drop(value); - })) { - rtabort!("thread local panicked on drop"); - } + // SAFETY: + // + // The pointer `ptr` has been built just above and comes from + // `try_register_dtor` where it is originally a Key coming from `self`, + // making it non-NUL and of the correct type. + // + // Right before we run the user destructor be sure to set the + // `Option` to `None`, and `dtor_state` to `RunningOrHasRun`. This + // causes future calls to `get` to run `try_initialize_drop` again, + // which will now fail, and return `None`. + // + // Wrap the call in a catch to ensure unwinding is caught in the event + // a panic takes place in a destructor. + if let Err(_) = panic::catch_unwind(panic::AssertUnwindSafe(|| unsafe { + let value = (*ptr).inner.take(); + (*ptr).dtor_state.set(DtorState::RunningOrHasRun); + drop(value); + })) { + rtabort!("thread local panicked on drop"); } } diff --git a/library/std/src/sys/common/thread_local/mod.rs b/library/std/src/sys/common/thread_local/mod.rs index 1fee84a04349c..a7528c06c9de2 100644 --- a/library/std/src/sys/common/thread_local/mod.rs +++ b/library/std/src/sys/common/thread_local/mod.rs @@ -1,35 +1,24 @@ -//! The following module declarations are outside cfg_if because the internal -//! `__thread_local_internal` macro does not seem to be exported properly when using cfg_if #![unstable(feature = "thread_local_internals", reason = "should not be necessary", issue = "none")] -#[cfg(all(target_thread_local, not(all(target_family = "wasm", not(target_feature = "atomics")))))] -mod fast_local; -#[cfg(all( - not(target_thread_local), - not(all(target_family = "wasm", not(target_feature = "atomics"))) -))] -mod os_local; -#[cfg(all(target_family = "wasm", not(target_feature = "atomics")))] -mod static_local; - -#[cfg(not(test))] cfg_if::cfg_if! { if #[cfg(all(target_family = "wasm", not(target_feature = "atomics")))] { #[doc(hidden)] - pub use static_local::statik::Key; - } else if #[cfg(all(target_thread_local, not(all(target_family = "wasm", not(target_feature = "atomics")))))] { + mod static_local; + #[doc(hidden)] + pub use static_local::{Key, thread_local_inner}; + } else if #[cfg(all(target_thread_local))] { + #[doc(hidden)] + mod fast_local; #[doc(hidden)] - pub use fast_local::fast::Key; - } else if #[cfg(all(not(target_thread_local), not(all(target_family = "wasm", not(target_feature = "atomics")))))] { + pub use fast_local::{Key, thread_local_inner}; + } else { #[doc(hidden)] - pub use os_local::os::Key; + mod os_local; + #[doc(hidden)] + pub use os_local::{Key, thread_local_inner}; } } -#[doc(hidden)] -#[cfg(test)] -pub use realstd::thread::__LocalKeyInner as Key; - mod lazy { use crate::cell::UnsafeCell; use crate::hint; diff --git a/library/std/src/sys/common/thread_local/os_local.rs b/library/std/src/sys/common/thread_local/os_local.rs index 1442a397e767e..e9516f9983f2a 100644 --- a/library/std/src/sys/common/thread_local/os_local.rs +++ b/library/std/src/sys/common/thread_local/os_local.rs @@ -1,13 +1,14 @@ +use super::lazy::LazyKeyInner; +use crate::cell::Cell; +use crate::sys_common::thread_local_key::StaticKey as OsStaticKey; +use crate::{fmt, marker, panic, ptr}; + #[doc(hidden)] -#[macro_export] -#[allow_internal_unstable( - thread_local_internals, - cfg_target_thread_local, - thread_local, - libstd_thread_internals -)] +#[allow_internal_unstable(thread_local_internals)] #[allow_internal_unsafe] -macro_rules! __thread_local_inner { +#[unstable(feature = "thread_local_internals", issue = "none")] +#[rustc_macro_transparency = "semitransparent"] +pub macro thread_local_inner { // used to generate the `LocalKey` value for const-initialized thread locals (@key $t:ty, const $init:expr) => {{ #[cfg_attr(not(bootstrap), inline)] @@ -21,8 +22,8 @@ macro_rules! __thread_local_inner { // same implementation as below for os thread locals. #[inline] const fn __init() -> $t { INIT_EXPR } - static __KEY: $crate::thread::__LocalKeyInner<$t> = - $crate::thread::__LocalKeyInner::new(); + static __KEY: $crate::thread::local_impl::Key<$t> = + $crate::thread::local_impl::Key::new(); #[allow(unused_unsafe)] unsafe { __KEY.get(move || { @@ -41,7 +42,7 @@ macro_rules! __thread_local_inner { unsafe { $crate::thread::LocalKey::new(__getit) } - }}; + }}, // used to generate the `LocalKey` value for `thread_local!` (@key $t:ty, $init:expr) => { @@ -55,8 +56,8 @@ macro_rules! __thread_local_inner { unsafe fn __getit( init: $crate::option::Option<&mut $crate::option::Option<$t>>, ) -> $crate::option::Option<&'static $t> { - static __KEY: $crate::thread::__LocalKeyInner<$t> = - $crate::thread::__LocalKeyInner::new(); + static __KEY: $crate::thread::local_impl::Key<$t> = + $crate::thread::local_impl::Key::new(); // FIXME: remove the #[allow(...)] marker when macros don't // raise warning for missing/extraneous unsafe blocks anymore. @@ -80,118 +81,110 @@ macro_rules! __thread_local_inner { $crate::thread::LocalKey::new(__getit) } } - }; + }, ($(#[$attr:meta])* $vis:vis $name:ident, $t:ty, $($init:tt)*) => { $(#[$attr])* $vis const $name: $crate::thread::LocalKey<$t> = - $crate::__thread_local_inner!(@key $t, $($init)*); - } + $crate::thread::local_impl::thread_local_inner!(@key $t, $($init)*); + }, } -#[doc(hidden)] -pub mod os { - use super::super::lazy::LazyKeyInner; - use crate::cell::Cell; - use crate::sys_common::thread_local_key::StaticKey as OsStaticKey; - use crate::{fmt, marker, panic, ptr}; - - /// Use a regular global static to store this key; the state provided will then be - /// thread-local. - pub struct Key { - // OS-TLS key that we'll use to key off. - os: OsStaticKey, - marker: marker::PhantomData>, - } +/// Use a regular global static to store this key; the state provided will then be +/// thread-local. +pub struct Key { + // OS-TLS key that we'll use to key off. + os: OsStaticKey, + marker: marker::PhantomData>, +} - impl fmt::Debug for Key { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - f.debug_struct("Key").finish_non_exhaustive() - } +impl fmt::Debug for Key { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.debug_struct("Key").finish_non_exhaustive() } +} - unsafe impl Sync for Key {} +unsafe impl Sync for Key {} - struct Value { - inner: LazyKeyInner, - key: &'static Key, +struct Value { + inner: LazyKeyInner, + key: &'static Key, +} + +impl Key { + #[rustc_const_unstable(feature = "thread_local_internals", issue = "none")] + pub const fn new() -> Key { + Key { os: OsStaticKey::new(Some(destroy_value::)), marker: marker::PhantomData } } - impl Key { - #[rustc_const_unstable(feature = "thread_local_internals", issue = "none")] - pub const fn new() -> Key { - Key { os: OsStaticKey::new(Some(destroy_value::)), marker: marker::PhantomData } + /// 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: impl FnOnce() -> T) -> Option<&'static T> { + // SAFETY: See the documentation for this method. + let ptr = unsafe { self.os.get() as *mut Value }; + if ptr.addr() > 1 { + // SAFETY: the check ensured the pointer is safe (its destructor + // is not running) + it is coming from a trusted source (self). + if let Some(ref value) = unsafe { (*ptr).inner.get() } { + return Some(value); + } } + // SAFETY: At this point we are sure we have no value and so + // initializing (or trying to) is safe. + unsafe { self.try_initialize(init) } + } - /// 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: impl FnOnce() -> T) -> Option<&'static T> { - // SAFETY: See the documentation for this method. - let ptr = unsafe { self.os.get() as *mut Value }; - if ptr.addr() > 1 { - // SAFETY: the check ensured the pointer is safe (its destructor - // is not running) + it is coming from a trusted source (self). - if let Some(ref value) = unsafe { (*ptr).inner.get() } { - return Some(value); - } - } - // SAFETY: At this point we are sure we have no value and so - // initializing (or trying to) is safe. - unsafe { self.try_initialize(init) } + // `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: 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 }; + if ptr.addr() == 1 { + // destructor is running + return None; } - // `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: 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 }; - if ptr.addr() == 1 { - // destructor is running - return None; + let ptr = if ptr.is_null() { + // If the lookup returned null, we haven't initialized our own + // local copy, so do that now. + let ptr = Box::into_raw(Box::new(Value { inner: LazyKeyInner::new(), key: self })); + // SAFETY: At this point we are sure there is no value inside + // ptr so setting it will not affect anyone else. + unsafe { + self.os.set(ptr as *mut u8); } - - let ptr = if ptr.is_null() { - // If the lookup returned null, we haven't initialized our own - // local copy, so do that now. - let ptr = Box::into_raw(Box::new(Value { inner: LazyKeyInner::new(), key: self })); - // SAFETY: At this point we are sure there is no value inside - // ptr so setting it will not affect anyone else. - unsafe { - self.os.set(ptr as *mut u8); - } - ptr - } else { - // recursive initialization - ptr - }; - - // SAFETY: ptr has been ensured as non-NUL just above an so can be - // dereferenced safely. - unsafe { Some((*ptr).inner.initialize(init)) } - } + ptr + } else { + // recursive initialization + ptr + }; + + // SAFETY: ptr has been ensured as non-NUL just above an so can be + // dereferenced safely. + unsafe { Some((*ptr).inner.initialize(init)) } } +} - unsafe extern "C" fn destroy_value(ptr: *mut u8) { - // SAFETY: - // - // The OS TLS ensures that this key contains a null value when this - // destructor starts to run. We set it back to a sentinel value of 1 to - // ensure that any future calls to `get` for this thread will return - // `None`. - // - // Note that to prevent an infinite loop we reset it back to null right - // before we return from the destructor ourselves. - // - // Wrap the call in a catch to ensure unwinding is caught in the event - // a panic takes place in a destructor. - if let Err(_) = panic::catch_unwind(|| unsafe { - let ptr = Box::from_raw(ptr as *mut Value); - let key = ptr.key; - key.os.set(ptr::invalid_mut(1)); - drop(ptr); - key.os.set(ptr::null_mut()); - }) { - rtabort!("thread local panicked on drop"); - } +unsafe extern "C" fn destroy_value(ptr: *mut u8) { + // SAFETY: + // + // The OS TLS ensures that this key contains a null value when this + // destructor starts to run. We set it back to a sentinel value of 1 to + // ensure that any future calls to `get` for this thread will return + // `None`. + // + // Note that to prevent an infinite loop we reset it back to null right + // before we return from the destructor ourselves. + // + // Wrap the call in a catch to ensure unwinding is caught in the event + // a panic takes place in a destructor. + if let Err(_) = panic::catch_unwind(|| unsafe { + let ptr = Box::from_raw(ptr as *mut Value); + let key = ptr.key; + key.os.set(ptr::invalid_mut(1)); + drop(ptr); + key.os.set(ptr::null_mut()); + }) { + rtabort!("thread local panicked on drop"); } } diff --git a/library/std/src/sys/common/thread_local/static_local.rs b/library/std/src/sys/common/thread_local/static_local.rs index ec4f2a12b7ee4..80322a978646b 100644 --- a/library/std/src/sys/common/thread_local/static_local.rs +++ b/library/std/src/sys/common/thread_local/static_local.rs @@ -1,13 +1,12 @@ +use super::lazy::LazyKeyInner; +use crate::fmt; + #[doc(hidden)] -#[macro_export] -#[allow_internal_unstable( - thread_local_internals, - cfg_target_thread_local, - thread_local, - libstd_thread_internals -)] +#[allow_internal_unstable(thread_local_internals)] #[allow_internal_unsafe] -macro_rules! __thread_local_inner { +#[unstable(feature = "thread_local_internals", issue = "none")] +#[rustc_macro_transparency = "semitransparent"] +pub macro thread_local_inner { // used to generate the `LocalKey` value for const-initialized thread locals (@key $t:ty, const $init:expr) => {{ #[inline] // see comments below @@ -30,7 +29,7 @@ macro_rules! __thread_local_inner { unsafe { $crate::thread::LocalKey::new(__getit) } - }}; + }}, // used to generate the `LocalKey` value for `thread_local!` (@key $t:ty, $init:expr) => { @@ -41,8 +40,8 @@ macro_rules! __thread_local_inner { unsafe fn __getit( init: $crate::option::Option<&mut $crate::option::Option<$t>>, ) -> $crate::option::Option<&'static $t> { - static __KEY: $crate::thread::__LocalKeyInner<$t> = - $crate::thread::__LocalKeyInner::new(); + static __KEY: $crate::thread::local_impl::Key<$t> = + $crate::thread::local_impl::Key::new(); // FIXME: remove the #[allow(...)] marker when macros don't // raise warning for missing/extraneous unsafe blocks anymore. @@ -66,50 +65,45 @@ macro_rules! __thread_local_inner { $crate::thread::LocalKey::new(__getit) } } - }; + }, ($(#[$attr:meta])* $vis:vis $name:ident, $t:ty, $($init:tt)*) => { $(#[$attr])* $vis const $name: $crate::thread::LocalKey<$t> = - $crate::__thread_local_inner!(@key $t, $($init)*); - } + $crate::thread::local_impl::thread_local_inner!(@key $t, $($init)*); + }, } /// On some targets like wasm there's no threads, so no need to generate /// thread locals and we can instead just use plain statics! -#[doc(hidden)] -pub mod statik { - use super::super::lazy::LazyKeyInner; - use crate::fmt; - pub struct Key { - inner: LazyKeyInner, - } +pub struct Key { + inner: LazyKeyInner, +} - unsafe impl Sync for Key {} +unsafe impl Sync for Key {} - impl fmt::Debug for Key { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - f.debug_struct("Key").finish_non_exhaustive() - } +impl fmt::Debug for Key { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.debug_struct("Key").finish_non_exhaustive() } +} - impl Key { - pub const fn new() -> Key { - Key { inner: LazyKeyInner::new() } - } +impl Key { + pub const fn new() -> Key { + Key { inner: LazyKeyInner::new() } + } - 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 - // of 'static is itself unsafe, making the get method unsafe. - let value = unsafe { - match self.inner.get() { - Some(ref value) => value, - None => self.inner.initialize(init), - } - }; + 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 + // of 'static is itself unsafe, making the get method unsafe. + let value = unsafe { + match self.inner.get() { + Some(ref value) => value, + None => self.inner.initialize(init), + } + }; - Some(value) - } + Some(value) } } diff --git a/library/std/src/thread/local.rs b/library/std/src/thread/local.rs index 7fdf03acc14d9..fa08fdc16530b 100644 --- a/library/std/src/thread/local.rs +++ b/library/std/src/thread/local.rs @@ -153,23 +153,23 @@ macro_rules! thread_local { () => {}; ($(#[$attr:meta])* $vis:vis static $name:ident: $t:ty = const { $init:expr }; $($rest:tt)*) => ( - $crate::__thread_local_inner!($(#[$attr])* $vis $name, $t, const $init); + $crate::thread::local_impl::thread_local_inner!($(#[$attr])* $vis $name, $t, const $init); $crate::thread_local!($($rest)*); ); ($(#[$attr:meta])* $vis:vis static $name:ident: $t:ty = const { $init:expr }) => ( - $crate::__thread_local_inner!($(#[$attr])* $vis $name, $t, const $init); + $crate::thread::local_impl::thread_local_inner!($(#[$attr])* $vis $name, $t, const $init); ); // process multiple declarations ($(#[$attr:meta])* $vis:vis static $name:ident: $t:ty = $init:expr; $($rest:tt)*) => ( - $crate::__thread_local_inner!($(#[$attr])* $vis $name, $t, $init); + $crate::thread::local_impl::thread_local_inner!($(#[$attr])* $vis $name, $t, $init); $crate::thread_local!($($rest)*); ); // handle a single declaration ($(#[$attr:meta])* $vis:vis static $name:ident: $t:ty = $init:expr) => ( - $crate::__thread_local_inner!($(#[$attr])* $vis $name, $t, $init); + $crate::thread::local_impl::thread_local_inner!($(#[$attr])* $vis $name, $t, $init); ); } diff --git a/library/std/src/thread/mod.rs b/library/std/src/thread/mod.rs index 13b845b25c92d..e8dc7cf92810a 100644 --- a/library/std/src/thread/mod.rs +++ b/library/std/src/thread/mod.rs @@ -204,9 +204,12 @@ pub use self::local::{AccessError, LocalKey}; // by the elf linker. "static" is for single-threaded platforms where a global // static is sufficient. +// Implementation details used by the thread_local!{} macro. #[doc(hidden)] -#[unstable(feature = "libstd_thread_internals", issue = "none")] -pub use crate::sys::common::thread_local::Key as __LocalKeyInner; +#[unstable(feature = "thread_local_internals", issue = "none")] +pub mod local_impl { + pub use crate::sys::common::thread_local::{thread_local_inner, Key}; +} //////////////////////////////////////////////////////////////////////////////// // Builder From 5a62ebb0b146252cafd0496a897a9cbd247187ec Mon Sep 17 00:00:00 2001 From: Mara Bos Date: Wed, 26 Apr 2023 21:02:45 +0200 Subject: [PATCH 2/3] Remove libstd_thread_internals from unstable book. --- .../src/library-features/libstd-thread-internals.md | 5 ----- 1 file changed, 5 deletions(-) delete mode 100644 src/doc/unstable-book/src/library-features/libstd-thread-internals.md diff --git a/src/doc/unstable-book/src/library-features/libstd-thread-internals.md b/src/doc/unstable-book/src/library-features/libstd-thread-internals.md deleted file mode 100644 index b682d12e7cdd1..0000000000000 --- a/src/doc/unstable-book/src/library-features/libstd-thread-internals.md +++ /dev/null @@ -1,5 +0,0 @@ -# `libstd_thread_internals` - -This feature is internal to the Rust compiler and is not intended for general use. - ------------------------- From 12fee7b00424a79755d178d21298adbd8985e784 Mon Sep 17 00:00:00 2001 From: Mara Bos Date: Wed, 26 Apr 2023 21:02:56 +0200 Subject: [PATCH 3/3] Update tests. --- tests/ui/macros/macro-local-data-key-priv.stderr | 2 +- tests/ui/threads-sendsync/issue-43733-2.rs | 2 +- tests/ui/threads-sendsync/issue-43733.mir.stderr | 4 ++-- tests/ui/threads-sendsync/issue-43733.rs | 8 +++----- tests/ui/threads-sendsync/issue-43733.thir.stderr | 6 +++--- 5 files changed, 10 insertions(+), 12 deletions(-) diff --git a/tests/ui/macros/macro-local-data-key-priv.stderr b/tests/ui/macros/macro-local-data-key-priv.stderr index fb8cab2794bc6..0f412bc86fb31 100644 --- a/tests/ui/macros/macro-local-data-key-priv.stderr +++ b/tests/ui/macros/macro-local-data-key-priv.stderr @@ -9,7 +9,7 @@ note: the constant `baz` is defined here | LL | thread_local!(static baz: f64 = 0.0); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - = note: this error originates in the macro `$crate::__thread_local_inner` which comes from the expansion of the macro `thread_local` (in Nightly builds, run with -Z macro-backtrace for more info) + = note: this error originates in the macro `$crate::thread::local_impl::thread_local_inner` which comes from the expansion of the macro `thread_local` (in Nightly builds, run with -Z macro-backtrace for more info) error: aborting due to previous error diff --git a/tests/ui/threads-sendsync/issue-43733-2.rs b/tests/ui/threads-sendsync/issue-43733-2.rs index 8f7a9c0837510..e9653dbe5c222 100644 --- a/tests/ui/threads-sendsync/issue-43733-2.rs +++ b/tests/ui/threads-sendsync/issue-43733-2.rs @@ -21,7 +21,7 @@ impl Key { } #[cfg(target_thread_local)] -use std::thread::__LocalKeyInner as Key; +use std::thread::local_impl::Key; static __KEY: Key<()> = Key::new(); //~^ ERROR `UnsafeCell>` cannot be shared between threads diff --git a/tests/ui/threads-sendsync/issue-43733.mir.stderr b/tests/ui/threads-sendsync/issue-43733.mir.stderr index 1e21a6b37a991..ff83e16add9ef 100644 --- a/tests/ui/threads-sendsync/issue-43733.mir.stderr +++ b/tests/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:21: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:26:42 + --> $DIR/issue-43733.rs:24:42 | LL | static FOO: std::thread::LocalKey = std::thread::LocalKey::new(__getit); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ call to unsafe function diff --git a/tests/ui/threads-sendsync/issue-43733.rs b/tests/ui/threads-sendsync/issue-43733.rs index 0eadef3e3e8cb..cac745f1e12f3 100644 --- a/tests/ui/threads-sendsync/issue-43733.rs +++ b/tests/ui/threads-sendsync/issue-43733.rs @@ -1,8 +1,6 @@ // ignore-wasm32 // revisions: mir thir // [thir]compile-flags: -Z thir-unsafeck -// normalize-stderr-test: "__LocalKeyInner::::get" -> "$$LOCALKEYINNER::::get" -// normalize-stderr-test: "__LocalKeyInner::::get" -> "$$LOCALKEYINNER::::get" #![feature(thread_local)] #![feature(cfg_target_thread_local, thread_local_internals)] @@ -12,15 +10,15 @@ type Foo = std::cell::RefCell; #[cfg(target_thread_local)] #[thread_local] -static __KEY: std::thread::__LocalKeyInner = std::thread::__LocalKeyInner::new(); +static __KEY: std::thread::local_impl::Key = std::thread::local_impl::Key::new(); #[cfg(not(target_thread_local))] -static __KEY: std::thread::__LocalKeyInner = std::thread::__LocalKeyInner::new(); +static __KEY: std::thread::local_impl::Key = std::thread::local_impl::Key::new(); fn __getit(_: Option<&mut Option>>) -> std::option::Option<&'static Foo> { __KEY.get(Default::default) //[mir]~^ ERROR call to unsafe function is unsafe - //[thir]~^^ ERROR call to unsafe function `__ + //[thir]~^^ ERROR call to unsafe function `Key::::get` } static FOO: std::thread::LocalKey = std::thread::LocalKey::new(__getit); diff --git a/tests/ui/threads-sendsync/issue-43733.thir.stderr b/tests/ui/threads-sendsync/issue-43733.thir.stderr index ea7ff4080486d..94ec724044c7a 100644 --- a/tests/ui/threads-sendsync/issue-43733.thir.stderr +++ b/tests/ui/threads-sendsync/issue-43733.thir.stderr @@ -1,5 +1,5 @@ -error[E0133]: call to unsafe function `$LOCALKEYINNER::::get` is unsafe and requires unsafe function or block - --> $DIR/issue-43733.rs:21:5 +error[E0133]: call to unsafe function `Key::::get` is unsafe and requires unsafe function or block + --> $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 `LocalKey::::new` is unsafe and requires unsafe function or block - --> $DIR/issue-43733.rs:26:42 + --> $DIR/issue-43733.rs:24:42 | LL | static FOO: std::thread::LocalKey = std::thread::LocalKey::new(__getit); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ call to unsafe function