Skip to content

Reuse machinery from tail_expr_drop_order for if_let_rescope - #137455

Merged
bors merged 1 commit into
rust-lang:masterfrom
compiler-errors:drop-lint-dtor
Feb 28, 2025
Merged

Reuse machinery from tail_expr_drop_order for if_let_rescope#137455
bors merged 1 commit into
rust-lang:masterfrom
compiler-errors:drop-lint-dtor

Conversation

@compiler-errors

@compiler-errorscompiler-errors commented Feb 23, 2025

Copy link
Copy Markdown
Contributor

Namely, it defines its own extract_component_with_significant_dtor which is a bit more accurate than Ty::has_significant_drop, since it has a hard-coded list of types from the ecosystem which are opted out of the lint.1

Also, since we extract the dtors themselves, adopt the same label we use in tail_expr_drop_order to point out the destructor impl. This makes it much clear what's actually being dropped, so it should be clearer to know when it's a false positive.

This conflicts with #137444, but I will rebase whichever lands first.

Footnotes

  1. Side-note, it's kinda a shame that now there are two functions that presumably do the same thing. But this isn't my circus, nor are these my monkeys.

@rustbot

Copy link
Copy Markdown
Collaborator

r? @cjgillot

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

@rustbotrustbot 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 Feb 23, 2025
@rustbot

Copy link
Copy Markdown
Collaborator

These commits modify the Cargo.lock file. Unintentional changes to Cargo.lock can be introduced when switching branches and rebasing PRs.

If this was unintentional then you should revert the changes before this PR is merged.
Otherwise, you can ignore this comment.

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

@bors

bors commented Feb 24, 2025

Copy link
Copy Markdown
Collaborator

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

@bors

bors commented Feb 25, 2025

Copy link
Copy Markdown
Collaborator

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

@oli-obk

Copy link
Copy Markdown
Contributor

r? @oli-obk

@bors r+

@bors

bors commented Feb 27, 2025

Copy link
Copy Markdown
Collaborator

📌 Commit 864cca8 has been approved by oli-obk

It is now in the queue for this repository.

@borsbors 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 Feb 27, 2025
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Feb 27, 2025
…oli-obk
Reuse machinery from `tail_expr_drop_order` for `if_let_rescope`
Namely, it defines its own `extract_component_with_significant_dtor` which is a bit more accurate than `Ty::has_significant_drop`, since it has a hard-coded list of types from the ecosystem which are opted out of the lint.[^a]
Also, since we extract the dtors themselves, adopt the same *label* we use in `tail_expr_drop_order` to point out the destructor impl. This makes it much clear what's actually being dropped, so it should be clearer to know when it's a false positive.
This conflicts with rust-lang#137444, but I will rebase whichever lands first.
[^a]: Side-note, it's kinda a shame that now there are two functions that presumably do the same thing. But this isn't my circus, nor are these my monkeys.
This was referenced Feb 27, 2025
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 27, 2025
…iaskrgr
Rollup of 8 pull requests
Successful merges:
- rust-lang#136542 ([`compiletest`-related cleanups 4/7] Make the distinction between root build directory vs test suite specific build directory in compiletest less confusing)
- rust-lang#136579 (Fix UB in ThinVec::flat_map_in_place)
- rust-lang#136688 (require trait impls to have matching const stabilities as the traits)
- rust-lang#136846 (Make `AssocOp` more like `ExprKind`)
- rust-lang#137304 (add `IntoBounds::intersect` and `RangeBounds::is_empty`)
- rust-lang#137455 (Reuse machinery from `tail_expr_drop_order` for `if_let_rescope`)
- rust-lang#137480 (Return unexpected termination error instead of panicing in `Thread::join`)
- rust-lang#137694 (Spruce up `AttributeKind` docs)
r? `@ghost`
`@rustbot` modify labels: rollup
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Feb 27, 2025
…oli-obk
Reuse machinery from `tail_expr_drop_order` for `if_let_rescope`
Namely, it defines its own `extract_component_with_significant_dtor` which is a bit more accurate than `Ty::has_significant_drop`, since it has a hard-coded list of types from the ecosystem which are opted out of the lint.[^a]
Also, since we extract the dtors themselves, adopt the same *label* we use in `tail_expr_drop_order` to point out the destructor impl. This makes it much clear what's actually being dropped, so it should be clearer to know when it's a false positive.
This conflicts with rust-lang#137444, but I will rebase whichever lands first.
[^a]: Side-note, it's kinda a shame that now there are two functions that presumably do the same thing. But this isn't my circus, nor are these my monkeys.
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 27, 2025
Rollup of 4 pull requests
Successful merges:
- rust-lang#136579 (Fix UB in ThinVec::flat_map_in_place)
- rust-lang#137455 (Reuse machinery from `tail_expr_drop_order` for `if_let_rescope`)
- rust-lang#137480 (Return unexpected termination error instead of panicing in `Thread::join`)
- rust-lang#137694 (Spruce up `AttributeKind` docs)
r? `@ghost`
`@rustbot` modify labels: rollup
try-job: i686-msvc-1
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 27, 2025
…iaskrgr
Rollup of 8 pull requests
Successful merges:
- rust-lang#136542 ([`compiletest`-related cleanups 4/7] Make the distinction between root build directory vs test suite specific build directory in compiletest less confusing)
- rust-lang#136579 (Fix UB in ThinVec::flat_map_in_place)
- rust-lang#136688 (require trait impls to have matching const stabilities as the traits)
- rust-lang#136846 (Make `AssocOp` more like `ExprKind`)
- rust-lang#137304 (add `IntoBounds::intersect` and `RangeBounds::is_empty`)
- rust-lang#137455 (Reuse machinery from `tail_expr_drop_order` for `if_let_rescope`)
- rust-lang#137480 (Return unexpected termination error instead of panicing in `Thread::join`)
- rust-lang#137694 (Spruce up `AttributeKind` docs)
r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 28, 2025
…iaskrgr
Rollup of 8 pull requests
Successful merges:
- rust-lang#136542 ([`compiletest`-related cleanups 4/7] Make the distinction between root build directory vs test suite specific build directory in compiletest less confusing)
- rust-lang#136579 (Fix UB in ThinVec::flat_map_in_place)
- rust-lang#136688 (require trait impls to have matching const stabilities as the traits)
- rust-lang#136846 (Make `AssocOp` more like `ExprKind`)
- rust-lang#137304 (add `IntoBounds::intersect` and `RangeBounds::is_empty`)
- rust-lang#137455 (Reuse machinery from `tail_expr_drop_order` for `if_let_rescope`)
- rust-lang#137480 (Return unexpected termination error instead of panicing in `Thread::join`)
- rust-lang#137694 (Spruce up `AttributeKind` docs)
r? `@ghost`
`@rustbot` modify labels: rollup
@bors
bors merged commit 25db95e into rust-lang:masterFeb 28, 2025
@rustbotrustbot added this to the 1.87.0 milestone Feb 28, 2025
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Feb 28, 2025
Rollup merge of rust-lang#137455 - compiler-errors:drop-lint-dtor, r=oli-obk
Reuse machinery from `tail_expr_drop_order` for `if_let_rescope`
Namely, it defines its own `extract_component_with_significant_dtor` which is a bit more accurate than `Ty::has_significant_drop`, since it has a hard-coded list of types from the ecosystem which are opted out of the lint.[^a]
Also, since we extract the dtors themselves, adopt the same *label* we use in `tail_expr_drop_order` to point out the destructor impl. This makes it much clear what's actually being dropped, so it should be clearer to know when it's a false positive.
This conflicts with rust-lang#137444, but I will rebase whichever lands first.
[^a]: Side-note, it's kinda a shame that now there are two functions that presumably do the same thing. But this isn't my circus, nor are these my monkeys.
@smalis-msftsmalis-msft mentioned this pull request Feb 28, 2025
@smalis-msft

Copy link
Copy Markdown

Given that #137444 was beta-backported, can/should this PR be too?

@compiler-errors

Copy link
Copy Markdown
ContributorAuthor

Nope, this is not really as drastic to warrant backporting.

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

Labels

S-waiting-on-borsStatus: Waiting on bors to run and complete tests. Bors will change the label on completion.T-compilerRelevant 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

@compiler-errors@rustbot@bors@oli-obk@smalis-msft@cjgillot