Skip to content

Commit fdaa765

Browse files
committed
Auto merge of rust-lang#84876 - alexcrichton:inline-thread-locals-cross-crate, r=Mark-Simulacrum
std: Attempt again to inline thread-local-init across crates Issue rust-lang#25088 has been part of `thread_local!` for quite some time now. Historical attempts have been made to add `#[inline]` to `__getit` in rust-lang#43931, rust-lang#50252, and rust-lang#59720, but these attempts ended up not landing at the time due to segfaults on Windows. In the interim though with `const`-initialized thread locals AFAIK this is the only remaining bug which is why you might want to use `#[thread_local]` over `thread_local!`. As a result I figured it was time to resubmit this and see how it fares on CI and if I can help debugging any issues that crop up. Closes rust-lang#25088
2 parents b573df7 + a8cdbd2 commit fdaa765

File tree

1 file changed

+24
-0
lines changed

1 file changed

+24
-0
lines changed

std/src/thread/local.rs

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -162,6 +162,7 @@ macro_rules! thread_local {
162162
macro_rules! __thread_local_inner {
163163
// used to generate the `LocalKey` value for const-initialized thread locals
164164
(@key $t:ty, const $init:expr) => {{
165+
#[cfg_attr(not(target_env = "msvc"), inline)] // see comments below
165166
unsafe fn __getit() -> $crate::option::Option<&'static $t> {
166167
const _REQUIRE_UNSTABLE: () = $crate::thread::require_unstable_const_init_thread_local();
167168

@@ -260,6 +261,29 @@ macro_rules! __thread_local_inner {
260261
#[inline]
261262
fn __init() -> $t { $init }
262263

264+
// When reading this function you might ask "why is this inlined
265+
// everywhere other than MSVC?", and that's a very reasonable
266+
// question to ask. The short story is that it segfaults rustc if
267+
// this function is inlined. The longer story is that MSVC looks to
268+
// not support `extern` references to thread locals across DLL
269+
// boundaries. This appears to at least not be supported in the ABI
270+
// that LLVM implements.
271+
//
272+
// Because of this we never inline on MVSC, but we do inline on
273+
// other platforms (where external references to thread locals
274+
// across DLLs are supported). A better fix for this would be to
275+
// inline this function on MSVC, but only for "statically linked"
276+
// components. For example if two separately compiled rlibs end up
277+
// getting linked into a DLL then it's fine to inline this function
278+
// across that boundary. It's only not fine to inline this function
279+
// across a DLL boundary. Unfortunately rustc doesn't currently have
280+
// this sort of logic available in an attribute, and it's not clear
281+
// that rustc is even equipped to answer this (it's more of a Cargo
282+
// question kinda). This means that, unfortunately, MSVC gets the
283+
// pessimistic path for now where it's never inlined.
284+
//
285+
// The issue of "should enable on MSVC sometimes" is #84933
286+
#[cfg_attr(not(target_env = "msvc"), inline)]
263287
unsafe fn __getit() -> $crate::option::Option<&'static $t> {
264288
#[cfg(all(target_arch = "wasm32", not(target_feature = "atomics")))]
265289
static __KEY: $crate::thread::__StaticLocalKeyInner<$t> =

0 commit comments

Comments
 (0)