Skip to content

Fix missing_const_for_thread_local false positive on targets without native #[thread_local] - #17567

Open
MohammedAlkindi wants to merge 2 commits into
rust-lang:masterfrom
MohammedAlkindi:fix/missing-const-thread-local-const-block
Open

Fix missing_const_for_thread_local false positive on targets without native #[thread_local]#17567
MohammedAlkindi wants to merge 2 commits into
rust-lang:masterfrom
MohammedAlkindi:fix/missing-const-thread-local-const-block

Conversation

@MohammedAlkindi

@MohammedAlkindi MohammedAlkindi commented Aug 15, 2026

Copy link
Copy Markdown

On targets without native #[thread_local] (no target_thread_local cfg — e.g. x86_64-pc-windows-gnu), current nightlies expand a const thread-local initializer into a plain non-const #[inline] fn __rust_std_internal_init_fn(). The lint's backend guard (!cx.tcx.is_const_fn(defid), from #12276) assumed the generated init fn is const whenever the initializer is, so these fns are no longer filtered out: is_min_const_fn approves the body and the lint fires on initializers that are already const, spanning the whole thread_local! invocation. Reproducer and details: #17566.

The fix adds one guard: skip when the initializer expression is ExprKind::ConstBlock — for an already-const initializer there is never anything to suggest, on any backend. The existing is_const_fn check is deliberately left in place for backends whose generated init fn is still const.

Verified on x86_64-pc-windows-gnu, the affected host (no Clippy CI host lacks target_thread_local, so CI cannot observe the bug or the fix):

  • before: cargo test --test compile-test -- missing_const_for_thread_local fails with 8 errors against the blessed 6 (extra whole-block spans on the already-const statics), and cargo test --test dogfood fails on clippy_utils/src/macros.rs:205 (could not compile clippy_utils)
  • after: both pass — tests/ui/missing_const_for_thread_local.rs ... ok, tests/ui/missing_const_for_thread_local.fixed ... ok, dogfood lints all crates clean
  • x86_64-pc-windows-msvc/Linux output is unchanged by construction: the new arm only rejects ConstBlock initializers, which never linted there

No ui-test change is included: the blessed .stderr already encodes the correct behavior, and no CI target can exercise the affected expansion path.

fixes #17566

changelog: [missing_const_for_thread_local]: fix false positive on targets without native #[thread_local] where the initializer is already const

On targets without native #[thread_local], std's thread_local! now
expands a const initializer into a plain non-const init fn, so the
is_const_fn guard from rust-lang#12276 no longer filters it and the lint fires
on initializers that are already const. Skip ExprKind::ConstBlock
initializers: for those there is never anything to suggest.

Fixes rust-lang#17566
@rustbot rustbot added S-waiting-on-community-reviews Status: This is awaiting for positive reviews from the community before a maintainer is assigned. S-waiting-on-review Status: Awaiting review from the assignee but also interested parties labels Aug 15, 2026
@rustbot

rustbot commented Aug 15, 2026

Copy link
Copy Markdown
Collaborator

Thanks for the pull request, and welcome!

You should hear from one of our reviewers after this PR gets at least 2 reviews from the community.

Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (S-waiting-on-review and S-waiting-on-author) stays updated, invoking these commands when appropriate:

  • @rustbot author: the review is finished, PR author should check the comments and take action accordingly
  • @rustbot review: the author is ready for a review, this PR will be queued again in the reviewer's queue

@rustbot

rustbot commented Aug 15, 2026

Copy link
Copy Markdown
Collaborator

⚠️ Warning ⚠️

  • There are issue links (such as #123) in the commit messages of the following commits.
    Please move them to the PR description, to avoid spamming the issues with references to the commit, and so this bot can automatically canonicalize them to avoid issues with subtree.

@NicDevTV NicDevTV left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@CommanderStorm CommanderStorm left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

community review:
please add the reproducer from the issue to the testcase for this.
This way we can ensure we don't regress on this code

View changes since this review

@CommanderStorm

Copy link
Copy Markdown
Contributor

skip when the initializer expression is ExprKind::ConstBlock — for an already-const initializer there is never anything to suggest, on any backend. The existing is_const_fn check is deliberately left in place for backends whose generated init fn is still const.

No ui-test change is included: the blessed .stderr already encodes the correct behavior, and no CI target can exercise the affected expansion path.

So do we have an testing gap?
Please explain this in your words. it is hard to follow your LLM-style comment.

@MohammedAlkindi

Copy link
Copy Markdown
Author

Added as fn issue_17566() in both the .rs and .fixed files, asserting no lint. It only bites on targets without native #[thread_local], so it passes trivially on CI's targets and is meaningful on the ones where the initializer expands to a non-const init fn.

I could not run the UI suite locally to confirm it: rustup-managed Rust binaries are blocked by an Application Control policy on this machine, so rustc -vV from the pinned nightly returns nothing and cargo test dies at rustc -vV with 0xc0e90002. Only a standalone stable install runs here, which cannot build clippy. Treating CI as the authority on this one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-community-reviews Status: This is awaiting for positive reviews from the community before a maintainer is assigned. S-waiting-on-review Status: Awaiting review from the assignee but also interested parties

Projects

None yet

Development

Successfully merging this pull request may close these issues.

missing_const_for_thread_local false positive on targets without #[thread_local]: fires on already-const initializers

4 participants