Skip to content

Drop elaboration: Only create a reset block if there are flags to reset. - #159195

Open
cjgillot wants to merge 1 commit into
rust-lang:mainfrom
cjgillot:elaborate-drop-flags
Open

Drop elaboration: Only create a reset block if there are flags to reset.#159195
cjgillot wants to merge 1 commit into
rust-lang:mainfrom
cjgillot:elaborate-drop-flags

Conversation

@cjgillot

Copy link
Copy Markdown
Contributor

Follow-up to #157491

I'm not totally convinced this is worth the effort. The generated pattern is trivially cleaned-up by SimplifyCfg, so we should not need to bother.

r? @saethlin since you reviewed the earlier PR

@rustbot

rustbot commented Jul 12, 2026

Copy link
Copy Markdown
Collaborator

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

@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 Jul 12, 2026
@cjgillot

Copy link
Copy Markdown
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 Jul 12, 2026
@rust-bors

This comment has been minimized.

rust-bors Bot pushed a commit that referenced this pull request Jul 12, 2026
Drop elaboration: Only create a reset block if there are flags to reset.
@rust-bors

rust-bors Bot commented Jul 12, 2026

Copy link
Copy Markdown
Contributor

☀️ Try build successful (CI)
Build commit: 10b8371 (10b8371bc23fc1cd123a8c30d6080c5322016dc8)
Base parent: 77cf889 (77cf889bc178ddb44d6a1c78e5a820b5abb31d8d)

@rust-timer

This comment has been minimized.

@rust-timer

Copy link
Copy Markdown
Collaborator

Finished benchmarking commit (10b8371): comparison URL.

Overall result: no relevant changes - no action needed

Benchmarking means the PR may be perf-sensitive. Consider adding rollup=never if this change is not fit for rolling up.

@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.9%)

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.6% [1.0%, 5.4%] 4
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-1.6% [-2.4%, -0.9%] 2
All ❌✅ (primary) - - 0

Cycles

Results (secondary 2.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)
4.4% [2.1%, 9.3%] 6
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-2.7% [-3.4%, -2.1%] 3
All ❌✅ (primary) - - 0

Binary size

Results (secondary -0.4%)

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)
- - 0
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-0.4% [-0.6%, -0.3%] 10
All ❌✅ (primary) - - 0

Bootstrap: 489.838s -> 490.392s (0.11%)
Artifact size: 389.25 MiB -> 389.89 MiB (0.16%)

@rustbot rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Jul 12, 2026
@saethlin

saethlin commented Aug 1, 2026

Copy link
Copy Markdown
Member

@bors r+ rollup=iffy (look at the mir-opt diff, this is going to be conflict-prone)

@rust-bors

rust-bors Bot commented Aug 1, 2026

Copy link
Copy Markdown
Contributor

📌 Commit 6f2cdf7 has been approved by saethlin

It is now in the queue for this repository.

@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 Aug 1, 2026
jhpratt added a commit to jhpratt/rust that referenced this pull request Aug 1, 2026
…aethlin

Drop elaboration: Only create a reset block if there are flags to reset.

Follow-up to rust-lang#157491

I'm not totally convinced this is worth the effort. The generated pattern is trivially cleaned-up by SimplifyCfg, so we should not need to bother.

r? @saethlin since you reviewed the earlier PR
jhpratt added a commit to jhpratt/rust that referenced this pull request Aug 1, 2026
…aethlin

Drop elaboration: Only create a reset block if there are flags to reset.

Follow-up to rust-lang#157491

I'm not totally convinced this is worth the effort. The generated pattern is trivially cleaned-up by SimplifyCfg, so we should not need to bother.

r? @saethlin since you reviewed the earlier PR
@jhpratt

jhpratt commented Aug 1, 2026

Copy link
Copy Markdown
Member

This may be the cause of #160308.

@bors r- #160308 (comment)

@bors try jobs=x86_64-gnu-llvm-21

test [mir-opt] tests/mir-opt/unreachable_enum_branching.rs ... ok

failures:

---- [mir-opt] tests/mir-opt/coroutine/async_drop.rs stdout ----
65 +         _26 = std::future::ResumeTy(move _27);
66 +         _25 = copy (_1.0: &mut {async fn body of array()});
67 +         _24 = discriminant((*_25));
- +         switchInt(move _24) -> [0: bb26, 1: bb25, 2: bb24, 3: bb22, 4: bb23, otherwise: bb11];
+ +         switchInt(move _24) -> [0: bb25, 1: bb24, 2: bb23, 3: bb21, 4: bb22, otherwise: bb11];
69       }
70   
71       bb1: {

85 -         StorageDead(_3);
86 -         drop(_1) -> [return: bb4, unwind: bb8];
87 +         nop;
- +         goto -> bb20;
+ +         goto -> bb4;
89       }
90   
91       bb4: {

104 -     bb6: {
---
108       }
109   
110 -     bb7 (cleanup): {

201 +         _21 = Pin::<&mut [AsyncInt; 2]>::new_unchecked(move _22) -> [return: bb18, unwind: bb5];
202       }
203   
-       bb20: {
+ -     bb20: {
205 -         _13 = &mut _6;
206 -         _10 = Pin::<&mut impl Future<Output = ()>>::new_unchecked(move _13) -> [return: bb19, unwind: bb11];
- +         goto -> bb4;
+ +     bb20 (cleanup): {
+ +         discriminant((*_25)) = 2;
+ +         resume;
208       }
209   
- -     bb21: {
+       bb21: {
211 -         _2 = move _14;
212 -         StorageDead(_14);
213 -         goto -> bb27;

- +     bb21 (cleanup): {
- +         discriminant((*_25)) = 2;
- +         resume;
+ +         StorageLive(_7);
+ +         _7 = move _26;
+ +         goto -> bb10;
217       }
218   
219       bb22: {

220 -         _2 = move _14;
---
227   
228       bb23: {

-           StorageLive(_14);
+ -         StorageLive(_14);
230 -         _14 = yield(const ()) -> [resume: bb21, drop: bb22];
- +         _14 = move _26;
- +         goto -> bb12;
+ +         assert(const false, "`async fn` resumed after panicking") -> [success: bb23, unwind continue];
233       }
234   
235       bb24: {

236 -         _16 = discriminant(_15);
237 -         switchInt(move _16) -> [0: bb9, 1: bb23, otherwise: bb16];
- +         assert(const false, "`async fn` resumed after panicking") -> [success: bb24, unwind continue];
+ +         assert(const false, "`async fn` resumed after completion") -> [success: bb24, unwind continue];
239       }
240   
241       bb25: {

242 -         _15 = <impl Future<Output = ()> as Future>::poll(move _17, move _18) -> [return: bb24, unwind: bb11];
- +         assert(const false, "`async fn` resumed after completion") -> [success: bb25, unwind continue];
-       }
-   
-       bb26: {
+ -     }
+ - 
+ -     bb26: {
247 -         _19 = move _2;
248 -         _18 = std::future::get_context::<'_, '_>(move _19) -> [return: bb25, unwind: bb11];
249 -     }


thread '[mir-opt] tests/mir-opt/coroutine/async_drop.rs' panicked at src/tools/compiletest/src/runtest/mir_opt.rs:73:21:
Actual MIR output differs from expected MIR output /checkout/tests/mir-opt/coroutine/async_drop.array-{closure#0}.StateTransform.diff
stack backtrace:
   5: __rustc::rust_begin_unwind
             at /rustc/08d5b675a9b2abdca5e2fe4eabe0e07bbda15d49/library/std/src/panicking.rs:679:5
   6: core::panicking::panic_fmt
             at /rustc/08d5b675a9b2abdca5e2fe4eabe0e07bbda15d49/library/core/src/panicking.rs:80:14

@rust-bors rust-bors Bot 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 Aug 1, 2026
@rust-bors

rust-bors Bot commented Aug 1, 2026

Copy link
Copy Markdown
Contributor

This pull request was unapproved.

View changes since this unapproval

@rust-bors

This comment has been minimized.

rust-bors Bot pushed a commit that referenced this pull request Aug 1, 2026
Drop elaboration: Only create a reset block if there are flags to reset.


try-job: x86_64-gnu-llvm-21
@rust-bors

rust-bors Bot commented Aug 1, 2026

Copy link
Copy Markdown
Contributor

💔 Test for a605958 failed: CI. Failed job:

@rust-log-analyzer

Copy link
Copy Markdown
Collaborator

The job x86_64-gnu-llvm-21 failed! Check out the build log: (web) (plain enhanced) (plain)

Click to see the possible cause of the failure (guessed by this bot)
test [mir-opt] tests/mir-opt/unusual_item_types.rs ... ok

failures:

---- [mir-opt] tests/mir-opt/coroutine/async_drop.rs stdout ----
65 +         _26 = std::future::ResumeTy(move _27);
66 +         _25 = copy (_1.0: &mut {async fn body of array()});
67 +         _24 = discriminant((*_25));
- +         switchInt(move _24) -> [0: bb26, 1: bb25, 2: bb24, 3: bb22, 4: bb23, otherwise: bb11];
+ +         switchInt(move _24) -> [0: bb25, 1: bb24, 2: bb23, 3: bb21, 4: bb22, otherwise: bb11];
69       }
70   
71       bb1: {

85 -         StorageDead(_3);
86 -         drop(_1) -> [return: bb4, unwind: bb8];
87 +         nop;
- +         goto -> bb20;
+ +         goto -> bb4;
89       }
90   
91       bb4: {

104 -     bb6: {
---
108       }
109   
110 -     bb7 (cleanup): {

201 +         _21 = Pin::<&mut [AsyncInt; 2]>::new_unchecked(move _22) -> [return: bb18, unwind: bb5];
202       }
203   
-       bb20: {
+ -     bb20: {
205 -         _13 = &mut _6;
206 -         _10 = Pin::<&mut impl Future<Output = ()>>::new_unchecked(move _13) -> [return: bb19, unwind: bb11];
- +         goto -> bb4;
+ +     bb20 (cleanup): {
+ +         discriminant((*_25)) = 2;
+ +         resume;
208       }
209   
- -     bb21: {
+       bb21: {
211 -         _2 = move _14;
212 -         StorageDead(_14);
213 -         goto -> bb27;

- +     bb21 (cleanup): {
- +         discriminant((*_25)) = 2;
- +         resume;
+ +         StorageLive(_7);
+ +         _7 = move _26;
+ +         goto -> bb10;
217       }
218   
219       bb22: {

220 -         _2 = move _14;
---
227   
228       bb23: {

-           StorageLive(_14);
+ -         StorageLive(_14);
230 -         _14 = yield(const ()) -> [resume: bb21, drop: bb22];
- +         _14 = move _26;
- +         goto -> bb12;
+ +         assert(const false, "`async fn` resumed after panicking") -> [success: bb23, unwind continue];
233       }
234   
235       bb24: {

236 -         _16 = discriminant(_15);
237 -         switchInt(move _16) -> [0: bb9, 1: bb23, otherwise: bb16];
- +         assert(const false, "`async fn` resumed after panicking") -> [success: bb24, unwind continue];
+ +         assert(const false, "`async fn` resumed after completion") -> [success: bb24, unwind continue];
239       }
240   
241       bb25: {

242 -         _15 = <impl Future<Output = ()> as Future>::poll(move _17, move _18) -> [return: bb24, unwind: bb11];
- +         assert(const false, "`async fn` resumed after completion") -> [success: bb25, unwind continue];
-       }
-   
-       bb26: {
+ -     }
+ - 
+ -     bb26: {
247 -         _19 = move _2;
248 -         _18 = std::future::get_context::<'_, '_>(move _19) -> [return: bb25, unwind: bb11];
249 -     }


thread '[mir-opt] tests/mir-opt/coroutine/async_drop.rs' panicked at src/tools/compiletest/src/runtest/mir_opt.rs:73:21:
Actual MIR output differs from expected MIR output /checkout/tests/mir-opt/coroutine/async_drop.array-{closure#0}.StateTransform.diff
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
---- [mir-opt] tests/mir-opt/coroutine/async_drop.rs stdout end ----

failures:
    [mir-opt] tests/mir-opt/coroutine/async_drop.rs

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants