Skip to content

Commit

Permalink
Rollup merge of #131866 - jieyouxu:thread_local, r=jhpratt
Browse files Browse the repository at this point in the history
Avoid use imports in `thread_local_inner!`

Previously, the use imports in `thread_local_inner!` can shadow user-provided types or type aliases of the names `Storage`, `EagerStorage`, `LocalStorage` and `LocalKey`. This PR fixes that by dropping the use imports and instead refer to the std-internal types via fully qualified paths. A basic test is added to ensure `thread_local!`s with static decls with type names that match the aforementioned std-internal type names can successfully compile.

Fixes #131863.
  • Loading branch information
jieyouxu authored Oct 18, 2024
2 parents 64bf99b + 7b2320c commit af85d52
Show file tree
Hide file tree
Showing 3 changed files with 49 additions and 22 deletions.
31 changes: 15 additions & 16 deletions library/std/src/sys/thread_local/native/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,20 +49,21 @@ pub use lazy::Storage as LazyStorage;
#[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
// NOTE: we cannot import `LocalKey`, `LazyStorage` or `EagerStorage` with a `use` because that
// can shadow user provided type or type alias with a matching name. Please update the shadowing
// test in `tests/thread.rs` if these types are renamed.

// Used to generate the `LocalKey` value for const-initialized thread locals.
(@key $t:ty, const $init:expr) => {{
const __INIT: $t = $init;

unsafe {
use $crate::mem::needs_drop;
use $crate::thread::LocalKey;
use $crate::thread::local_impl::EagerStorage;

LocalKey::new(const {
if needs_drop::<$t>() {
$crate::thread::LocalKey::new(const {
if $crate::mem::needs_drop::<$t>() {
|_| {
#[thread_local]
static VAL: EagerStorage<$t> = EagerStorage::new(__INIT);
static VAL: $crate::thread::local_impl::EagerStorage<$t>
= $crate::thread::local_impl::EagerStorage::new(__INIT);
VAL.get()
}
} else {
Expand All @@ -84,21 +85,19 @@ pub macro thread_local_inner {
}

unsafe {
use $crate::mem::needs_drop;
use $crate::thread::LocalKey;
use $crate::thread::local_impl::LazyStorage;

LocalKey::new(const {
if needs_drop::<$t>() {
$crate::thread::LocalKey::new(const {
if $crate::mem::needs_drop::<$t>() {
|init| {
#[thread_local]
static VAL: LazyStorage<$t, ()> = LazyStorage::new();
static VAL: $crate::thread::local_impl::LazyStorage<$t, ()>
= $crate::thread::local_impl::LazyStorage::new();
VAL.get_or_init(init, __init)
}
} else {
|init| {
#[thread_local]
static VAL: LazyStorage<$t, !> = LazyStorage::new();
static VAL: $crate::thread::local_impl::LazyStorage<$t, !>
= $crate::thread::local_impl::LazyStorage::new();
VAL.get_or_init(init, __init)
}
}
Expand Down
17 changes: 11 additions & 6 deletions library/std/src/sys/thread_local/os.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,19 +15,24 @@ pub macro thread_local_inner {
$crate::thread::local_impl::thread_local_inner!(@key $t, { const INIT_EXPR: $t = $init; INIT_EXPR })
},

// used to generate the `LocalKey` value for `thread_local!`
// NOTE: we cannot import `Storage` or `LocalKey` with a `use` because that can shadow user
// provided type or type alias with a matching name. Please update the shadowing test in
// `tests/thread.rs` if these types are renamed.

// used to generate the `LocalKey` value for `thread_local!`.
(@key $t:ty, $init:expr) => {{
#[inline]
fn __init() -> $t { $init }

// NOTE: this cannot import `LocalKey` or `Storage` with a `use` because that can shadow
// user provided type or type alias with a matching name. Please update the shadowing test
// in `tests/thread.rs` if these types are renamed.
unsafe {
use $crate::thread::LocalKey;
use $crate::thread::local_impl::Storage;

// Inlining does not work on windows-gnu due to linking errors around
// dllimports. See https://github.com/rust-lang/rust/issues/109797.
LocalKey::new(#[cfg_attr(windows, inline(never))] |init| {
static VAL: Storage<$t> = Storage::new();
$crate::thread::LocalKey::new(#[cfg_attr(windows, inline(never))] |init| {
static VAL: $crate::thread::local_impl::Storage<$t>
= $crate::thread::local_impl::Storage::new();
VAL.get(init, __init)
})
}
Expand Down
23 changes: 23 additions & 0 deletions library/std/tests/thread.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,29 @@ fn thread_local_containing_const_statements() {
assert_eq!(REFCELL.take(), 1);
}

#[test]
fn thread_local_hygeiene() {
// Previously `thread_local_inner!` had use imports for `LocalKey`, `Storage`, `EagerStorage`
// and `LazyStorage`. The use imports will shadow a user-provided type or type alias if the
// user-provided type or type alias has the same name. Make sure that this does not happen. See
// <https://github.com/rust-lang/rust/issues/131863>.
//
// NOTE: if the internal implementation details change (i.e. get renamed), this test should be
// updated.

#![allow(dead_code)]
type LocalKey = ();
type Storage = ();
type LazyStorage = ();
type EagerStorage = ();
thread_local! {
static A: LocalKey = const { () };
static B: Storage = const { () };
static C: LazyStorage = const { () };
static D: EagerStorage = const { () };
}
}

#[test]
// Include an ignore list on purpose, so that new platforms don't miss it
#[cfg_attr(
Expand Down

0 comments on commit af85d52

Please sign in to comment.