Skip to content

thread::LocalKey: document meaning of 'inner' function argument - #163398

Merged
rust-bors[bot] merged 1 commit into
rust-lang:mainfrom
RalfJung:thread-local-key-inner
Oct 11, 2026
Merged

thread::LocalKey: document meaning of 'inner' function argument#163398
rust-bors[bot] merged 1 commit into
rust-lang:mainfrom
RalfJung:thread-local-key-inner

Conversation

@RalfJung

@RalfJung RalfJung commented Sep 27, 2026

Copy link
Copy Markdown
Member

#92123 gave this function an argument, but without documenting what the argument means or does. I hope I reverse engineered this correctly.

Cc @m-ou-se @joboet

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Sep 27, 2026
@rustbot

rustbot commented Sep 27, 2026

Copy link
Copy Markdown
Collaborator

r? @clarfonthey

rustbot has assigned @clarfonthey.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

Why was this reviewer chosen?

The reviewer was selected based on:

  • Owners of files modified in this PR: @ChrisDenton, libs
  • @ChrisDenton, libs expanded to 13 candidates
  • Random selection from 7 candidates

joboet
joboet reviewed Sep 27, 2026
View reviewed changes
Comment thread library/std/src/thread/local.rs Outdated
// thread: if it is not yet initialized, and if the argument is `Some(&mut Some(val))`, the
// inner `val` should be `take`en out and used as the initial value instead of the default. This
// is purely an optimization for the case where the value will be immediately overwritten; it is
// okay for `inner` to always ignore `init`.

@joboet joboet Sep 27, 2026 •
edited by rustbot
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

That's not always true, this only applies in the case of const-initialisers. In other cases, init must not be ignored as LocalKey::set guarantees that it will succeed even if the initialiser panics.

View changes since the review

@RalfJung RalfJung Sep 27, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Ah! The contract is quite subtle then. I updated the comment.

FWIW, if the destructor of the default value panics, that could still cause LocalKey::set to panic.

@joboet joboet Sep 27, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hmm, true. This seems almost impossible to prevent, and such behaviour isn't that surprising – but we should probably document it...

@RalfJung RalfJung Sep 27, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Well we could prevent the unwind part of that panic with rust-lang/rfcs#3288... but some sort of failure (like abort) cannot really be prevented, yeah. It's also normal in Rust that seting a value drops the old value which can fail.

@RalfJung RalfJung Oct 10, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@joboet I updated the comment based on your feedback, is there anything else you'd like me to change?

@RalfJung
RalfJung force-pushed the thread-local-key-inner branch 2 times, most recently from c9c2f66 to e62fd11 Compare September 27, 2026 09:43
@clarfonthey

clarfonthey commented Sep 29, 2026

Copy link
Copy Markdown
Contributor

r? joboet since you seem to understand this better than I do. I'd just be guessing like Ralf is.

@rustbot rustbot assigned joboet and unassigned clarfonthey Sep 29, 2026
@rustbot

rustbot commented Sep 29, 2026

Copy link
Copy Markdown
Collaborator

joboet is currently at their maximum review capacity.
They may take a while to respond.

joboet
joboet approved these changes Oct 11, 2026
View reviewed changes

@joboet joboet left a comment •
edited by rustbot
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

r=me with the nit resolved.

View changes since this review

Comment thread library/std/src/sys/thread_local/native/mod.rs Outdated
@RalfJung

RalfJung commented Oct 11, 2026

Copy link
Copy Markdown
Member Author

@bors squash

@rust-bors

This comment has been minimized.

Sign in to view
@RalfJung @joboet
thread::LocalKey: document meaning of 'inner' function argument
47e5c89 
* thread::LocalKey: document meaning of 'inner' function argument
* nit

Co-authored-by: Jonas Böttiger <jonasboettiger@icloud.com>
@rust-bors

rust-bors Bot commented Oct 11, 2026

Copy link
Copy Markdown
Contributor

🔨 2 commits were squashed into 47e5c89.

@rust-bors
rust-bors Bot force-pushed the thread-local-key-inner branch from 742efae to 47e5c89 Compare October 11, 2026 09:36
@RalfJung

RalfJung commented Oct 11, 2026

Copy link
Copy Markdown
Member Author

@bors r=joboet

@rust-bors

rust-bors Bot commented Oct 11, 2026

Copy link
Copy Markdown
Contributor

📌 Commit 47e5c89 has been tentatively approved by joboet

It will be put into the queue for this repository once PR CI succeeds.

@rust-bors rust-bors Bot added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 11, 2026
@RalfJung

RalfJung commented Oct 11, 2026

Copy link
Copy Markdown
Member Author

@bors rollup

@rust-bors

This comment has been minimized.

Sign in to view
@rust-bors rust-bors Bot added merged-by-bors This PR was explicitly merged by bors. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Oct 11, 2026
@rust-bors

rust-bors Bot commented Oct 11, 2026

Copy link
Copy Markdown
Contributor

☀️ Test successful - CI
Approved by: joboet
Duration: 3h 2m 54s
Pushing 0617e57 to main...

@rust-bors
rust-bors Bot merged commit 0617e57 into rust-lang:main Oct 11, 2026
15 checks passed
@rustbot rustbot added this to the 1.101.0 milestone Oct 11, 2026
@rust-bors rust-bors Bot mentioned this pull request Oct 11, 2026
Open
@github-actions

github-actions Bot commented Oct 11, 2026

Copy link
Copy Markdown
Contributor
What is this? This is an experimental post-merge analysis report that shows differences in test outcomes between the merged PR and its parent PR.

Comparing 7211ea5 (parent) -> 0617e57 (this PR)

Test differences

Show 56 test diffs

56 doctest diffs were found. These are ignored, as they are noisy.

Test dashboard

Run

cargo run --manifest-path src/ci/citool/Cargo.toml -- \
    test-dashboard 0617e57afe33d3afb1d5d5489030cae802f6c17e --output-dir test-dashboard

And then open test-dashboard/index.html in your browser to see an overview of all executed tests.

Job duration changes

  1. test-i686-msvc: 1h 31m -> 2h 30m (+64.5%)
  2. dist-various-2: 33m 17s -> 53m 34s (+60.9%)
  3. dist-loongarch64-linux: 1h 19m -> 2h 7m (+59.7%)
  4. test-x86_64-mingw-1: 1h 57m -> 3h 1m (+54.8%)
  5. dist-armhf-linux: 1h -> 1h 30m (+51.2%)
  6. test-various: 1h 27m -> 2h 11m (+50.2%)
  7. dist-powerpc64le-linux-gnu: 1h 6m -> 1h 39m (+49.3%)
  8. test-x86_64-gnu-miri: 57m 43s -> 1h 23m (+44.9%)
  9. test-x86_64-gnu-stdlib-semver-check: 17m 58s -> 9m 55s (-44.8%)
  10. test-x86_64-rust-for-linux: 1h 1m -> 35m 25s (-42.5%)
How to interpret the job duration changes?

Job durations can vary a lot, based on the actual runner instance
that executed the job, system noise, invalidated caches, etc. The table above is provided
mostly for t-infra members, for simpler debugging of potential CI slow-downs.

@rust-timer

rust-timer commented Oct 11, 2026

Copy link
Copy Markdown
Collaborator

Finished benchmarking commit (0617e57): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

This perf run didn't have relevant results for this metric.

Max RSS (memory usage)

Results (primary 2.8%, secondary 4.7%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

mean range count
Regressions ❌
(primary)		 2.8% [2.8%, 2.8%] 1
Regressions ❌
(secondary)		 6.5% [4.0%, 11.5%] 5
Improvements ✅
(primary)		 - - 0
Improvements ✅
(secondary)		 -4.5% [-4.5%, -4.5%] 1
All ❌✅ (primary) 2.8% [2.8%, 2.8%] 1

Cycles

Results (primary -2.3%, secondary 1.2%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

mean range count
Regressions ❌
(primary)		 - - 0
Regressions ❌
(secondary)		 5.8% [3.4%, 8.3%] 2
Improvements ✅
(primary)		 -2.3% [-2.3%, -2.3%] 1
Improvements ✅
(secondary)		 -3.4% [-4.1%, -2.7%] 2
All ❌✅ (primary) -2.3% [-2.3%, -2.3%] 1

Binary size

This perf run didn't have relevant results for this metric.

Bootstrap: 492.622s -> 487.225s (-1.10%)
Artifact size: 407.21 MiB -> 406.47 MiB (-0.18%)

@github-actions github-actions Bot mentioned this pull request Oct 13, 2026
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@joboet joboet joboet approved these changes

Assignees

@joboet joboet

Labels

merged-by-bors This PR was explicitly merged by bors. T-libs Relevant to the library team, which will review and decide on the PR/issue.

Projects

None yet

Milestone

1.101.0

Development

Successfully merging this pull request may close these issues.

5 participants

@RalfJung @rustbot @clarfonthey @rust-timer @joboet