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

Stabilize THIR unsafeck #117673

Merged
merged 3 commits into from Jan 5, 2024

Conversation

matthewjasper
Copy link
Contributor

@matthewjasper matthewjasper commented Nov 7, 2023

  • Make -Zthir-unsafeck=on, the default
  • Update some compiler comments/names to treat THIR unsafeck as the default.
  • Union patterns are now unsafe unless the field is matched to a wildcard pattern.
  • Fixes an issue with safety checking inline constant patterns.

@rustbot
Copy link
Collaborator

rustbot commented Nov 7, 2023

r? @TaKO8Ki

(rustbot has picked a reviewer for you, use r? to override)

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Nov 7, 2023
@rust-log-analyzer

This comment has been minimized.

@lqd
Copy link
Member

lqd commented Nov 7, 2023

A perf run would be nice as well: the compiler perf WG had plans to help with this switch because THIR unsafeck was a bit faster than MIR unsafeck.

@rustbot
Copy link
Collaborator

rustbot commented Nov 7, 2023

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

@rust-log-analyzer

This comment has been minimized.

@rustbot rustbot added A-testsuite Area: The testsuite used to check the correctness of rustc T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) labels Nov 7, 2023
@rust-log-analyzer

This comment has been minimized.

@matthewjasper
Copy link
Contributor Author

@bors try @rust-timer queue

@rust-timer

This comment has been minimized.

@rustbot rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Nov 8, 2023
@bors
Copy link
Contributor

bors commented Nov 8, 2023

⌛ Trying commit e33cb9f with merge 9ee6473...

bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 8, 2023
…ation, r=<try>

Stabilize THIR unsafeck

- Removes `-Zthir-unsafeck`, stabilizing the behaviour of `-Zthir-unsafeck=on`.
- Removes MIR unsafeck.
- Union patterns are now unsafe unless the field is matched to a wildcard pattern.

Opening for a crater run in case we need a compatibility lint.
@bors
Copy link
Contributor

bors commented Nov 8, 2023

☀️ Try build successful - checks-actions
Build commit: 9ee6473 (9ee647322baa27cc6bb1a95b4595bad94b334fcf)

@rust-timer

This comment has been minimized.

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (9ee6473): comparison URL.

Overall result: ✅ improvements - no action needed

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR may lead to changes in compiler perf.

@bors rollup=never
@rustbot label: -S-waiting-on-perf -perf-regression

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
-0.3% [-0.8%, -0.2%] 66
Improvements ✅
(secondary)
-0.8% [-1.1%, -0.4%] 9
All ❌✅ (primary) -0.3% [-0.8%, -0.2%] 66

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
0.7% [0.5%, 1.0%] 8
Regressions ❌
(secondary)
3.2% [1.2%, 5.2%] 2
Improvements ✅
(primary)
-1.3% [-5.6%, -0.5%] 16
Improvements ✅
(secondary)
-4.9% [-11.5%, -1.9%] 8
All ❌✅ (primary) -0.6% [-5.6%, 1.0%] 24

Cycles

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
2.1% [2.1%, 2.1%] 1
Improvements ✅
(primary)
-0.6% [-0.9%, -0.5%] 7
Improvements ✅
(secondary)
-1.3% [-2.2%, -0.8%] 4
All ❌✅ (primary) -0.6% [-0.9%, -0.5%] 7

Binary size

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
0.0% [0.0%, 0.0%] 6
Improvements ✅
(primary)
-0.2% [-0.5%, -0.0%] 9
Improvements ✅
(secondary)
-0.4% [-0.7%, -0.0%] 12
All ❌✅ (primary) -0.2% [-0.5%, -0.0%] 9

Bootstrap: 664.079s -> 663.606s (-0.07%)
Artifact size: 308.83 MiB -> 308.73 MiB (-0.03%)

@rustbot rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Nov 8, 2023
@matthewjasper
Copy link
Contributor Author

@craterbot check

@craterbot
Copy link
Collaborator

👌 Experiment pr-117673 created and queued.
🤖 Automatically detected try build 9ee6473
🔍 You can check out the queue and this experiment's details.

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

@craterbot craterbot added S-waiting-on-crater Status: Waiting on a crater run to be completed. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 8, 2023
@craterbot
Copy link
Collaborator

🚧 Experiment pr-117673 is now running

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

@lqd
Copy link
Member

lqd commented Nov 9, 2023

@matthewjasper I also wonder if we should stabilize and enable THIR unsafeck first, and only remove MIR unsafeck after a period of time has passed, to allow for feedback and more testing? That way we'd still be able to easily flip a flag back to MIR unsafeck for beta/stable branches if we needed to.

I guess that can be discussed during FCP.

@craterbot
Copy link
Collaborator

🚨 Report generation of pr-117673 failed: Error: missing label values ["pr-117673"]
🛠️ If the error is fixed use the retry-report command.

🆘 Can someone from the infra team check in on this? @rust-lang/infra
ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

@Mark-Simulacrum
Copy link
Member

@craterbot retry-report

@matthewjasper
Copy link
Contributor Author

@bors r=cjgillot

@bors
Copy link
Contributor

bors commented Jan 5, 2024

📌 Commit 7832ebb has been approved by cjgillot

It is now in the queue for this repository.

@bors bors 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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jan 5, 2024
@bors
Copy link
Contributor

bors commented Jan 5, 2024

⌛ Testing commit 7832ebb with merge 6bc08a7...

@bors
Copy link
Contributor

bors commented Jan 5, 2024

☀️ Test successful - checks-actions
Approved by: cjgillot
Pushing 6bc08a7 to master...

@matthiaskrgr
Copy link
Member

The pr body says Removes -Zthir-unsafeck but it seems that
-Z thir-unsafeck=val -- use the THIR unsafety checker (default: yes) is still a thing, is this intended?

@compiler-errors
Copy link
Member

@matthiaskrgr: The PR description was never updated to reflect decisions made within the thread, which was just to set -Zthir-unsafeck=true by default.

@matthewjasper: It would be nice if you updated the description of this PR so it reflects what this PR actually does.

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (6bc08a7): comparison URL.

Overall result: ❌✅ regressions and improvements - ACTION NEEDED

Next Steps: If you can justify the regressions found in this perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please open an issue or create a new PR that fixes the regressions, add a comment linking to the newly created issue or PR, and then add the perf-regression-triaged label to this PR.

@rustbot label: +perf-regression
cc @rust-lang/wg-compiler-performance

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
0.3% [0.2%, 0.4%] 2
Improvements ✅
(primary)
-0.4% [-0.9%, -0.2%] 39
Improvements ✅
(secondary)
-0.6% [-1.1%, -0.4%] 9
All ❌✅ (primary) -0.4% [-0.9%, -0.2%] 39

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
-2.6% [-8.1%, -0.6%] 12
Improvements ✅
(secondary)
-3.0% [-5.2%, -1.5%] 3
All ❌✅ (primary) -2.6% [-8.1%, -0.6%] 12

Cycles

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
-1.1% [-1.2%, -1.0%] 2
Improvements ✅
(secondary)
-2.5% [-2.8%, -2.3%] 5
All ❌✅ (primary) -1.1% [-1.2%, -1.0%] 2

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 669.803s -> 670.411s (0.09%)
Artifact size: 311.12 MiB -> 311.14 MiB (0.01%)

@rustbot rustbot added the perf-regression Performance regression. label Jan 5, 2024
@nnethercote
Copy link
Contributor

Perf improvements greatly outweigh the tiny number of regressions.

@rustbot label: +perf-regression-triaged

@rustbot rustbot added the perf-regression-triaged The performance regression has been triaged. label Jan 7, 2024
wip-sync pushed a commit to NetBSD/pkgsrc-wip that referenced this pull request Mar 29, 2024
Pkgsrc changes:
 * Adapt checksums and patches.

Upstream chnages:

Version 1.77.0 (2024-03-21)
==========================

- [Reveal opaque types within the defining body for exhaustiveness checking.]
  (rust-lang/rust#116821)
- [Stabilize C-string literals.]
  (rust-lang/rust#117472)
- [Stabilize THIR unsafeck.]
  (rust-lang/rust#117673)
- [Add lint `static_mut_refs` to warn on references to mutable statics.]
  (rust-lang/rust#117556)
- [Support async recursive calls (as long as they have indirection).]
  (rust-lang/rust#117703)
- [Undeprecate lint `unstable_features` and make use of it in the compiler.]
  (rust-lang/rust#118639)
- [Make inductive cycles in coherence ambiguous always.]
  (rust-lang/rust#118649)
- [Get rid of type-driven traversal in const-eval interning]
  (rust-lang/rust#119044),
  only as a [future compatiblity lint]
  (rust-lang/rust#122204) for now.
- [Deny braced macro invocations in let-else.]
  (rust-lang/rust#119062)

Compiler
--------

- [Include lint `soft_unstable` in future breakage reports.]
  (rust-lang/rust#116274)
- [Make `i128` and `u128` 16-byte aligned on x86-based targets.]
  (rust-lang/rust#116672)
- [Use `--verbose` in diagnostic output.]
  (rust-lang/rust#119129)
- [Improve spacing between printed tokens.]
  (rust-lang/rust#120227)
- [Merge the `unused_tuple_struct_fields` lint into `dead_code`.]
  (rust-lang/rust#118297)
- [Error on incorrect implied bounds in well-formedness check]
  (rust-lang/rust#118553),
  with a temporary exception for Bevy.
- [Fix coverage instrumentation/reports for non-ASCII source code.]
  (rust-lang/rust#119033)
- [Fix `fn`/`const` items implied bounds and well-formedness check.]
  (rust-lang/rust#120019)
- [Promote `riscv32{im|imafc}-unknown-none-elf` targets to tier 2.]
  (rust-lang/rust#118704)
- Add several new tier 3 targets:
  - [`aarch64-unknown-illumos`]
    (rust-lang/rust#112936)
  - [`hexagon-unknown-none-elf`]
    (rust-lang/rust#117601)
  - [`riscv32imafc-esp-espidf`]
    (rust-lang/rust#119738)
  - [`riscv32im-risc0-zkvm-elf`]
    (rust-lang/rust#117958)

Refer to Rust's [platform support page][platform-support-doc]
for more information on Rust's tiered platform support.

Libraries
---------

- [Implement `From<&[T; N]>` for `Cow<[T]>`.]
  (rust-lang/rust#113489)
- [Remove special-case handling of `vec.split_off
  (0)`.](rust-lang/rust#119917)

Stabilized APIs
---------------

- [`array::each_ref`]
  (https://doc.rust-lang.org/stable/std/primitive.array.html#method.each_ref)
- [`array::each_mut`]
  (https://doc.rust-lang.org/stable/std/primitive.array.html#method.each_mut)
- [`core::net`]
  (https://doc.rust-lang.org/stable/core/net/index.html)
- [`f32::round_ties_even`]
  (https://doc.rust-lang.org/stable/std/primitive.f32.html#method.round_ties_even)
- [`f64::round_ties_even`]
  (https://doc.rust-lang.org/stable/std/primitive.f64.html#method.round_ties_even)
- [`mem::offset_of!`]
  (https://doc.rust-lang.org/stable/std/mem/macro.offset_of.html)
- [`slice::first_chunk`]
  (https://doc.rust-lang.org/stable/std/primitive.slice.html#method.first_chunk)
- [`slice::first_chunk_mut`]
  (https://doc.rust-lang.org/stable/std/primitive.slice.html#method.first_chunk_mut)
- [`slice::split_first_chunk`]
  (https://doc.rust-lang.org/stable/std/primitive.slice.html#method.split_first_chunk)
- [`slice::split_first_chunk_mut`]
  (https://doc.rust-lang.org/stable/std/primitive.slice.html#method.split_first_chunk_mut)
- [`slice::last_chunk`]
  (https://doc.rust-lang.org/stable/std/primitive.slice.html#method.last_chunk)
- [`slice::last_chunk_mut`]
  (https://doc.rust-lang.org/stable/std/primitive.slice.html#method.last_chunk_mut)
- [`slice::split_last_chunk`]
  (https://doc.rust-lang.org/stable/std/primitive.slice.html#method.split_last_chunk)
- [`slice::split_last_chunk_mut`]
  (https://doc.rust-lang.org/stable/std/primitive.slice.html#method.split_last_chunk_mut)
- [`slice::chunk_by`]
  (https://doc.rust-lang.org/stable/std/primitive.slice.html#method.chunk_by)
- [`slice::chunk_by_mut`]
  (https://doc.rust-lang.org/stable/std/primitive.slice.html#method.chunk_by_mut)
- [`Bound::map`]
  (https://doc.rust-lang.org/stable/std/ops/enum.Bound.html#method.map)
- [`File::create_new`]
  (https://doc.rust-lang.org/stable/std/fs/struct.File.html#method.create_new)
- [`Mutex::clear_poison`]
  (https://doc.rust-lang.org/stable/std/sync/struct.Mutex.html#method.clear_poison)
- [`RwLock::clear_poison`]
  (https://doc.rust-lang.org/stable/std/sync/struct.RwLock.html#method.clear_poison)

Cargo
-----

- [Extend the build directive syntax with `cargo::`.]
  (rust-lang/cargo#12201)
- [Stabilize metadata `id` format as `PackageIDSpec`.]
  (rust-lang/cargo#12914)
- [Pull out as `cargo-util-schemas` as a crate.]
  (rust-lang/cargo#13178)
- [Strip all debuginfo when debuginfo is not requested.]
  (rust-lang/cargo#13257)
- [Inherit jobserver from env for all kinds of runners.]
  (rust-lang/cargo#12776)
- [Deprecate rustc plugin support in cargo.]
  (rust-lang/cargo#13248)

Rustdoc
-----

- [Allows links in markdown headings.]
  (rust-lang/rust#117662)
- [Search for tuples and unit by type with `()`.]
  (rust-lang/rust#118194)
- [Clean up the source sidebar's hide button.]
  (rust-lang/rust#119066)
- [Prevent JS injection from `localStorage`.]
  (rust-lang/rust#120250)

Misc
----

- [Recommend version-sorting for all sorting in style guide.]
  (rust-lang/rust#115046)

Internal Changes
----------------

These changes do not affect any public interfaces of Rust, but they represent
significant improvements to the performance or internals of rustc and related
tools.

- [Add more weirdness to `weird-exprs.rs`.]
  (rust-lang/rust#119028)
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Apr 2, 2024
…ompiler-errors

Remove dangling `.mir.stderr` and `.thir.stderr` test files

They are not needed since rust-lang#117673
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Apr 2, 2024
Rollup merge of rust-lang#123371 - eduardosm:dangling-test-files, r=compiler-errors

Remove dangling `.mir.stderr` and `.thir.stderr` test files

They are not needed since rust-lang#117673
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-testsuite Area: The testsuite used to check the correctness of rustc disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. finished-final-comment-period The final comment period is finished for this PR / Issue. merged-by-bors This PR was explicitly merged by bors. perf-regression Performance regression. perf-regression-triaged The performance regression has been triaged. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-lang Relevant to the language team, which will review and decide on the PR/issue. T-opsem Relevant to the opsem team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet