-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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
Implement RFC 3184 - thread local cell methods #92123
Conversation
(rust-highfive has picked a reviewer for you, use r? to override) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice! Note that I got a bit confused by the nested Option
dance at the beginning, hence some extra comments and renames, but nothing paramount.
@danielhenrymantilla Thanks for the review! |
This is bocked on an issue with the CI/test runner, where a large output from rustc gets trimmed and |
I filed an issue for it: #94322 Thanks for the reminder. |
50e6b83
to
363833b
Compare
…etrochenkov Avoid emitting full macro body into JSON errors While investigating rust-lang#94322, it was noted that currently the JSON diagnostics for macro backtraces include the full def_site span -- the whole macro body. It seems like this shouldn't be necessary, so this PR adjusts the span to just be the "guessed head", typically the macro name. It doesn't look like we keep enough information to synthesize a nicer span here at this time. Atop rust-lang#92123, this reduces output for the src/test/ui/suggestions/missing-lifetime-specifier.rs test from 660 KB to 156 KB locally.
…etrochenkov Avoid emitting full macro body into JSON errors While investigating rust-lang#94322, it was noted that currently the JSON diagnostics for macro backtraces include the full def_site span -- the whole macro body. It seems like this shouldn't be necessary, so this PR adjusts the span to just be the "guessed head", typically the macro name. It doesn't look like we keep enough information to synthesize a nicer span here at this time. Atop rust-lang#92123, this reduces output for the src/test/ui/suggestions/missing-lifetime-specifier.rs test from 660 KB to 156 KB locally.
…etrochenkov Avoid emitting full macro body into JSON errors While investigating rust-lang#94322, it was noted that currently the JSON diagnostics for macro backtraces include the full def_site span -- the whole macro body. It seems like this shouldn't be necessary, so this PR adjusts the span to just be the "guessed head", typically the macro name. It doesn't look like we keep enough information to synthesize a nicer span here at this time. Atop rust-lang#92123, this reduces output for the src/test/ui/suggestions/missing-lifetime-specifier.rs test from 660 KB to 156 KB locally.
☔ The latest upstream changes (presumably #94373) made this pull request unmergeable. Please resolve the merge conflicts. |
363833b
to
41248d5
Compare
This comment has been minimized.
This comment has been minimized.
41248d5
to
e602bef
Compare
@joshtriplett The CI issue has been fixed, so this is now ready for review. |
@bors r+ |
📌 Commit e602beff3293960273780440b63240be5568441c has been approved by |
e602bef
to
a6e7f26
Compare
@bors r=joshtriplett |
📌 Commit a6e7f26 has been approved by |
⌛ Testing commit a6e7f26 with merge 35d2d53a0ecf89712d3378790151a186fe4f6379... |
The job Click to see the possible cause of the failure (guessed by this bot)
|
💔 Test failed - checks-actions |
@bors retry |
☀️ Test successful - checks-actions |
Finished benchmarking commit (ab2bd41): comparison url. Summary: This benchmark run did not return any relevant results. 37 results were found to be statistically significant but too small to be relevant. If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf. @rustbot label: -perf-regression |
This implements RFC 3184, with @danielhenrymantilla's suggestion for the
with_
method names.Tracking issue: #92122