Don't try to remove `drop_in_place` calls in `RemoveUnneededDrops` by WaffleLapkin · Pull Request #155044 · rust-lang/rust

WaffleLapkin · 2026-04-09T14:26:16Z

As per my justification in #154327 (comment)

r? scottmcm

rustbot · 2026-04-09T14:26:19Z

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

WaffleLapkin · 2026-04-09T14:27:25Z

tests/mir-opt/remove_unneeded_drop_in_place.rs

 unsafe fn slice_in_place(ptr: *mut [char]) {
    // CHECK-LABEL: fn slice_in_place(_1: *mut [char])
-    // CHECK: bb0: {
-    // CHECK-NEXT: return;
+    // CHECK:      bb0: {
+    // CHECK-NEXT:   StorageLive(_2);
+    // CHECK-NEXT:   _2 = copy _1;
+    // CHECK-NEXT:   StorageDead(_2);
+    // CHECK-NEXT:   return;
    // CHECK-NEXT: }
    std::ptr::drop_in_place(ptr)
 }


View changes since the review

idk how this used to pass, given that the diff is unchanged...

You removed the compile-flags, so that probably impacted it? The filecheck validations are on final mir always...

scottmcm

Had a suggestion, but r=me.

I don't remember this ever being all that critical, so if nothing's failing without it, might as well make it simpler.

View changes since this review

tests/mir-opt/inline/inline_empty_drop_glue.rs

Kobzol · 2026-04-10T13:51:00Z

@bors try @rust-timer queue

Just in case :)

Don't try to remove `drop_in_place` calls in `RemoveUnneededDrops`

rust-bors · 2026-04-10T16:01:42Z

☀️ Try build successful (CI)
Build commit: 7c3a7ce (7c3a7cee12ab3b78727df15301d06630f5abfac6, parent: 25a54d4ba87873ef02ef0e5fce49baa57198deb3)

rust-timer · 2026-04-10T16:48:18Z

Finished benchmarking commit (7c3a7ce): comparison URL.

Overall result: no relevant changes - no action needed

Benchmarking means the PR may be perf-sensitive. It's automatically marked not fit for rolling up. Overriding is possible but disadvised: it risks changing compiler perf.

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

Instruction count

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

Max RSS (memory usage)

Results (secondary -1.3%)

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

Cycles

Results (secondary 9.1%)

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)	10.8%	[5.8%, 14.7%]	7
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-3.1%	[-3.1%, -3.1%]	1
All ❌✅ (primary)	-	-	0

Binary size

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

Bootstrap: 488.4s -> 489.587s (0.24%)
Artifact size: 395.55 MiB -> 395.52 MiB (-0.01%)

Kobzol · 2026-04-10T17:52:55Z

@bors rollup=maybe

scottmcm · 2026-04-10T17:55:02Z

My suggestion was addressed and CI is clean
@bors r+

rust-bors · 2026-04-10T17:55:05Z

📌 Commit fa73f03 has been approved by scottmcm

It is now in the queue for this repository.

rust-bors · 2026-04-11T04:16:51Z

☀️ Test successful - CI
Approved by: scottmcm
Duration: 3h 21m 20s
Pushing c7c14d4 to main...

github-actions · 2026-04-11T04:19:22Z

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 d1d3ef4 (parent) -> c7c14d4 (this PR)

Test differences

Show 7 test diffs

Stage 1

[mir-opt] tests/mir-opt/inline/inline_empty_drop_glue.rs: [missing] -> pass (J1)
[mir-opt] tests/mir-opt/remove_unneeded_drop_in_place.rs: pass -> [missing] (J1)

Stage 2

[mir-opt] tests/mir-opt/inline/inline_empty_drop_glue.rs: [missing] -> pass (J0)
[mir-opt] tests/mir-opt/remove_unneeded_drop_in_place.rs: pass -> [missing] (J0)
[run-make] tests/run-make/compressed-debuginfo-zstd: ignore (ignored if LLVM wasn't build with zstd for ELF section compression or LLVM is not the default codegen backend) -> pass (J2)

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

Job group index

Test dashboard

Run

cargo run --manifest-path src/ci/citool/Cargo.toml -- \
    test-dashboard c7c14d4fb0310429e05e1a24dcf879053ee25b09 --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

pr-check-1: 47m 37s -> 31m 16s (-34.3%)
x86_64-rust-for-linux: 1h 10m -> 48m 58s (-30.4%)
x86_64-gnu-tools: 1h 23m -> 1h 1m (-26.3%)
pr-check-2: 57m 27s -> 43m (-25.1%)
x86_64-gnu-miri: 1h 54m -> 1h 28m (-23.1%)
optional-x86_64-gnu-parallel-frontend: 3h 17m -> 2h 31m (-23.0%)
x86_64-gnu-stable: 2h 48m -> 2h 12m (-21.5%)
armhf-gnu: 1h 48m -> 1h 26m (-20.8%)
dist-x86_64-apple: 1h 52m -> 2h 15m (+19.9%)
x86_64-gnu-nopt: 2h 51m -> 2h 18m (-19.1%)

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 · 2026-04-11T04:58:47Z

Finished benchmarking commit (c7c14d4): 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 (secondary 6.0%)

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

Cycles

Results (primary 0.8%, secondary -2.4%)

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

	mean	range	count
Regressions ❌ (primary)	2.7%	[2.7%, 2.7%]	1
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-1.1%	[-1.1%, -1.1%]	1
Improvements ✅ (secondary)	-2.4%	[-2.4%, -2.4%]	1
All ❌✅ (primary)	0.8%	[-1.1%, 2.7%]	2

Binary size

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

Bootstrap: 491.73s -> 490.09s (-0.33%)
Artifact size: 394.13 MiB -> 394.19 MiB (0.01%)

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 9, 2026

rustbot assigned scottmcm Apr 9, 2026

WaffleLapkin commented Apr 9, 2026

View reviewed changes

scottmcm approved these changes Apr 9, 2026

View reviewed changes

tests/mir-opt/inline/inline_empty_drop_glue.rs Show resolved Hide resolved

don't try to remove drop_in_place calls in RemoveUnneededDrops

fa73f03

WaffleLapkin force-pushed the dont-remove-unneeded-drops branch from 2a174db to fa73f03 Compare April 10, 2026 07:33

This comment has been minimized.

Sign in to view

rust-bors bot pushed a commit that referenced this pull request Apr 10, 2026

Auto merge of #155044 - WaffleLapkin:dont-remove-unneeded-drops, r=<try>

7c3a7ce

Don't try to remove `drop_in_place` calls in `RemoveUnneededDrops`

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

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 10, 2026

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 Apr 10, 2026

This comment has been minimized.

Sign in to view

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 Apr 11, 2026

rust-bors bot merged commit c7c14d4 into rust-lang:main Apr 11, 2026
13 checks passed

rustbot added this to the 1.97.0 milestone Apr 11, 2026

WaffleLapkin deleted the dont-remove-unneeded-drops branch April 11, 2026 16:30

scottmcm mentioned this pull request Apr 12, 2026

change the type of the argument of drop_in_place lang item to &mut _ #154327

Open

Uh oh!

Conversation

WaffleLapkin commented Apr 9, 2026

Uh oh!

rustbot commented Apr 9, 2026

Uh oh!

WaffleLapkin Apr 9, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

scottmcm left a comment • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Kobzol commented Apr 10, 2026

Uh oh!

This comment has been minimized.

This comment has been minimized.

rust-bors bot commented Apr 10, 2026

Uh oh!

This comment has been minimized.

rust-timer commented Apr 10, 2026

Overall result: no relevant changes - no action needed

Uh oh!

Kobzol commented Apr 10, 2026

Uh oh!

scottmcm commented Apr 10, 2026

Uh oh!

rust-bors bot commented Apr 10, 2026

Uh oh!

This comment has been minimized.

rust-bors bot commented Apr 11, 2026

Uh oh!

Uh oh!

github-actions bot commented Apr 11, 2026

Test differences

Stage 1

Stage 2

Job duration changes

Uh oh!

rust-timer commented Apr 11, 2026

Overall result: no relevant changes - no action needed

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

WaffleLapkin Apr 9, 2026 •

edited by rustbot

Loading

scottmcm left a comment •

edited by rustbot

Loading