Remove MIR unsafe check #123322

matthewjasper · 2024-04-01T13:24:12Z

Now that THIR unsafeck is enabled by default in stable I think we can remove MIR unsafeck entirely. This PR also removes safety information from MIR.

rustbot · 2024-04-01T13:24:20Z

r? @Nadrieril

rustbot has assigned @Nadrieril.
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

rustbot · 2024-04-01T13:24:22Z

Some changes occurred in match lowering

cc @Nadrieril

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

tests/incremental/hashes/function_interfaces.rs

Nadrieril · 2024-04-01T16:01:12Z

This is a very satisfying PR :D Implementation looks good, I'd like someone else to confirm that this is a good idea; @compiler-errors?

compiler-errors

I'm curious if y'all think this needs an additional FCP given that the FCP in #117673 seemed to exclude the changes to MIR building and removing MIR unsafeck.

The changes look good to me tho.

If anything, it's worth a ping to @rust-lang/compiler for the MIR changes and @rust-lang/lang for the removal of MIR unsafeck (i.e. full commitment to THIR unsafeck).

lcnr · 2024-04-02T02:48:47Z

@bors try @rust-timer queue

…=<try> Remove MIR unsafe check Now that THIR unsafeck is enabled by default in stable I think we can remove MIR unsafeck entirely. This PR also removes safety information from MIR.

bors · 2024-04-02T02:49:57Z

⌛ Trying commit c10ad18 with merge 361ad8e...

lcnr · 2024-04-02T02:50:40Z

Given that thir-unsafeck has now hit stable (version 1.77), we can now safely remove MIR unsafe check. Don't believe this to require any additional team approval.

scottmcm · 2024-04-02T03:35:37Z

I don't think anything is needed here from lang. Personally I'd consider it removing off-in-stable code in rustc, which is entirely up to compiler.

bors · 2024-04-02T04:24:30Z

☀️ Try build successful - checks-actions
Build commit: 361ad8e (361ad8e0935797cfb92d931aedd56456973d1c5f)

rust-timer · 2024-04-02T06:31:55Z

Finished benchmarking commit (361ad8e): comparison URL.

Overall result: ❌✅ regressions and 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)	1.1%	[1.1%, 1.1%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-2.5%	[-2.5%, -2.5%]	1
All ❌✅ (primary)	-	-	0

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)	2.7%	[2.1%, 3.3%]	2
Regressions ❌ (secondary)	4.7%	[3.9%, 5.5%]	2
Improvements ✅ (primary)	-4.6%	[-4.6%, -4.6%]	1
Improvements ✅ (secondary)	-2.9%	[-2.9%, -2.9%]	1
All ❌✅ (primary)	0.3%	[-4.6%, 3.3%]	3

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)	1.2%	[1.2%, 1.2%]	1
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	1.2%	[1.2%, 1.2%]	1

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
Improvements ✅ (primary)	-0.2%	[-0.5%, -0.0%]	7
Improvements ✅ (secondary)	-0.5%	[-0.9%, -0.0%]	12
All ❌✅ (primary)	-0.2%	[-0.5%, -0.0%]	7

Bootstrap: 669.168s -> 667.487s (-0.25%)
Artifact size: 315.68 MiB -> 315.62 MiB (-0.02%)

matthewjasper · 2024-04-02T09:36:25Z

Changes look like noise

lcnr · 2024-04-02T10:38:18Z

compiler/rustc_mir_build/src/check_unsafety.rs

@@ -909,11 +909,6 @@ impl UnsafeOpKind {
 }

 pub fn check_unsafety(tcx: TyCtxt<'_>, def: LocalDefId) {


in separate PR: should this get moved into its own crate rustc_thir_unsafeck or rustc_thir_analysis?

lcnr · 2024-04-02T10:40:20Z

r? @lcnr

@bors r=compiler-errors,lcnr rollup=iffy

bors · 2024-04-02T10:40:22Z

📌 Commit c10ad18 has been approved by compiler-errors,lcnr

It is now in the queue for this repository.

bors · 2024-04-02T12:12:38Z

💔 Test failed - checks-actions

compiler-errors · 2024-04-02T16:08:20Z

@bors r=lcnr,compiler-errors

bors · 2024-04-02T16:08:23Z

📌 Commit 2645e60 has been approved by lcnr,compiler-errors

It is now in the queue for this repository.

bors · 2024-04-03T00:04:22Z

🔒 Merge conflict

This pull request and the master branch diverged in a way that cannot be automatically merged. Please rebase on top of the latest master branch, and let the reviewer approve again.

How do I rebase?

Assuming self is your fork and upstream is this repository, you can resolve the conflict following these steps:

git checkout remove-mir-unsafeck (switch to your branch)
git fetch upstream master (retrieve the latest master)
git rebase upstream/master -p (rebase on top of it)
Follow the on-screen instruction to resolve conflicts (check git status if you got lost).
git push self remove-mir-unsafeck --force-with-lease (update this PR)

You may also read Git Rebasing to Resolve Conflicts by Drew Blessing for a short tutorial.

Please avoid the "Resolve conflicts" button on GitHub. It uses git merge instead of git rebase which makes the PR commit history more difficult to read.

Sometimes step 4 will complete without asking for resolution. This is usually due to difference between how Cargo.lock conflict is handled during merge and rebase. This is normal, and you should still perform step 5 to update this PR.

Error message

Removing tests/ui/asm/x86_64/goto.thirunsafeck.stderr
CONFLICT (modify/delete): tests/mir-opt/inline/unchecked_shifts.unchecked_shr_signed_smaller.PreCodegen.after.panic-unwind.mir deleted in HEAD and modified in heads/homu-tmp. Version heads/homu-tmp of tests/mir-opt/inline/unchecked_shifts.unchecked_shr_signed_smaller.PreCodegen.after.panic-unwind.mir left in tree.
CONFLICT (modify/delete): tests/mir-opt/inline/unchecked_shifts.unchecked_shr_signed_smaller.PreCodegen.after.panic-abort.mir deleted in HEAD and modified in heads/homu-tmp. Version heads/homu-tmp of tests/mir-opt/inline/unchecked_shifts.unchecked_shr_signed_smaller.PreCodegen.after.panic-abort.mir left in tree.
CONFLICT (modify/delete): tests/mir-opt/inline/unchecked_shifts.unchecked_shr_signed_smaller.Inline.panic-unwind.diff deleted in HEAD and modified in heads/homu-tmp. Version heads/homu-tmp of tests/mir-opt/inline/unchecked_shifts.unchecked_shr_signed_smaller.Inline.panic-unwind.diff left in tree.
CONFLICT (modify/delete): tests/mir-opt/inline/unchecked_shifts.unchecked_shr_signed_smaller.Inline.panic-abort.diff deleted in HEAD and modified in heads/homu-tmp. Version heads/homu-tmp of tests/mir-opt/inline/unchecked_shifts.unchecked_shr_signed_smaller.Inline.panic-abort.diff left in tree.
Auto-merging tests/mir-opt/inline/unchecked_shifts.unchecked_shr_signed_bigger.PreCodegen.after.panic-unwind.mir
CONFLICT (content): Merge conflict in tests/mir-opt/inline/unchecked_shifts.unchecked_shr_signed_bigger.PreCodegen.after.panic-unwind.mir
Auto-merging tests/mir-opt/inline/unchecked_shifts.unchecked_shr_signed_bigger.PreCodegen.after.panic-abort.mir
CONFLICT (content): Merge conflict in tests/mir-opt/inline/unchecked_shifts.unchecked_shr_signed_bigger.PreCodegen.after.panic-abort.mir
Auto-merging tests/mir-opt/inline/unchecked_shifts.unchecked_shr_signed_bigger.Inline.panic-unwind.diff
CONFLICT (content): Merge conflict in tests/mir-opt/inline/unchecked_shifts.unchecked_shr_signed_bigger.Inline.panic-unwind.diff
Auto-merging tests/mir-opt/inline/unchecked_shifts.unchecked_shr_signed_bigger.Inline.panic-abort.diff
CONFLICT (content): Merge conflict in tests/mir-opt/inline/unchecked_shifts.unchecked_shr_signed_bigger.Inline.panic-abort.diff
Auto-merging tests/mir-opt/inline/unchecked_shifts.unchecked_shl_unsigned_smaller.PreCodegen.after.panic-unwind.mir
CONFLICT (content): Merge conflict in tests/mir-opt/inline/unchecked_shifts.unchecked_shl_unsigned_smaller.PreCodegen.after.panic-unwind.mir
Auto-merging tests/mir-opt/inline/unchecked_shifts.unchecked_shl_unsigned_smaller.PreCodegen.after.panic-abort.mir
CONFLICT (content): Merge conflict in tests/mir-opt/inline/unchecked_shifts.unchecked_shl_unsigned_smaller.PreCodegen.after.panic-abort.mir
Auto-merging tests/mir-opt/inline/unchecked_shifts.unchecked_shl_unsigned_smaller.Inline.panic-unwind.diff
CONFLICT (content): Merge conflict in tests/mir-opt/inline/unchecked_shifts.unchecked_shl_unsigned_smaller.Inline.panic-unwind.diff
Auto-merging tests/mir-opt/inline/unchecked_shifts.unchecked_shl_unsigned_smaller.Inline.panic-abort.diff
CONFLICT (content): Merge conflict in tests/mir-opt/inline/unchecked_shifts.unchecked_shl_unsigned_smaller.Inline.panic-abort.diff
CONFLICT (modify/delete): tests/mir-opt/inline/unchecked_shifts.unchecked_shl_unsigned_bigger.PreCodegen.after.panic-unwind.mir deleted in HEAD and modified in heads/homu-tmp. Version heads/homu-tmp of tests/mir-opt/inline/unchecked_shifts.unchecked_shl_unsigned_bigger.PreCodegen.after.panic-unwind.mir left in tree.
CONFLICT (modify/delete): tests/mir-opt/inline/unchecked_shifts.unchecked_shl_unsigned_bigger.PreCodegen.after.panic-abort.mir deleted in HEAD and modified in heads/homu-tmp. Version heads/homu-tmp of tests/mir-opt/inline/unchecked_shifts.unchecked_shl_unsigned_bigger.PreCodegen.after.panic-abort.mir left in tree.
CONFLICT (modify/delete): tests/mir-opt/inline/unchecked_shifts.unchecked_shl_unsigned_bigger.Inline.panic-unwind.diff deleted in HEAD and modified in heads/homu-tmp. Version heads/homu-tmp of tests/mir-opt/inline/unchecked_shifts.unchecked_shl_unsigned_bigger.Inline.panic-unwind.diff left in tree.
CONFLICT (modify/delete): tests/mir-opt/inline/unchecked_shifts.unchecked_shl_unsigned_bigger.Inline.panic-abort.diff deleted in HEAD and modified in heads/homu-tmp. Version heads/homu-tmp of tests/mir-opt/inline/unchecked_shifts.unchecked_shl_unsigned_bigger.Inline.panic-abort.diff left in tree.
Auto-merging tests/incremental/hashes/function_interfaces.rs
Removing compiler/rustc_mir_transform/src/check_unsafety.rs
Automatic merge failed; fix conflicts and then commit the result.

bors · 2024-04-03T00:16:33Z

☔ The latest upstream changes (presumably #123385) made this pull request unmergeable. Please resolve the merge conflicts.

This also remove safety information from MIR.

lcnr · 2024-04-03T09:20:34Z

@bors r=lcnr,compiler-errors p=1 (prone to merge conflicts apparently)

bors · 2024-04-03T09:20:37Z

📌 Commit a277c90 has been approved by lcnr,compiler-errors

It is now in the queue for this repository.

bors · 2024-04-03T10:30:38Z

⌛ Testing commit a277c90 with merge 99c42d2...

bors · 2024-04-03T12:31:26Z

☀️ Test successful - checks-actions
Approved by: lcnr,compiler-errors
Pushing 99c42d2 to master...

rust-timer · 2024-04-03T13:48:44Z

Finished benchmarking commit (99c42d2): comparison URL.

Overall result: ❌✅ regressions and improvements - no action needed

@rustbot label: -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.3%	[0.3%, 0.3%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-0.9%	[-1.7%, -0.2%]	2
All ❌✅ (primary)	-	-	0

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.1%	[0.1%, 0.1%]	1
Regressions ❌ (secondary)	4.9%	[4.7%, 5.2%]	2
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	0.1%	[0.1%, 0.1%]	1

Cycles

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

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
Improvements ✅ (primary)	-0.2%	[-0.5%, -0.0%]	6
Improvements ✅ (secondary)	-0.5%	[-0.8%, -0.0%]	9
All ❌✅ (primary)	-0.2%	[-0.5%, -0.0%]	6

Bootstrap: 667.159s -> 668.489s (0.20%)
Artifact size: 318.19 MiB -> 318.05 MiB (-0.05%)

…=lcnr,compiler-errors Remove MIR unsafe check Now that THIR unsafeck is enabled by default in stable I think we can remove MIR unsafeck entirely. This PR also removes safety information from MIR.

rustbot assigned Nadrieril Apr 1, 2024

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 Apr 1, 2024

This comment has been minimized.

Sign in to view

matthewjasper force-pushed the remove-mir-unsafeck branch from 1f07c6b to 5385ae3 Compare April 1, 2024 13:45

This comment has been minimized.

Sign in to view

matthewjasper force-pushed the remove-mir-unsafeck branch from 5385ae3 to c10ad18 Compare April 1, 2024 14:59

Nadrieril reviewed Apr 1, 2024

View reviewed changes

tests/incremental/hashes/function_interfaces.rs Show resolved Hide resolved

compiler-errors approved these changes Apr 1, 2024

View reviewed changes

This comment has been minimized.

Sign in to view

rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Apr 2, 2024

This comment has been minimized.

Sign in to view

rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Apr 2, 2024

lcnr reviewed Apr 2, 2024

View reviewed changes

rustbot assigned lcnr and unassigned Nadrieril Apr 2, 2024

bors removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 2, 2024

bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Apr 2, 2024

matthewjasper force-pushed the remove-mir-unsafeck branch from c10ad18 to 2645e60 Compare April 2, 2024 15:33

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-review Status: Awaiting review from the assignee but also interested parties. labels Apr 2, 2024

bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Apr 3, 2024

Remove MIR unsafe check

a277c90

This also remove safety information from MIR.

matthewjasper force-pushed the remove-mir-unsafeck branch from 2645e60 to a277c90 Compare April 3, 2024 08:50

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 Apr 3, 2024

bors added the merged-by-bors This PR was explicitly merged by bors. label Apr 3, 2024

bors merged commit 99c42d2 into rust-lang:master Apr 3, 2024
12 checks passed

rustbot added this to the 1.79.0 milestone Apr 3, 2024

This was referenced Apr 3, 2024

Support tail calls in mir via TerminatorKind::TailCall #113128

Merged

Add assert_unsafe_precondition to unchecked_{add,sub,neg,mul,shl,shr} methods #121571

Merged

matthewjasper deleted the remove-mir-unsafeck branch April 3, 2024 13:08

bors mentioned this pull request Apr 3, 2024

Add FileCheck annotations to mir-opt/dest-prop tests #122300

Merged

lcnr mentioned this pull request Apr 5, 2024

update and archive project-thir-unsafeck rust-lang/team#1422

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove MIR unsafe check #123322

Remove MIR unsafe check #123322

matthewjasper commented Apr 1, 2024

rustbot commented Apr 1, 2024

rustbot commented Apr 1, 2024

This comment has been minimized.

This comment has been minimized.

Nadrieril commented Apr 1, 2024

compiler-errors left a comment

lcnr commented Apr 2, 2024

This comment has been minimized.

bors commented Apr 2, 2024

lcnr commented Apr 2, 2024

scottmcm commented Apr 2, 2024 •

edited

Loading

bors commented Apr 2, 2024

This comment has been minimized.

rust-timer commented Apr 2, 2024

matthewjasper commented Apr 2, 2024

lcnr Apr 2, 2024 •

edited

Loading

lcnr commented Apr 2, 2024

bors commented Apr 2, 2024

bors commented Apr 2, 2024

compiler-errors commented Apr 2, 2024

bors commented Apr 2, 2024

bors commented Apr 3, 2024

bors commented Apr 3, 2024

lcnr commented Apr 3, 2024

bors commented Apr 3, 2024

bors commented Apr 3, 2024

bors commented Apr 3, 2024

rust-timer commented Apr 3, 2024

		@@ -909,11 +909,6 @@ impl UnsafeOpKind {
		}

		pub fn check_unsafety(tcx: TyCtxt<'_>, def: LocalDefId) {

Remove MIR unsafe check #123322

Remove MIR unsafe check #123322

Conversation

matthewjasper commented Apr 1, 2024

rustbot commented Apr 1, 2024

rustbot commented Apr 1, 2024

This comment has been minimized.

This comment has been minimized.

Nadrieril commented Apr 1, 2024

compiler-errors left a comment

Choose a reason for hiding this comment

lcnr commented Apr 2, 2024

This comment has been minimized.

bors commented Apr 2, 2024

lcnr commented Apr 2, 2024

scottmcm commented Apr 2, 2024 • edited Loading

bors commented Apr 2, 2024

This comment has been minimized.

rust-timer commented Apr 2, 2024

Overall result: ❌✅ regressions and improvements - no action needed

matthewjasper commented Apr 2, 2024

lcnr Apr 2, 2024 • edited Loading

Choose a reason for hiding this comment

lcnr commented Apr 2, 2024

bors commented Apr 2, 2024

bors commented Apr 2, 2024

compiler-errors commented Apr 2, 2024

bors commented Apr 2, 2024

bors commented Apr 3, 2024

bors commented Apr 3, 2024

lcnr commented Apr 3, 2024

bors commented Apr 3, 2024

bors commented Apr 3, 2024

bors commented Apr 3, 2024

rust-timer commented Apr 3, 2024

Overall result: ❌✅ regressions and improvements - no action needed

scottmcm commented Apr 2, 2024 •

edited

Loading

lcnr Apr 2, 2024 •

edited

Loading