Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Hygiene issues with use declarations in thread_local! implementation #131863

Closed
Timbals opened this issue Oct 18, 2024 · 2 comments · Fixed by #131866 or #132101
Closed

Hygiene issues with use declarations in thread_local! implementation #131863

Timbals opened this issue Oct 18, 2024 · 2 comments · Fixed by #131866 or #132101
Labels
C-bug Category: This is a bug. regression-from-stable-to-stable Performance or correctness regression from one stable version to another. T-libs Relevant to the library team, which will review and decide on the PR/issue.

Comments

@Timbals
Copy link

Timbals commented Oct 18, 2024

use std::cell::RefCell;

struct Dom;

type Storage<T> = RefCell<Option<T>>;

thread_local! {
    static CURRENT_DOM: Storage<Dom> = const { RefCell::new(None) };
}

The above code worked in 1.80.1 but fails to compile with 1.81.0 on platforms without cfg(target_thread_local). In my case this was aarch64-linux-android.

The issue seems to be that the macro expansion contains a use declaration that pulls in another type called Storage into the scope:

use $crate::thread::LocalKey;
use $crate::thread::local_impl::Storage;

This changed recently in #126953.

A similar change was made to the implementation for platforms with cfg(target_thread_local) in #125525:

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

Replacing Storage with EagerStorage in the example produces a similar regression for 1.79.0 -> 1.80.0.

The example is from yakui.

Version it worked on

It most recently worked on: 1.80.1

Version with regression

rustc --version --verbose:

rustc 1.81.0 (eeb90cda1 2024-09-04)
binary: rustc
commit-hash: eeb90cda1969383f56a2637cbd3037bdf598841c
commit-date: 2024-09-04
host: x86_64-pc-windows-msvc
release: 1.81.0
LLVM version: 18.1.7

Backtrace

Backtrace

error[E0658]: use of unstable library feature 'thread_local_internals': internal details of the thread_local macro
 --> src\lib.rs:8:25
  |
8 |     static CURRENT_DOM: Storage<Dom> = const { RefCell::new(None) };
  |                         ^^^^^^^^^^^^

error[E0271]: expected `__init` to be a fn item that returns `Storage<Dom>`, but it returns `RefCell<Option<Dom>>`
 --> src\lib.rs:7:1
  |
7 | / thread_local! {
8 | |     static CURRENT_DOM: Storage<Dom> = const { RefCell::new(None) };
9 | | }
  | | ^
  | | |
  | |_expected `Storage<Dom>`, found `RefCell<Option<Dom>>`
  |   required by a bound introduced by this call
  |
  = note: expected struct `std::thread::local_impl::Storage<Dom>`
             found struct `RefCell<Option<Dom>>`
note: required by a bound in `std::thread::local_impl::Storage::<T>::get`
 --> /rustc/eeb90cda1969383f56a2637cbd3037bdf598841c/library/std/src/sys/thread_local/os.rs:69:5
  = 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[E0308]: mismatched types
 --> src\lib.rs:7:1
  |
7 | / thread_local! {
8 | |     static CURRENT_DOM: Storage<Dom> = const { RefCell::new(None) };
9 | | }
  | | ^
  | | |
  | |_expected `Option<&mut Option<Storage<Dom>>>`, found `Option<&mut Option<RefCell<...>>>`
  |   arguments to this method are incorrect
  |
  = note: expected enum `Option<&mut Option<std::thread::local_impl::Storage<Dom>>>`
             found enum `Option<&mut Option<RefCell<Option<Dom>>>>`
note: method defined here
 --> /rustc/eeb90cda1969383f56a2637cbd3037bdf598841c/library/std/src/sys/thread_local/os.rs:69:12
  = 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[E0308]: mismatched types
 --> src\lib.rs:7:1
  |
7 | / thread_local! {
8 | |     static CURRENT_DOM: Storage<Dom> = const { RefCell::new(None) };
9 | | }
  | |_^ expected `*const RefCell<Option<Dom>>`, found `*const Storage<Dom>`
  |
  = note: expected raw pointer `*const RefCell<Option<Dom>>`
             found raw pointer `*const std::thread::local_impl::Storage<Dom>`
  = 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)

Some errors have detailed explanations: E0271, E0308, E0658.
For more information about an error, try `rustc --explain E0271`.
error: could not compile `minimal` (lib) due to 4 previous errors

@rustbot modify labels: +regression-from-stable-to-stable -regression-untriaged

@Timbals Timbals added C-bug Category: This is a bug. regression-untriaged Untriaged performance or correctness regression. labels Oct 18, 2024
@rustbot rustbot added I-prioritize Issue: Indicates that prioritization has been requested for this issue. needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. regression-from-stable-to-stable Performance or correctness regression from one stable version to another. and removed regression-untriaged Untriaged performance or correctness regression. labels Oct 18, 2024
@jieyouxu jieyouxu added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue. and removed T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Oct 18, 2024
@jieyouxu jieyouxu removed the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Oct 18, 2024
@jhpratt jhpratt added the beta-nominated Nominated for backporting to the compiler in the beta channel. label Oct 18, 2024
@jhpratt
Copy link
Member

jhpratt commented Oct 18, 2024

Nominating for backport to beta as this is a regression. I think it's uncommon enough that a backport to stable isn't necessary, though that's up to the team.

jhpratt added a commit to jhpratt/rust that referenced this issue Oct 18, 2024
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 rust-lang#131863.
@jieyouxu
Copy link
Member

(@jhpratt usually I think we add the beta nomination label on the PR itself not the issue because we might possibly get more than one backlinked PRs, not relevant in this case anyway)

@Noratrieb Noratrieb removed the beta-nominated Nominated for backporting to the compiler in the beta channel. label Oct 18, 2024
jhpratt added a commit to jhpratt/rust that referenced this issue Oct 18, 2024
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 rust-lang#131863.
jhpratt added a commit to jhpratt/rust that referenced this issue Oct 18, 2024
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 rust-lang#131863.
@apiraino apiraino removed the I-prioritize Issue: Indicates that prioritization has been requested for this issue. label Oct 18, 2024
@bors bors closed this as completed in af85d52 Oct 18, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Oct 18, 2024
Rollup merge of rust-lang#131866 - jieyouxu:thread_local, r=jhpratt

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 rust-lang#131863.
youknowone added a commit to youknowone/rust that referenced this issue Oct 24, 2024
Fixes rust-lang#131863 for wasm targets

All other macros were done in rust-lang#131866, but this sub module is missed.
workingjubilee added a commit to workingjubilee/rustc that referenced this issue Oct 24, 2024
…r=tgross35

Avoid using imports in thread_local_inner! in static

Fixes rust-lang#131863 for wasm targets

All other macros were done in rust-lang#131866, but this sub module is missed.

r? `@jieyouxu`
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Oct 25, 2024
Rollup merge of rust-lang#132101 - youknowone:thread_local-gyneiene, r=tgross35

Avoid using imports in thread_local_inner! in static

Fixes rust-lang#131863 for wasm targets

All other macros were done in rust-lang#131866, but this sub module is missed.

r? `@jieyouxu`
cuviper pushed a commit to cuviper/rust that referenced this issue Nov 7, 2024
Fixes rust-lang#131863 for wasm targets

All other macros were done in rust-lang#131866, but this sub module is missed.

(cherry picked from commit 5368b12)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: This is a bug. regression-from-stable-to-stable Performance or correctness regression from one stable version to another. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
6 participants