std::thread::LocalKeyState: Update compile and run tests

Author: joshlf

src/libstd/thread/local.rs

@@ -192,7 +192,7 @@ macro_rules! __thread_local_inner {
             static __KEY: $crate::thread::__OsLocalKeyInner<$t> =
                 $crate::thread::__OsLocalKeyInner::new();
 
-            unsafe fn __getit() -> &'static Option<$t> { __KEY.get() }
+            unsafe fn __getit() -> &'static $crate::option::Option<$t> { __KEY.get() }
 
             unsafe fn __get_state() -> $crate::thread::LocalKeyState { __KEY.get_state() }
 
@@ -504,39 +504,33 @@ pub mod fast {
             }
         }
 
-        pub fn get(&self) -> &'static Option<T> {
-            unsafe { &*self.inner.get() }
+        pub unsafe fn get(&self) -> &'static Option<T> {
+            &*self.inner.get()
         }
 
-        pub fn get_state(&self) -> LocalKeyState {
-            unsafe { *self.state.get() }
+        pub unsafe fn get_state(&self) -> LocalKeyState {
+            *self.state.get()
         }
 
-        pub fn pre_init(&self) {
-            unsafe {
-                // It's critical that we set the state to Initializing before
-                // registering destructors - if registering destructors causes
-                // allocation, and the global allocator uses TLS, then the
-                // allocator needs to be able to detect that the TLS is in
-                // the Initializing state and perform appropriate fallback
-                // logic rather than recursing infinitely.
-                *self.state.get() = LocalKeyState::Initializing;
-                self.register_dtor();
-            }
+        pub unsafe fn pre_init(&self) {
+            // It's critical that we set the state to Initializing before
+            // registering destructors - if registering destructors causes
+            // allocation, and the global allocator uses TLS, then the
+            // allocator needs to be able to detect that the TLS is in
+            // the Initializing state and perform appropriate fallback
+            // logic rather than recursing infinitely.
+            *self.state.get() = LocalKeyState::Initializing;
+            self.register_dtor();
         }
 
-        pub fn post_init(&self, val: T) {
-            unsafe {
-                *self.inner.get() = Some(val);
-                *self.state.get() = LocalKeyState::Valid;
-            }
+        pub unsafe fn post_init(&self, val: T) {
+            *self.inner.get() = Some(val);
+            *self.state.get() = LocalKeyState::Valid;
         }
 
-        pub fn rollback_init(&self) {
-            unsafe {
-                *self.inner.get() = None;
-                *self.state.get() = LocalKeyState::Uninitialized;
-            }
+        pub unsafe fn rollback_init(&self) {
+            *self.inner.get() = None;
+            *self.state.get() = LocalKeyState::Uninitialized;
         }
 
         unsafe fn register_dtor(&self) {
@@ -605,54 +599,46 @@ pub mod os {
             }
         }
 
-        pub fn get(&self) -> &'static Option<T> {
-            unsafe {
-                match *self.state.get() {
-                    LocalKeyState::Valid => {
-                        // Since the state is Valid, we know that os points to
-                        // an allocated Value<T>.
-                        let ptr = self.os.get() as *mut Value<T>;
-                        debug_assert!(!ptr.is_null());
-                        &*(*ptr).value.get()
-                    }
-                    _ => {
-                        // The dummy_value is guaranteed to always be None. This
-                        // allows us to avoid allocating if the state isn't Valid.
-                        // This is critical because it means that an allocator can
-                        // call try_with and detect that the key is in state
-                        // Initializing without recursing infinitely.
-                        &*self.dummy_value.get()
-                    }
+        pub unsafe fn get(&self) -> &'static Option<T> {
+            match *self.state.get() {
+                LocalKeyState::Valid => {
+                    // Since the state is Valid, we know that os points to
+                    // an allocated Value<T>.
+                    let ptr = self.os.get() as *mut Value<T>;
+                    debug_assert!(!ptr.is_null());
+                    &*(*ptr).value.get()
+                }
+                _ => {
+                    // The dummy_value is guaranteed to always be None. This
+                    // allows us to avoid allocating if the state isn't Valid.
+                    // This is critical because it means that an allocator can
+                    // call try_with and detect that the key is in state
+                    // Initializing without recursing infinitely.
+                    &*self.dummy_value.get()
                 }
             }
         }
 
-        pub fn get_state(&self) -> LocalKeyState {
-            unsafe { *self.state.get() }
+        pub unsafe fn get_state(&self) -> LocalKeyState {
+            *self.state.get()
         }
 
-        pub fn pre_init(&self) {
-            unsafe {
-                *self.state.get() = LocalKeyState::Initializing;
-            }
+        pub unsafe fn pre_init(&self) {
+            *self.state.get() = LocalKeyState::Initializing;
         }
 
-        pub fn rollback_init(&self) {
-            unsafe {
-                *self.state.get() = LocalKeyState::Uninitialized;
-            }
+        pub unsafe fn rollback_init(&self) {
+            *self.state.get() = LocalKeyState::Uninitialized;
         }
 
-        pub fn post_init(&self, val: T) {
-            unsafe {
-                let ptr: Box<Value<T>> = box Value {
-                                                 key: &*(self as *const _),
-                                                 value: UnsafeCell::new(Some(val)),
-                                             };
-                let ptr = Box::into_raw(ptr);
-                self.os.set(ptr as *mut u8);
-                *self.state.get() = LocalKeyState::Valid;
-            }
+        pub unsafe fn post_init(&self, val: T) {
+            let ptr: Box<Value<T>> = box Value {
+                                             key: &*(self as *const _),
+                                             value: UnsafeCell::new(Some(val)),
+                                         };
+            let ptr = Box::into_raw(ptr);
+            self.os.set(ptr as *mut u8);
+            *self.state.get() = LocalKeyState::Valid;
         }
     }
 

src/test/compile-fail/issue-43733-2.rs

@@ -16,6 +16,7 @@
 #[cfg(not(target_thread_local))]
 struct Key<T> {
     _data: std::cell::UnsafeCell<Option<T>>,
+    _state: std::cell::UnsafeCell<std::thread::LocalKeyState>,
     _flag: std::cell::Cell<bool>,
 }
 
@@ -34,6 +35,7 @@ use std::thread::__FastLocalKeyInner as Key;
 
 static __KEY: Key<()> = Key::new();
 //~^ ERROR `std::cell::UnsafeCell<std::option::Option<()>>: std::marker::Sync` is not satisfied
+//~| ERROR `std::cell::UnsafeCell<std::thread::LocalKeyState>: std::marker::Sync` is not satisfied
 //~| ERROR `std::cell::Cell<bool>: std::marker::Sync` is not satisfied
 
 fn main() {}

src/test/compile-fail/issue-43733.rs

@@ -9,28 +9,44 @@
 // except according to those terms.
 
 #![feature(const_fn, drop_types_in_const)]
-#![feature(cfg_target_thread_local, thread_local_internals)]
+#![feature(cfg_target_thread_local, thread_local_internals, thread_local_state)]
 
 type Foo = std::cell::RefCell<String>;
 
-#[cfg(target_thread_local)]
 static __KEY: std::thread::__FastLocalKeyInner<Foo> =
     std::thread::__FastLocalKeyInner::new();
 
 #[cfg(not(target_thread_local))]
 static __KEY: std::thread::__OsLocalKeyInner<Foo> =
     std::thread::__OsLocalKeyInner::new();
 
-fn __getit() -> std::option::Option<
-    &'static std::cell::UnsafeCell<
-        std::option::Option<Foo>>>
-{
+fn __getit() -> &'static std::option::Option<Foo> {
     __KEY.get() //~ ERROR  invocation of unsafe method requires unsafe
 }
 
+fn __get_state() -> std::thread::LocalKeyState {
+    __KEY.get_state() //~ ERROR  invocation of unsafe method requires unsafe
+}
+
+fn __pre_init() {
+    __KEY.pre_init() //~ ERROR  invocation of unsafe method requires unsafe
+}
+
+fn __post_init(val: Foo) {
+    __KEY.post_init(val) //~ ERROR  invocation of unsafe method requires unsafe
+}
+
+fn __rollback_init() {
+    __KEY.rollback_init() //~ ERROR  invocation of unsafe method requires unsafe
+}
+
 static FOO: std::thread::LocalKey<Foo> =
-    std::thread::LocalKey::new(__getit, Default::default);
-//~^ ERROR call to unsafe function requires unsafe
+    std::thread::LocalKey::new(__getit, //~ ERROR call to unsafe function requires unsafe
+                               __get_state,
+                               __pre_init,
+                               Default::default,
+                               __post_init,
+                               __rollback_init);
 
 fn main() {
     FOO.with(|foo| println!("{}", foo.borrow()));

src/test/run-fail/tls-init-on-init.rs

@@ -0,0 +1,35 @@
+// Copyright 2015 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+// ignore-emscripten no threads support
+
+// Can't include entire panic message because it'd exceed tidy's 100-character line length limit.
+// error-pattern:cannot access a TLS value while it is being initialized or during or after
+
+#![feature(thread_local_state)]
+
+use std::thread::{self, LocalKeyState};
+
+struct Foo;
+
+thread_local!(static FOO: Foo = Foo::init());
+
+impl Foo {
+    fn init() -> Foo {
+        FOO.with(|_| {});
+        Foo
+    }
+}
+
+fn main() {
+    thread::spawn(|| {
+        FOO.with(|_| {});
+    }).join().unwrap();
+}