From 7b2320c3df9e57e8920a8eeec94e907e3d3e6347 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E8=AE=B8=E6=9D=B0=E5=8F=8B=20Jieyou=20Xu=20=28Joe=29?= <39484203+jieyouxu@users.noreply.github.com> Date: Fri, 18 Oct 2024 10:12:11 +0800 Subject: [PATCH] Avoid shadowing user provided types or type aliases in `thread_local!` By using qualified imports, i.e. `$crate::...::LocalKey`. --- .../std/src/sys/thread_local/native/mod.rs | 31 +++++++++---------- library/std/src/sys/thread_local/os.rs | 17 ++++++---- library/std/tests/thread.rs | 23 ++++++++++++++ 3 files changed, 49 insertions(+), 22 deletions(-) diff --git a/library/std/src/sys/thread_local/native/mod.rs b/library/std/src/sys/thread_local/native/mod.rs index f498dee0899f9..a5dffe3c45883 100644 --- a/library/std/src/sys/thread_local/native/mod.rs +++ b/library/std/src/sys/thread_local/native/mod.rs @@ -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 { @@ -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) } } diff --git a/library/std/src/sys/thread_local/os.rs b/library/std/src/sys/thread_local/os.rs index 26ce3322a16e3..f5a2aaa6c6a3f 100644 --- a/library/std/src/sys/thread_local/os.rs +++ b/library/std/src/sys/thread_local/os.rs @@ -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) }) } diff --git a/library/std/tests/thread.rs b/library/std/tests/thread.rs index 83574176186a4..1bb17d149fa10 100644 --- a/library/std/tests/thread.rs +++ b/library/std/tests/thread.rs @@ -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 + // . + // + // 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(