Make needs_drop less pessimistic on generators#70015
Make needs_drop less pessimistic on generators#70015bors merged 3 commits intorust-lang:masterfrom jonas-schievink:gen-needs-drop
needs_drop less pessimistic on generators#70015Conversation
|
r? @davidtwco (rust_highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
src/librustc_ty/needs_drop.rs
Outdated
There was a problem hiding this comment.
Not entirely sure the erase_late_bound_regions is completely correct here
|
@bors r+ |
|
📌 Commit 489d79d has been approved by |
bors
added
S-waiting-on-bors
…atthewjasper Make `needs_drop` less pessimistic on generators Generators only have non-trivial drop logic when they may store (in upvars or across yields) a type that does. This prevents generation of some unnecessary MIR in simple generators. There might be some impact on compile times, but this is probably limited in real-world applications. ~~This builds off of rust-lang#69814 since that contains some fixes that are made relevant by *this* PR (see rust-lang#69814 (comment) (this has been merged)
|
Failed in #70325 (comment), @bors r- |
bors
added
S-waiting-on-author
|
@bors r=matthewjasper |
|
📌 Commit 1df7641 has been approved by |
bors
added
S-waiting-on-bors
…atthewjasper Make `needs_drop` less pessimistic on generators Generators only have non-trivial drop logic when they may store (in upvars or across yields) a type that does. This prevents generation of some unnecessary MIR in simple generators. There might be some impact on compile times, but this is probably limited in real-world applications. ~~This builds off of rust-lang#69814 since that contains some fixes that are made relevant by *this* PR (see rust-lang#69814 (comment) (this has been merged)
…atthewjasper Make `needs_drop` less pessimistic on generators Generators only have non-trivial drop logic when they may store (in upvars or across yields) a type that does. This prevents generation of some unnecessary MIR in simple generators. There might be some impact on compile times, but this is probably limited in real-world applications. ~~This builds off of rust-lang#69814 since that contains some fixes that are made relevant by *this* PR (see rust-lang#69814 (comment) (this has been merged)
…atthewjasper Make `needs_drop` less pessimistic on generators Generators only have non-trivial drop logic when they may store (in upvars or across yields) a type that does. This prevents generation of some unnecessary MIR in simple generators. There might be some impact on compile times, but this is probably limited in real-world applications. ~~This builds off of rust-lang#69814 since that contains some fixes that are made relevant by *this* PR (see rust-lang#69814 (comment) (this has been merged)
|
That's weird. They pass locally even after a rebase and I'm pretty sure CI also runs mir-opt tests. |
|
Some CI builders have |
|
Ah, you're right. Can reproduce. |
|
I wonder why this wasn't hit before though – the failing test checks for cleanup blocks, which should be removed by the time the generator transform runs when |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
@jonas-schievink Ping from triage, would you mind rebasing and resolve the merge conflicts? Thanks. |
|
Rebased, and ignored the test on wasm32, like some other mir-opt tests. @bors r=matthewjasper rollup=never |
|
📌 Commit ae53315 has been approved by |
bors
added
S-waiting-on-bors
| } | ||
| } | ||
|
|
||
| ty::Generator(_, substs, _) => { |
There was a problem hiding this comment.
Do we have assertions somewhere that could catch bugs in needs_drop (i.e., catch cases where a type has drop glue but needs_drop says no)?
There was a problem hiding this comment.
Not that I know of, but we have several mir-opt tests for generators, as well as diagnostics tests, which were (correctly) affected by this change.
|
⌛ Testing commit ae53315 with merge ae96d19171e368760d20cfd76399a4e85b07f5d6... |
|
💔 Test failed - checks-azure |
bors
added
S-waiting-on-review
|
@bors retry |
bors
added
S-waiting-on-bors
|
☀️ Test successful - checks-azure |
8 participants
Generators only have non-trivial drop logic when they may store (in upvars or across yields) a type that does.
This prevents generation of some unnecessary MIR in simple generators. There might be some impact on compile times, but this is probably limited in real-world applications.
This builds off of #69814 since that contains some fixes that are made relevant by this PR (see #69814 (comment)).(this has been merged)