From 7ffccf1020851f21b2600fa2717a300a0afef255 Mon Sep 17 00:00:00 2001
From: joboet <jonasboettiger@icloud.com>
Date: Sun, 24 Sep 2023 19:46:29 +0200
Subject: [PATCH 1/3] std: rewrite native thread-local storage

---
 .../std/src/sys/thread_local/fast_local.rs    | 432 ++++++++++--------
 library/std/src/sys/thread_local/mod.rs       |   5 +-
 library/std/src/thread/mod.rs                 |   2 +-
 3 files changed, 249 insertions(+), 190 deletions(-)

diff --git a/library/std/src/sys/thread_local/fast_local.rs b/library/std/src/sys/thread_local/fast_local.rs
index 646dcd7f3a3e8..34db18e2f2292 100644
--- a/library/std/src/sys/thread_local/fast_local.rs
+++ b/library/std/src/sys/thread_local/fast_local.rs
@@ -1,72 +1,93 @@
-use super::lazy::LazyKeyInner;
-use crate::cell::Cell;
+//! Thread local support for platforms with native TLS.
+//!
+//! To achieve the best performance, we need to differentiate between four types
+//! of TLS, resulting from the method of initialization used (`const` or lazy)
+//! and the drop requirements of the stored type, as indicated by
+//! [`needs_drop`](crate::mem::needs_drop). However, across these types a TLS
+//! variable can only be in three different different states, allowing us to
+//! use a common implementation specialized by type parameters. The three states
+//! are:
+//!
+//! 1. [`Initial`](State::Initial): the destructor has not been registered and/or
+//!    the variable is uninitialized.
+//! 2. [`Alive`](State::Alive): if it exists, the destructor is registered. The
+//!    variable is initialized.
+//! 3. [`Destroyed`](State::Destroyed): the TLS variable has been destroyed.
+//!
+//! Upon accessing the TLS variable through [`get_or_init`](Storage::get_or_init),
+//! the current state is compared:
+//!
+//! 1. If the state is `Initial`, a macro-provided closure is run with the
+//!    parameter stored with the state, which returns the initialization value
+//!    for the variable. The state is transitioned to `Alive` and the destructor
+//!    is registered if the variable should be destroyed.
+//! 2. If the state is `Alive`, initialization was previously completed, so a
+//!    reference to the stored value is returned.
+//! 3. If the state is `Destroyed`, the TLS variable was already destroyed, so
+//!    return [`None`].
+//!
+//! The TLS destructor sets the state to `Destroyed` and drops the current value.
+//!
+//! `const`-initialized variables use the initialization value as parameter to
+//! the initialization closure, which is just the [`identity`](crate::convert::identity)
+//! function. Storing this value in the variable from the beginning means that
+//! no copying needs to take place (if the optimizer does a good job).
+//!
+//! Lazily initialized variables simply use `()` as parameter and perform the
+//! actual initialization inside of the closure.
+//!
+//! By using the `!` type (never type) as type parameter for the destroyed state,
+//! we can eliminate the `Destroyed` state for values that do not need a
+//! destructor using a generic parameter, which allows niche optimizations to
+//! occur for the `State` enum. Otherwise, `()` is used.
+
+#![deny(unsafe_op_in_unsafe_fn)]
+
+use super::abort_on_dtor_unwind;
+use crate::cell::UnsafeCell;
+use crate::hint::unreachable_unchecked;
+use crate::mem::forget;
+use crate::ptr;
 use crate::sys::thread_local_dtor::register_dtor;
-use crate::{fmt, mem, panic};
 
 #[doc(hidden)]
-#[allow_internal_unstable(thread_local_internals, cfg_target_thread_local, thread_local)]
+#[allow_internal_unstable(
+    thread_local_internals,
+    cfg_target_thread_local,
+    thread_local,
+    never_type
+)]
 #[allow_internal_unsafe]
 #[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) => {{
+        const __INIT: $t = $init;
+
         #[inline]
         #[deny(unsafe_op_in_unsafe_fn)]
-        // FIXME: Use `SyncUnsafeCell` instead of allowing `static_mut_refs` lint
-        #[cfg_attr(bootstrap, allow(static_mut_ref))]
-        #[cfg_attr(not(bootstrap), allow(static_mut_refs))]
         unsafe fn __getit(
             _init: $crate::option::Option<&mut $crate::option::Option<$t>>,
         ) -> $crate::option::Option<&'static $t> {
-            const INIT_EXPR: $t = $init;
-            // If the platform has support for `#[thread_local]`, use it.
-            #[thread_local]
-            static mut VAL: $t = INIT_EXPR;
-
-            // If a dtor isn't needed we can do something "very raw" and
-            // just get going.
-            if !$crate::mem::needs_drop::<$t>() {
+            use $crate::thread::local_impl::Storage;
+            use $crate::mem::needs_drop;
+            use $crate::convert::identity;
+            use $crate::ptr::addr_of;
+
+            if needs_drop::<$t>() {
+                #[thread_local]
+                static VAL: Storage<$t, $t, ()> = Storage::new(__INIT);
                 unsafe {
-                    return $crate::option::Option::Some(&VAL)
+                    VAL.get_or_init(identity)
                 }
-            }
-
-            // 0 == dtor not registered
-            // 1 == dtor registered, dtor not run
-            // 2 == dtor registered and is running or has run
-            #[thread_local]
-            static STATE: $crate::cell::Cell<$crate::primitive::u8> = $crate::cell::Cell::new(0);
-
-            // Safety: Performs `drop_in_place(ptr as *mut $t)`, and requires
-            // all that comes with it.
-            unsafe extern "C" fn destroy(ptr: *mut $crate::primitive::u8) {
-                $crate::thread::local_impl::abort_on_dtor_unwind(|| {
-                    let old_state = STATE.replace(2);
-                    $crate::debug_assert_eq!(old_state, 1);
-                    // Safety: safety requirement is passed on to caller.
-                    unsafe { $crate::ptr::drop_in_place(ptr.cast::<$t>()); }
-                });
-            }
-
-            unsafe {
-                match STATE.get() {
-                    // 0 == we haven't registered a destructor, so do
-                    //   so now.
-                    0 => {
-                        $crate::thread::local_impl::Key::<$t>::register_dtor(
-                            $crate::ptr::addr_of_mut!(VAL) as *mut $crate::primitive::u8,
-                            destroy,
-                        );
-                        STATE.set(1);
-                        $crate::option::Option::Some(&VAL)
-                    }
-                    // 1 == the destructor is registered and the value
-                    //   is valid, so return the pointer.
-                    1 => $crate::option::Option::Some(&VAL),
-                    // otherwise the destructor has already run, so we
-                    // can't give access.
-                    _ => $crate::option::Option::None,
+            } else {
+                // Just use thread-local statics directly instead of going
+                // through `Storage`.
+                #[thread_local]
+                static VAL: $t = __INIT;
+                unsafe {
+                    $crate::option::Option::Some(&*addr_of!(VAL))
                 }
             }
         }
@@ -77,171 +98,206 @@ pub macro thread_local_inner {
     }},
 
     // used to generate the `LocalKey` value for `thread_local!`
-    (@key $t:ty, $init:expr) => {
-        {
-            #[inline]
-            fn __init() -> $t { $init }
-
-            #[inline]
-            unsafe fn __getit(
-                init: $crate::option::Option<&mut $crate::option::Option<$t>>,
-            ) -> $crate::option::Option<&'static $t> {
-                #[thread_local]
-                static __KEY: $crate::thread::local_impl::Key<$t> =
-                    $crate::thread::local_impl::Key::<$t>::new();
+    (@key $t:ty, $init:expr) => {{
+        #[inline]
+        fn __init() -> $t {
+            $init
+        }
 
+        #[inline]
+        #[deny(unsafe_op_in_unsafe_fn)]
+        unsafe fn __getit(
+            init: $crate::option::Option<&mut $crate::option::Option<$t>>,
+        ) -> $crate::option::Option<&'static $t> {
+            use $crate::thread::local_impl::{Storage, take_or_call};
+            use $crate::mem::needs_drop;
+
+            if needs_drop::<$t>() {
+                #[thread_local]
+                static VAL: Storage<(), $t, ()> = Storage::new(());
+                unsafe {
+                    VAL.get_or_init(|()| take_or_call(init, __init))
+                }
+            } else {
+                #[thread_local]
+                static VAL: Storage<(), $t, !> = Storage::new(());
                 unsafe {
-                    __KEY.get(move || {
-                        if let $crate::option::Option::Some(init) = init {
-                            if let $crate::option::Option::Some(value) = init.take() {
-                                return value;
-                            }
-                            if $crate::cfg!(debug_assertions) {
-                                $crate::unreachable!("missing default value");
-                            }
-                        }
-                        __init()
-                    })
+                    VAL.get_or_init(|()| take_or_call(init, __init))
                 }
             }
+        }
 
-            unsafe {
-                $crate::thread::LocalKey::new(__getit)
-            }
+        unsafe {
+            $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_impl::thread_local_inner!(@key $t, $($init)*);
     },
 }
 
-#[derive(Copy, Clone)]
-enum DtorState {
-    Unregistered,
-    Registered,
-    RunningOrHasRun,
+#[inline]
+pub fn take_or_call<T, F>(i: Option<&mut Option<T>>, f: F) -> T
+where
+    F: FnOnce() -> T,
+{
+    i.and_then(Option::take).unwrap_or_else(f)
 }
 
-// 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
-    //   * 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<T>,
-
-    // Metadata to keep track of the state of the destructor. Remember that
-    // this variable is thread-local, not global.
-    dtor_state: Cell<DtorState>,
+pub trait DestroyedState {
+    const EXISTS: bool;
+    fn create() -> Self;
 }
 
-impl<T> fmt::Debug for Key<T> {
-    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        f.debug_struct("Key").finish_non_exhaustive()
+impl DestroyedState for ! {
+    const EXISTS: bool = false;
+    fn create() -> ! {
+        panic!("attempted to create nonexistent state");
     }
 }
-impl<T> Key<T> {
-    pub const fn new() -> Key<T> {
-        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);
-        }
-    }
+impl DestroyedState for () {
+    const EXISTS: bool = true;
+    #[inline]
+    fn create() {}
+}
 
-    pub unsafe fn get<F: FnOnce() -> 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),
-            }
-        }
+enum State<I, T, D> {
+    Initial(I),
+    Alive(T),
+    Destroyed(D),
+}
+
+#[allow(missing_debug_implementations)]
+pub struct Storage<I, T, D> {
+    state: UnsafeCell<State<I, T, D>>,
+}
+
+impl<I, T, D> Storage<I, T, D>
+where
+    D: DestroyedState,
+{
+    /// Create a new TLS storage with the provided initialization parameter.
+    pub const fn new(param: I) -> Storage<I, T, D> {
+        Storage { state: UnsafeCell::new(State::Initial(param)) }
     }
 
-    // `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<F: FnOnce() -> T>(&self, init: F) -> Option<&'static T> {
-        // SAFETY: See comment above (this function doc).
-        if !mem::needs_drop::<T>() || unsafe { self.try_register_dtor() } {
-            // SAFETY: See comment above (this function doc).
-            Some(unsafe { self.inner.initialize(init) })
-        } else {
-            None
+    /// Get a reference to the TLS value, potentially initializing it with the
+    /// provided closure. If the TLS variable has been destroyed already, `None`
+    /// is returned.
+    ///
+    /// # Safety
+    /// * The `self` reference must remain valid until the TLS destructor is run,
+    ///   at which point the returned reference is invalidated.
+    /// * If the closure may panic or access this variable, the initialization
+    ///   parameter must implement `Copy`.
+    /// * The returned reference may only be used until thread destruction occurs
+    ///   and may not be used after reentrant initialization has occurred.
+    ///
+    // FIXME(#110897): return NonNull instead of lying about the lifetime.
+    #[inline]
+    pub unsafe fn get_or_init<F>(&self, init: F) -> Option<&'static T>
+    where
+        F: FnOnce(I) -> T,
+    {
+        // SAFETY:
+        // No mutable reference to the inner value exists outside the calls to
+        // `replace`. The lifetime of the returned reference fulfills the terms
+        // outlined above.
+        let state = unsafe { &*self.state.get() };
+        match state {
+            State::Alive(v) => Some(v),
+            State::Destroyed(_) => None,
+            State::Initial(_) => unsafe { self.initialize(init) },
         }
     }
 
-    // `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<T>`
-                // is safe because the function will build a pointer to a
-                // Key<T>, which is the type of self and so find the correct
-                // size.
-                unsafe { register_dtor(self as *const _ as *mut u8, destroy_value::<T>) };
-                self.dtor_state.set(DtorState::Registered);
-                true
+    #[cold]
+    unsafe fn initialize<F>(&self, init: F) -> Option<&'static T>
+    where
+        F: FnOnce(I) -> T,
+    {
+        // Perform initialization
+
+        // SAFETY:
+        // The state is valid for reading (see above) and must be `Initial` at
+        // this point. If the closure panics or recursively initializes the
+        // variable, the initialization parameter is duplicated. The caller
+        // asserts that this is valid. Otherwise the old copy is leaked below.
+        let state = unsafe { self.state.get().read() };
+        let State::Initial(param) = state else { unsafe { unreachable_unchecked() } };
+
+        let v = init(param);
+
+        // SAFETY:
+        // If references to the inner value exist, they were created in `init`
+        // and are invalidated here. The caller promises to never use them
+        // after this.
+        let old = unsafe { self.state.get().replace(State::Alive(v)) };
+
+        let recursive = match old {
+            State::Initial(v) => {
+                // The value was duplicated above, leak the old value to prevent
+                // double freeing.
+                forget(v);
+                false
             }
-            DtorState::Registered => {
-                // recursively initialized
+            State::Alive(v) => {
+                // The storage was recursively initialized, so drop the previous
+                // value. This could be changed to a panic in the future.
+                drop(v);
                 true
             }
-            DtorState::RunningOrHasRun => false,
+            State::Destroyed(_) => {
+                unreachable!("thread is still alive but TLS storage was destroyed");
+            }
+        };
+
+        if D::EXISTS && !recursive {
+            // If a `Destroyed` state exists and the variable was not initialized
+            // before, register the destructor.
+            unsafe {
+                register_dtor(ptr::from_ref(self).cast_mut().cast(), destroy::<I, T, D>);
+            }
+        }
+
+        // SAFETY:
+        // Initialization was completed and the state was set to `Alive`, so the
+        // reference fulfills the terms outlined above.
+        unsafe {
+            let State::Alive(v) = &*self.state.get() else { unreachable_unchecked() };
+            Some(v)
         }
     }
 }
 
-unsafe extern "C" fn destroy_value<T>(ptr: *mut u8) {
-    let ptr = ptr as *mut Key<T>;
-
-    // SAFETY:
-    //
-    // The pointer `ptr` has been built just above and comes from
-    // `try_register_dtor` where it is originally a Key<T> 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<T>` 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");
-    }
+/// Transition an `Alive` TLS variable into the `Destroyed` state, dropping its
+/// value. Should only be called when `D::SHOULD_DESTROY` is `true`. May abort
+/// otherwise.
+///
+/// # Safety
+/// * Must only be called at thread destruction.
+/// * `ptr` must point to an instance of `Storage` with matching generic parameters
+///   and be valid for accessing that instance.
+/// * Must only be called if the state is `Alive`.
+unsafe extern "C" fn destroy<I, T, D>(ptr: *mut u8)
+where
+    D: DestroyedState,
+{
+    // Print a nice abort message if a panic occurs.
+    abort_on_dtor_unwind(|| {
+        let storage = unsafe { &*(ptr as *const Storage<I, T, D>) };
+        // Update the state before running the destructor as it may attempt to
+        // access the variable.
+        let state = unsafe { storage.state.get().replace(State::Destroyed(D::create())) };
+        match state {
+            State::Alive(v) => drop(v),
+            // This should not occur but do nothing if it does.
+            State::Destroyed(_) => {}
+            State::Initial(_) => {
+                unreachable!("destructor cannot run if it was not registered yet");
+            }
+        }
+    })
 }
diff --git a/library/std/src/sys/thread_local/mod.rs b/library/std/src/sys/thread_local/mod.rs
index 8b2c839f837d4..01e1edf50290e 100644
--- a/library/std/src/sys/thread_local/mod.rs
+++ b/library/std/src/sys/thread_local/mod.rs
@@ -15,7 +15,7 @@ cfg_if::cfg_if! {
         #[doc(hidden)]
         mod fast_local;
         #[doc(hidden)]
-        pub use fast_local::{Key, thread_local_inner};
+        pub use fast_local::{Storage, take_or_call, thread_local_inner};
     } else {
         #[doc(hidden)]
         mod os_local;
@@ -24,6 +24,9 @@ cfg_if::cfg_if! {
     }
 }
 
+// Not used by the fast-local TLS anymore.
+// FIXME(#110897): remove this.
+#[allow(unused)]
 mod lazy {
     use crate::cell::UnsafeCell;
     use crate::hint;
diff --git a/library/std/src/thread/mod.rs b/library/std/src/thread/mod.rs
index 85de2980133d5..5a5c3a6fe152a 100644
--- a/library/std/src/thread/mod.rs
+++ b/library/std/src/thread/mod.rs
@@ -205,7 +205,7 @@ cfg_if::cfg_if! {
         #[doc(hidden)]
         #[unstable(feature = "thread_local_internals", issue = "none")]
         pub mod local_impl {
-            pub use crate::sys::thread_local::{thread_local_inner, Key, abort_on_dtor_unwind};
+            pub use crate::sys::thread_local::*;
         }
     }
 }

From 7b697337c549bb931af5e7ec330e686d443e5999 Mon Sep 17 00:00:00 2001
From: joboet <jonasboettiger@icloud.com>
Date: Tue, 12 Mar 2024 14:02:48 +0100
Subject: [PATCH 2/3] delete UI tests that only check internal implementation
 details of thread-locals

---
 src/tools/tidy/src/issues.txt                |  2 --
 tests/ui/threads-sendsync/issue-43733-2.rs   | 30 ------------------
 tests/ui/threads-sendsync/issue-43733.rs     | 32 --------------------
 tests/ui/threads-sendsync/issue-43733.stderr | 19 ------------
 4 files changed, 83 deletions(-)
 delete mode 100644 tests/ui/threads-sendsync/issue-43733-2.rs
 delete mode 100644 tests/ui/threads-sendsync/issue-43733.rs
 delete mode 100644 tests/ui/threads-sendsync/issue-43733.stderr

diff --git a/src/tools/tidy/src/issues.txt b/src/tools/tidy/src/issues.txt
index 91bbf5041ff58..dffdf11628294 100644
--- a/src/tools/tidy/src/issues.txt
+++ b/src/tools/tidy/src/issues.txt
@@ -4010,8 +4010,6 @@
 "ui/test-attrs/issue-53675-a-test-called-panic.rs",
 "ui/threads-sendsync/issue-24313.rs",
 "ui/threads-sendsync/issue-29488.rs",
-"ui/threads-sendsync/issue-43733-2.rs",
-"ui/threads-sendsync/issue-43733.rs",
 "ui/threads-sendsync/issue-4446.rs",
 "ui/threads-sendsync/issue-4448.rs",
 "ui/threads-sendsync/issue-8827.rs",
diff --git a/tests/ui/threads-sendsync/issue-43733-2.rs b/tests/ui/threads-sendsync/issue-43733-2.rs
deleted file mode 100644
index 372ebf2cff97f..0000000000000
--- a/tests/ui/threads-sendsync/issue-43733-2.rs
+++ /dev/null
@@ -1,30 +0,0 @@
-//@ needs-threads
-//@ dont-check-compiler-stderr
-#![feature(cfg_target_thread_local, thread_local_internals)]
-
-// On platforms *without* `#[thread_local]`, use
-// a custom non-`Sync` type to fake the same error.
-#[cfg(not(target_thread_local))]
-struct Key<T> {
-    _data: std::cell::UnsafeCell<Option<T>>,
-    _flag: std::cell::Cell<()>,
-}
-
-#[cfg(not(target_thread_local))]
-impl<T> Key<T> {
-    const fn new() -> Self {
-        Key {
-            _data: std::cell::UnsafeCell::new(None),
-            _flag: std::cell::Cell::new(()),
-        }
-    }
-}
-
-#[cfg(target_thread_local)]
-use std::thread::local_impl::Key;
-
-static __KEY: Key<()> = Key::new();
-//~^ ERROR `UnsafeCell<Option<()>>` cannot be shared between threads
-//~| ERROR cannot be shared between threads safely [E0277]
-
-fn main() {}
diff --git a/tests/ui/threads-sendsync/issue-43733.rs b/tests/ui/threads-sendsync/issue-43733.rs
deleted file mode 100644
index c90f60887cfd2..0000000000000
--- a/tests/ui/threads-sendsync/issue-43733.rs
+++ /dev/null
@@ -1,32 +0,0 @@
-//@ needs-threads
-#![feature(thread_local)]
-#![feature(cfg_target_thread_local, thread_local_internals)]
-
-use std::cell::RefCell;
-
-type Foo = std::cell::RefCell<String>;
-
-#[cfg(target_thread_local)]
-#[thread_local]
-static __KEY: std::thread::local_impl::Key<Foo> = std::thread::local_impl::Key::new();
-
-#[cfg(not(target_thread_local))]
-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)
-    //~^ ERROR call to unsafe function `Key::<T>::get` is unsafe
-}
-
-static FOO: std::thread::LocalKey<Foo> = std::thread::LocalKey::new(__getit);
-//~^ ERROR call to unsafe function `LocalKey::<T>::new` is unsafe
-
-fn main() {
-    FOO.with(|foo| println!("{}", foo.borrow()));
-    std::thread::spawn(|| {
-        FOO.with(|foo| *foo.borrow_mut() += "foo");
-    })
-    .join()
-    .unwrap();
-    FOO.with(|foo| println!("{}", foo.borrow()));
-}
diff --git a/tests/ui/threads-sendsync/issue-43733.stderr b/tests/ui/threads-sendsync/issue-43733.stderr
deleted file mode 100644
index 9b13646a22852..0000000000000
--- a/tests/ui/threads-sendsync/issue-43733.stderr
+++ /dev/null
@@ -1,19 +0,0 @@
-error[E0133]: call to unsafe function `Key::<T>::get` is unsafe and requires unsafe function or block
-  --> $DIR/issue-43733.rs:17:5
-   |
-LL |     __KEY.get(Default::default)
-   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^ call to unsafe function
-   |
-   = 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:21:42
-   |
-LL | static FOO: std::thread::LocalKey<Foo> = std::thread::LocalKey::new(__getit);
-   |                                          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ call to unsafe function
-   |
-   = note: consult the function's documentation for information on how to avoid undefined behavior
-
-error: aborting due to 2 previous errors
-
-For more information about this error, try `rustc --explain E0133`.

From a087a37a4362b615c306689878410ed69140e527 Mon Sep 17 00:00:00 2001
From: joboet <jonasboettiger@icloud.com>
Date: Tue, 12 Mar 2024 15:39:58 +0100
Subject: [PATCH 3/3] update UI test, make it run on more platforms

---
 .../suggestions/missing-lifetime-specifier.rs |  31 ++--
 .../missing-lifetime-specifier.stderr         | 138 +++++++++---------
 2 files changed, 87 insertions(+), 82 deletions(-)

diff --git a/tests/ui/suggestions/missing-lifetime-specifier.rs b/tests/ui/suggestions/missing-lifetime-specifier.rs
index 3fa7f75f86219..d4fcbb9b37940 100644
--- a/tests/ui/suggestions/missing-lifetime-specifier.rs
+++ b/tests/ui/suggestions/missing-lifetime-specifier.rs
@@ -1,6 +1,11 @@
-// different number of duplicated diagnostics on different targets
-//@ only-x86_64
-//@ only-linux
+// The specific errors produced depend the thread-local implementation.
+// Run only on platforms with "fast" TLS.
+//@ ignore-windows FIXME(#84933)
+//@ ignore-wasm globals are used instead of thread locals
+//@ ignore-emscripten globals are used instead of thread locals
+//@ ignore-android does not use #[thread_local]
+//@ ignore-nto does not use #[thread_local]
+// Different number of duplicated diagnostics on different targets
 //@ compile-flags: -Zdeduplicate-diagnostics=yes
 
 #![allow(bare_trait_objects)]
@@ -20,31 +25,31 @@ pub union Qux<'t, 'k, I> {
 trait Tar<'t, 'k, I> {}
 
 thread_local! {
-    //~^ ERROR lifetime may not live long enough
-    //~| ERROR lifetime may not live long enough
+    //~^ ERROR borrowed data escapes outside of function
+    //~| ERROR borrowed data escapes outside of function
     static a: RefCell<HashMap<i32, Vec<Vec<Foo>>>> = RefCell::new(HashMap::new());
       //~^ ERROR missing lifetime specifiers
       //~| ERROR missing lifetime specifiers
 }
 thread_local! {
-    //~^ ERROR lifetime may not live long enough
-    //~| ERROR lifetime may not live long enough
-    //~| ERROR lifetime may not live long enough
+    //~^ ERROR borrowed data escapes outside of function
+    //~| ERROR borrowed data escapes outside of function
+    //~| ERROR borrowed data escapes outside of function
     static b: RefCell<HashMap<i32, Vec<Vec<&Bar>>>> = RefCell::new(HashMap::new());
       //~^ ERROR missing lifetime specifiers
       //~| ERROR missing lifetime specifiers
 }
 thread_local! {
-    //~^ ERROR lifetime may not live long enough
-    //~| ERROR lifetime may not live long enough
+    //~^ ERROR borrowed data escapes outside of function
+    //~| ERROR borrowed data escapes outside of function
     static c: RefCell<HashMap<i32, Vec<Vec<Qux<i32>>>>> = RefCell::new(HashMap::new());
     //~^ ERROR missing lifetime specifiers
     //~| ERROR missing lifetime specifiers
 }
 thread_local! {
-    //~^ ERROR lifetime may not live long enough
-    //~| ERROR lifetime may not live long enough
-    //~| ERROR lifetime may not live long enough
+    //~^ ERROR borrowed data escapes outside of function
+    //~| ERROR borrowed data escapes outside of function
+    //~| ERROR borrowed data escapes outside of function
     static d: RefCell<HashMap<i32, Vec<Vec<&Tar<i32>>>>> = RefCell::new(HashMap::new());
     //~^ ERROR missing lifetime specifiers
     //~| ERROR missing lifetime specifiers
diff --git a/tests/ui/suggestions/missing-lifetime-specifier.stderr b/tests/ui/suggestions/missing-lifetime-specifier.stderr
index 62eca16214871..04ac5778adfdd 100644
--- a/tests/ui/suggestions/missing-lifetime-specifier.stderr
+++ b/tests/ui/suggestions/missing-lifetime-specifier.stderr
@@ -1,5 +1,5 @@
 error[E0106]: missing lifetime specifiers
-  --> $DIR/missing-lifetime-specifier.rs:25:44
+  --> $DIR/missing-lifetime-specifier.rs:30:44
    |
 LL |     static a: RefCell<HashMap<i32, Vec<Vec<Foo>>>> = RefCell::new(HashMap::new());
    |                                            ^^^ expected 2 lifetime parameters
@@ -11,7 +11,7 @@ LL |     static a: RefCell<HashMap<i32, Vec<Vec<Foo<'static, 'static>>>>> = RefC
    |                                               ++++++++++++++++++
 
 error[E0106]: missing lifetime specifiers
-  --> $DIR/missing-lifetime-specifier.rs:25:44
+  --> $DIR/missing-lifetime-specifier.rs:30:44
    |
 LL | / thread_local! {
 LL | |
@@ -26,7 +26,7 @@ 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 specifiers
-  --> $DIR/missing-lifetime-specifier.rs:33:44
+  --> $DIR/missing-lifetime-specifier.rs:38:44
    |
 LL |     static b: RefCell<HashMap<i32, Vec<Vec<&Bar>>>> = RefCell::new(HashMap::new());
    |                                            ^^^^ expected 2 lifetime parameters
@@ -40,7 +40,7 @@ LL |     static b: RefCell<HashMap<i32, Vec<Vec<&'static Bar<'static, 'static>>>
    |                                             +++++++    ++++++++++++++++++
 
 error[E0106]: missing lifetime specifiers
-  --> $DIR/missing-lifetime-specifier.rs:33:44
+  --> $DIR/missing-lifetime-specifier.rs:38:44
    |
 LL | / thread_local! {
 LL | |
@@ -58,7 +58,7 @@ 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:40:47
+  --> $DIR/missing-lifetime-specifier.rs:45:47
    |
 LL |     static c: RefCell<HashMap<i32, Vec<Vec<Qux<i32>>>>> = RefCell::new(HashMap::new());
    |                                               ^ expected 2 lifetime parameters
@@ -70,7 +70,7 @@ LL |     static c: RefCell<HashMap<i32, Vec<Vec<Qux<'static, 'static, i32>>>>> =
    |                                                +++++++++++++++++
 
 error[E0106]: missing lifetime specifiers
-  --> $DIR/missing-lifetime-specifier.rs:40:47
+  --> $DIR/missing-lifetime-specifier.rs:45:47
    |
 LL | / thread_local! {
 LL | |
@@ -85,7 +85,7 @@ 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 specifiers
-  --> $DIR/missing-lifetime-specifier.rs:48:44
+  --> $DIR/missing-lifetime-specifier.rs:53:44
    |
 LL |     static d: RefCell<HashMap<i32, Vec<Vec<&Tar<i32>>>>> = RefCell::new(HashMap::new());
    |                                            ^   ^ expected 2 lifetime parameters
@@ -99,7 +99,7 @@ LL |     static d: RefCell<HashMap<i32, Vec<Vec<&'static Tar<'static, 'static, i
    |                                             +++++++     +++++++++++++++++
 
 error[E0106]: missing lifetime specifiers
-  --> $DIR/missing-lifetime-specifier.rs:48:44
+  --> $DIR/missing-lifetime-specifier.rs:53:44
    |
 LL | / thread_local! {
 LL | |
@@ -117,7 +117,7 @@ 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 specifier
-  --> $DIR/missing-lifetime-specifier.rs:58:44
+  --> $DIR/missing-lifetime-specifier.rs:63:44
    |
 LL |     static f: RefCell<HashMap<i32, Vec<Vec<&Tar<'static, i32>>>>> = RefCell::new(HashMap::new());
    |                                            ^ expected named lifetime parameter
@@ -129,7 +129,7 @@ LL |     static f: RefCell<HashMap<i32, Vec<Vec<&'static Tar<'static, i32>>>>> =
    |                                             +++++++
 
 error[E0106]: missing lifetime specifier
-  --> $DIR/missing-lifetime-specifier.rs:58:44
+  --> $DIR/missing-lifetime-specifier.rs:63:44
    |
 LL | / thread_local! {
 LL | |     static f: RefCell<HashMap<i32, Vec<Vec<&Tar<'static, i32>>>>> = RefCell::new(HashMap::new());
@@ -143,7 +143,7 @@ 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]: union takes 2 lifetime arguments but 1 lifetime argument was supplied
-  --> $DIR/missing-lifetime-specifier.rs:54:44
+  --> $DIR/missing-lifetime-specifier.rs:59:44
    |
 LL |     static e: RefCell<HashMap<i32, Vec<Vec<Qux<'static, i32>>>>> = RefCell::new(HashMap::new());
    |                                            ^^^ ------- supplied 1 lifetime argument
@@ -151,7 +151,7 @@ LL |     static e: RefCell<HashMap<i32, Vec<Vec<Qux<'static, i32>>>>> = RefCell:
    |                                            expected 2 lifetime arguments
    |
 note: union defined here, with 2 lifetime parameters: `'t`, `'k`
-  --> $DIR/missing-lifetime-specifier.rs:16:11
+  --> $DIR/missing-lifetime-specifier.rs:21:11
    |
 LL | pub union Qux<'t, 'k, I> {
    |           ^^^ --  --
@@ -161,7 +161,7 @@ LL |     static e: RefCell<HashMap<i32, Vec<Vec<Qux<'static, 'static, i32>>>>> =
    |                                                       +++++++++
 
 error[E0107]: trait takes 2 lifetime arguments but 1 lifetime argument was supplied
-  --> $DIR/missing-lifetime-specifier.rs:58:45
+  --> $DIR/missing-lifetime-specifier.rs:63:45
    |
 LL |     static f: RefCell<HashMap<i32, Vec<Vec<&Tar<'static, i32>>>>> = RefCell::new(HashMap::new());
    |                                             ^^^ ------- supplied 1 lifetime argument
@@ -169,7 +169,7 @@ LL |     static f: RefCell<HashMap<i32, Vec<Vec<&Tar<'static, i32>>>>> = RefCell
    |                                             expected 2 lifetime arguments
    |
 note: trait defined here, with 2 lifetime parameters: `'t`, `'k`
-  --> $DIR/missing-lifetime-specifier.rs:20:7
+  --> $DIR/missing-lifetime-specifier.rs:25:7
    |
 LL | trait Tar<'t, 'k, I> {}
    |       ^^^ --  --
@@ -178,8 +178,8 @@ help: add missing lifetime argument
 LL |     static f: RefCell<HashMap<i32, Vec<Vec<&Tar<'static, 'static, i32>>>>> = RefCell::new(HashMap::new());
    |                                                        +++++++++
 
-error: lifetime may not live long enough
-  --> $DIR/missing-lifetime-specifier.rs:22:1
+error[E0521]: borrowed data escapes outside of function
+  --> $DIR/missing-lifetime-specifier.rs:27:1
    |
 LL | / thread_local! {
 LL | |
@@ -190,13 +190,15 @@ LL | |
 LL | | }
    | | ^
    | | |
+   | | `init` is a reference that is only valid in the function body
+   | | `init` escapes the function body here
    | |_has type `Option<&mut Option<RefCell<HashMap<i32, Vec<Vec<Foo<'1, '_>>>>>>>`
-   |   returning this value requires that `'1` must outlive `'static`
+   |   argument requires that `'1` must outlive `'static`
    |
    = 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: lifetime may not live long enough
-  --> $DIR/missing-lifetime-specifier.rs:22:1
+error[E0521]: borrowed data escapes outside of function
+  --> $DIR/missing-lifetime-specifier.rs:27:1
    |
 LL | / thread_local! {
 LL | |
@@ -207,13 +209,15 @@ LL | |
 LL | | }
    | | ^
    | | |
+   | | `init` is a reference that is only valid in the function body
+   | | `init` escapes the function body here
    | |_has type `Option<&mut Option<RefCell<HashMap<i32, Vec<Vec<Foo<'_, '2>>>>>>>`
-   |   returning this value requires that `'2` must outlive `'static`
+   |   argument requires that `'2` must outlive `'static`
    |
    = 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: lifetime may not live long enough
-  --> $DIR/missing-lifetime-specifier.rs:29:1
+error[E0521]: borrowed data escapes outside of function
+  --> $DIR/missing-lifetime-specifier.rs:34:1
    |
 LL | / thread_local! {
 LL | |
@@ -224,16 +228,16 @@ LL | |     static b: RefCell<HashMap<i32, Vec<Vec<&Bar>>>> = RefCell::new(HashMa
 LL | |
 LL | |
 LL | | }
-   | |_^ returning this value requires that `'1` must outlive `'static`
+   | | ^
+   | | |
+   | | `init` is a reference that is only valid in the function body
+   | |_`init` escapes the function body here
+   |   argument requires that `'1` must outlive `'static`
    |
    = 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)
-help: to declare that the trait object captures data from argument `init`, you can add an explicit `'_` lifetime bound
-   |
-LL |     static b: RefCell<HashMap<i32, Vec<Vec<&Bar + '_>>>> = RefCell::new(HashMap::new());
-   |                                                 ++++
 
-error: lifetime may not live long enough
-  --> $DIR/missing-lifetime-specifier.rs:29:1
+error[E0521]: borrowed data escapes outside of function
+  --> $DIR/missing-lifetime-specifier.rs:34:1
    |
 LL | / thread_local! {
 LL | |
@@ -244,17 +248,15 @@ LL | |
 LL | | }
    | | ^
    | | |
+   | | `init` is a reference that is only valid in the function body
+   | | `init` escapes the function body here
    | |_has type `Option<&mut Option<RefCell<HashMap<i32, Vec<Vec<&dyn Bar<'2, '_>>>>>>>`
-   |   returning this value requires that `'2` must outlive `'static`
+   |   argument requires that `'2` must outlive `'static`
    |
    = 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)
-help: to declare that the trait object captures data from argument `init`, you can add an explicit `'_` lifetime bound
-   |
-LL |     static b: RefCell<HashMap<i32, Vec<Vec<&Bar + '_>>>> = RefCell::new(HashMap::new());
-   |                                                 ++++
 
-error: lifetime may not live long enough
-  --> $DIR/missing-lifetime-specifier.rs:29:1
+error[E0521]: borrowed data escapes outside of function
+  --> $DIR/missing-lifetime-specifier.rs:34:1
    |
 LL | / thread_local! {
 LL | |
@@ -265,17 +267,15 @@ LL | |
 LL | | }
    | | ^
    | | |
+   | | `init` is a reference that is only valid in the function body
+   | | `init` escapes the function body here
    | |_has type `Option<&mut Option<RefCell<HashMap<i32, Vec<Vec<&dyn Bar<'_, '3>>>>>>>`
-   |   returning this value requires that `'3` must outlive `'static`
+   |   argument requires that `'3` must outlive `'static`
    |
    = 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)
-help: to declare that the trait object captures data from argument `init`, you can add an explicit `'_` lifetime bound
-   |
-LL |     static b: RefCell<HashMap<i32, Vec<Vec<&Bar + '_>>>> = RefCell::new(HashMap::new());
-   |                                                 ++++
 
-error: lifetime may not live long enough
-  --> $DIR/missing-lifetime-specifier.rs:37:1
+error[E0521]: borrowed data escapes outside of function
+  --> $DIR/missing-lifetime-specifier.rs:42:1
    |
 LL | / thread_local! {
 LL | |
@@ -286,13 +286,15 @@ LL | |
 LL | | }
    | | ^
    | | |
+   | | `init` is a reference that is only valid in the function body
+   | | `init` escapes the function body here
    | |_has type `Option<&mut Option<RefCell<HashMap<i32, Vec<Vec<Qux<'1, '_, i32>>>>>>>`
-   |   returning this value requires that `'1` must outlive `'static`
+   |   argument requires that `'1` must outlive `'static`
    |
    = 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: lifetime may not live long enough
-  --> $DIR/missing-lifetime-specifier.rs:37:1
+error[E0521]: borrowed data escapes outside of function
+  --> $DIR/missing-lifetime-specifier.rs:42:1
    |
 LL | / thread_local! {
 LL | |
@@ -303,13 +305,15 @@ LL | |
 LL | | }
    | | ^
    | | |
+   | | `init` is a reference that is only valid in the function body
+   | | `init` escapes the function body here
    | |_has type `Option<&mut Option<RefCell<HashMap<i32, Vec<Vec<Qux<'_, '2, i32>>>>>>>`
-   |   returning this value requires that `'2` must outlive `'static`
+   |   argument requires that `'2` must outlive `'static`
    |
    = 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: lifetime may not live long enough
-  --> $DIR/missing-lifetime-specifier.rs:44:1
+error[E0521]: borrowed data escapes outside of function
+  --> $DIR/missing-lifetime-specifier.rs:49:1
    |
 LL | / thread_local! {
 LL | |
@@ -320,16 +324,16 @@ LL | |     static d: RefCell<HashMap<i32, Vec<Vec<&Tar<i32>>>>> = RefCell::new(H
 LL | |
 LL | |
 LL | | }
-   | |_^ returning this value requires that `'1` must outlive `'static`
+   | | ^
+   | | |
+   | | `init` is a reference that is only valid in the function body
+   | |_`init` escapes the function body here
+   |   argument requires that `'1` must outlive `'static`
    |
    = 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)
-help: to declare that the trait object captures data from argument `init`, you can add an explicit `'_` lifetime bound
-   |
-LL |     static d: RefCell<HashMap<i32, Vec<Vec<&Tar<i32> + '_>>>> = RefCell::new(HashMap::new());
-   |                                                      ++++
 
-error: lifetime may not live long enough
-  --> $DIR/missing-lifetime-specifier.rs:44:1
+error[E0521]: borrowed data escapes outside of function
+  --> $DIR/missing-lifetime-specifier.rs:49:1
    |
 LL | / thread_local! {
 LL | |
@@ -340,17 +344,15 @@ LL | |
 LL | | }
    | | ^
    | | |
+   | | `init` is a reference that is only valid in the function body
+   | | `init` escapes the function body here
    | |_has type `Option<&mut Option<RefCell<HashMap<i32, Vec<Vec<&dyn Tar<'2, '_, i32>>>>>>>`
-   |   returning this value requires that `'2` must outlive `'static`
+   |   argument requires that `'2` must outlive `'static`
    |
    = 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)
-help: to declare that the trait object captures data from argument `init`, you can add an explicit `'_` lifetime bound
-   |
-LL |     static d: RefCell<HashMap<i32, Vec<Vec<&Tar<i32> + '_>>>> = RefCell::new(HashMap::new());
-   |                                                      ++++
 
-error: lifetime may not live long enough
-  --> $DIR/missing-lifetime-specifier.rs:44:1
+error[E0521]: borrowed data escapes outside of function
+  --> $DIR/missing-lifetime-specifier.rs:49:1
    |
 LL | / thread_local! {
 LL | |
@@ -361,16 +363,14 @@ LL | |
 LL | | }
    | | ^
    | | |
+   | | `init` is a reference that is only valid in the function body
+   | | `init` escapes the function body here
    | |_has type `Option<&mut Option<RefCell<HashMap<i32, Vec<Vec<&dyn Tar<'_, '3, i32>>>>>>>`
-   |   returning this value requires that `'3` must outlive `'static`
+   |   argument requires that `'3` must outlive `'static`
    |
    = 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)
-help: to declare that the trait object captures data from argument `init`, you can add an explicit `'_` lifetime bound
-   |
-LL |     static d: RefCell<HashMap<i32, Vec<Vec<&Tar<i32> + '_>>>> = RefCell::new(HashMap::new());
-   |                                                      ++++
 
 error: aborting due to 22 previous errors
 
-Some errors have detailed explanations: E0106, E0107.
+Some errors have detailed explanations: E0106, E0107, E0521.
 For more information about an error, try `rustc --explain E0106`.