Auto merge of #110861 - m-ou-se:thread-local-restructure, r=workingjubilee

Restructure and rename std thread_local internals to make it less of a maze

Every time I try to work on std's thread local internals, it feels like I'm trying to navigate a confusing maze made of macros, deeply nested modules, and types with multiple names/aliases. Time to clean it up a bit.

This PR:

- Exports `Key` with its own name (`Key`), instead of `__LocalKeyInner`
- Uses `pub macro` to put `__thread_local_inner` into a (unstable, hidden) module, removing `#[macro_export]`, removing it from the crate root.
- Removes the `__` from `__thread_local_inner`.
- Removes a few unnecessary `allow_internal_unstable` features from the macros
- Removes the `libstd_thread_internals` feature. (Merged with `thread_local_internals`.)
    - And removes it from the unstable book
- Gets rid of the deeply nested modules for the `Key` definitions (`mod fast` / `mod os` / `mod statik`).
- Turns a `#[cfg]` mess into a single `cfg_if`, now that there's no `#[macro_export]` anymore that breaks with `cfg_if`.
- Simplifies the `cfg_if` conditions to not repeat the conditions.
- Removes useless `normalize-stderr-test`, which were left over from when the `Key` types had different names on different platforms.
- Removes a seemingly unnecessary `realstd` re-export on `cfg(test)`.

This PR changes nothing about the thread local implementation. That's for a later PR. (Which should hopefully be easier once all this stuff is a bit cleaned up.)

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,33 +108,26 @@ 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 {
+#[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.
-    //
-    // LLVM issue: https://bugs.llvm.org/show_bug.cgi?id=41722
-    pub struct Key<T> {
+// 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<T> {
     // If `LazyKeyInner::get` returns `None`, that indicates either:
     //   * The value has never been initialized
     //   * The value is being recursively initialized
@@ -147,15 +141,15 @@ pub mod fast {
     // Metadata to keep track of the state of the destructor. Remember that
     // this variable is thread-local, not global.
     dtor_state: Cell<DtorState>,
-    }
+}
 
-    impl<T> fmt::Debug for Key<T> {
+impl<T> fmt::Debug for Key<T> {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         f.debug_struct("Key").finish_non_exhaustive()
     }
-    }
+}
 
-    impl<T> Key<T> {
+impl<T> Key<T> {
     pub const fn new() -> Key<T> {
         Key { inner: LazyKeyInner::new(), dtor_state: Cell::new(DtorState::Unregistered) }
     }
@@ -225,9 +219,9 @@ pub mod fast {
             DtorState::RunningOrHasRun => false,
         }
     }
-    }
+}
 
-    unsafe extern "C" fn destroy_value<T>(ptr: *mut u8) {
+unsafe extern "C" fn destroy_value<T>(ptr: *mut u8) {
     let ptr = ptr as *mut Key<T>;
 
     // SAFETY:
@@ -250,5 +244,4 @@ pub mod fast {
     })) {
         rtabort!("thread local panicked on drop");
     }
-    }
 }

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 fast_local::fast::Key;
-    } else if #[cfg(all(not(target_thread_local), not(all(target_family = "wasm", not(target_feature = "atomics")))))] {
+        pub use static_local::{Key, thread_local_inner};
+    } else if #[cfg(all(target_thread_local))] {
         #[doc(hidden)]
-        pub use os_local::os::Key;
+        mod fast_local;
+        #[doc(hidden)]
+        pub use fast_local::{Key, thread_local_inner};
+    } else {
+        #[doc(hidden)]
+        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;

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,42 +81,35 @@ 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<T> {
+/// Use a regular global static to store this key; the state provided will then be
+/// thread-local.
+pub struct Key<T> {
     // OS-TLS key that we'll use to key off.
     os: OsStaticKey,
     marker: marker::PhantomData<Cell<T>>,
-    }
+}
 
-    impl<T> fmt::Debug for Key<T> {
+impl<T> fmt::Debug for Key<T> {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         f.debug_struct("Key").finish_non_exhaustive()
     }
-    }
+}
 
-    unsafe impl<T> Sync for Key<T> {}
+unsafe impl<T> Sync for Key<T> {}
 
-    struct Value<T: 'static> {
+struct Value<T: 'static> {
     inner: LazyKeyInner<T>,
     key: &'static Key<T>,
-    }
+}
 
-    impl<T: 'static> Key<T> {
+impl<T: 'static> Key<T> {
     #[rustc_const_unstable(feature = "thread_local_internals", issue = "none")]
     pub const fn new() -> Key<T> {
         Key { os: OsStaticKey::new(Some(destroy_value::<T>)), marker: marker::PhantomData }
@@ -169,9 +163,9 @@ pub mod os {
         // dereferenced safely.
         unsafe { Some((*ptr).inner.initialize(init)) }
     }
-    }
+}
 
-    unsafe extern "C" fn destroy_value<T: 'static>(ptr: *mut u8) {
+unsafe extern "C" fn destroy_value<T: 'static>(ptr: *mut u8) {
     // SAFETY:
     //
     // The OS TLS ensures that this key contains a null value when this
@@ -193,5 +187,4 @@ pub mod os {
     }) {
         rtabort!("thread local panicked on drop");
     }
-    }
 }

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,33 +65,29 @@ 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<T> {
+pub struct Key<T> {
     inner: LazyKeyInner<T>,
-    }
+}
 
-    unsafe impl<T> Sync for Key<T> {}
+unsafe impl<T> Sync for Key<T> {}
 
-    impl<T> fmt::Debug for Key<T> {
+impl<T> fmt::Debug for Key<T> {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         f.debug_struct("Key").finish_non_exhaustive()
     }
-    }
+}
 
-    impl<T> Key<T> {
+impl<T> Key<T> {
     pub const fn new() -> Key<T> {
         Key { inner: LazyKeyInner::new() }
     }
@@ -111,5 +106,4 @@ pub mod statik {
 
         Some(value)
     }
-    }
 }

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);
     );
 }
 

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

src/doc/unstable-book/src/library-features/libstd-thread-internals.md

@@ -1,5 +0,0 @@
-# `libstd_thread_internals`
-
-This feature is internal to the Rust compiler and is not intended for general use.
-
-------------------------

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
 

tests/ui/threads-sendsync/issue-43733-2.rs

@@ -21,7 +21,7 @@ impl<T> Key<T> {
 }
 
 #[cfg(target_thread_local)]
-use std::thread::__LocalKeyInner as Key;
+use std::thread::local_impl::Key;
 
 static __KEY: Key<()> = Key::new();
 //~^ ERROR `UnsafeCell<Option<()>>` cannot be shared between threads

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<Foo> = std::thread::LocalKey::new(__getit);
    |                                          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ call to unsafe function

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::<T>::get" -> "$$LOCALKEYINNER::<T>::get"
-// normalize-stderr-test: "__LocalKeyInner::<T>::get" -> "$$LOCALKEYINNER::<T>::get"
 #![feature(thread_local)]
 #![feature(cfg_target_thread_local, thread_local_internals)]
 
@@ -12,15 +10,15 @@ type Foo = std::cell::RefCell<String>;
 
 #[cfg(target_thread_local)]
 #[thread_local]
-static __KEY: std::thread::__LocalKeyInner<Foo> = std::thread::__LocalKeyInner::new();
+static __KEY: std::thread::local_impl::Key<Foo> = std::thread::local_impl::Key::new();
 
 #[cfg(not(target_thread_local))]
-static __KEY: std::thread::__LocalKeyInner<Foo> = std::thread::__LocalKeyInner::new();
+static __KEY: std::thread::local_impl::Key<Foo> = std::thread::local_impl::Key::new();
 
 fn __getit(_: Option<&mut Option<RefCell<String>>>) -> 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::<T>::get`
 }
 
 static FOO: std::thread::LocalKey<Foo> = std::thread::LocalKey::new(__getit);

tests/ui/threads-sendsync/issue-43733.thir.stderr

@@ -1,5 +1,5 @@
-error[E0133]: call to unsafe function `$LOCALKEYINNER::<T>::get` is unsafe and requires unsafe function or block
-  --> $DIR/issue-43733.rs:21:5
+error[E0133]: call to unsafe function `Key::<T>::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::<T>::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<Foo> = std::thread::LocalKey::new(__getit);
    |                                          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ call to unsafe function