Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clarity improvements to DropTree #122080

Merged
merged 4 commits into from Mar 11, 2024
Merged

Clarity improvements to DropTree #122080

merged 4 commits into from Mar 11, 2024

Conversation

Zalathar
Copy link
Contributor

@Zalathar Zalathar commented Mar 6, 2024

These changes are based on some points of confusion I had when initially trying to understand this code.

The only “functional” change is an additional assertion in <ExitScopes as DropTreeBuilder>::link_entry_point, checking that the dummy terminator is TerminatorKind::UnwindResume as expected.

This clarifies that we're adding an "entry point", not just adding an "entry"
of some kind.
This allows us to use real field names instead of tuple element numbers.

Renaming `previous_drops` to `existing_drops_map` clarifies that "previous" was
unrelated to drop order.
@rustbot
Copy link
Collaborator

rustbot commented Mar 6, 2024

r? @fmease

rustbot has assigned @fmease.
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 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 Mar 6, 2024
@Zalathar
Copy link
Contributor Author

Zalathar commented Mar 6, 2024

Originally I had added a debug_assert!, but looking at the other implementations I noticed that they both have some kind of span_bug! assertion anyway, so adding one to the ExitScopes impl seems unobtrusive.

@fmease
Copy link
Member

fmease commented Mar 9, 2024

Thanks! Generally makes sense but I don't feel comfortable approving this as I'm not super familiar with MIR (yet).

r? @oli-obk

@rustbot rustbot assigned oli-obk and unassigned fmease Mar 9, 2024
@oli-obk
Copy link
Contributor

oli-obk commented Mar 11, 2024

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Mar 11, 2024

📌 Commit 5ba70bd has been approved by oli-obk

It is now in the queue for this repository.

@bors 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 Mar 11, 2024
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Mar 11, 2024
Clarity improvements to `DropTree`

These changes are based on some points of confusion I had when initially trying to understand this code.

The only “functional” change is an additional assertion in `<ExitScopes as DropTreeBuilder>::link_entry_point`, checking that the dummy terminator is `TerminatorKind::UnwindResume` as expected.
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 11, 2024
…kingjubilee

Rollup of 15 pull requests

Successful merges:

 - rust-lang#116791 (Allow codegen backends to opt-out of parallel codegen)
 - rust-lang#116793 (Allow targets to override default codegen backend)
 - rust-lang#117458 (LLVM Bitcode Linker: A self contained linker for nvptx and other targets)
 - rust-lang#119385 (Fix type resolution of associated const equality bounds (take 2))
 - rust-lang#121438 (std support for wasm32 panic=unwind)
 - rust-lang#121893 (Add tests (and a bit of cleanup) for interior mut handling in promotion and const-checking)
 - rust-lang#122080 (Clarity improvements to `DropTree`)
 - rust-lang#122152 (Improve diagnostics for parenthesized type arguments)
 - rust-lang#122166 (Remove the unused `field_remapping` field from `TypeLowering`)
 - rust-lang#122249 (interpret: do not call machine read hooks during validation)
 - rust-lang#122299 (Store backtrace for `must_produce_diag`)
 - rust-lang#122318 (Revision-related tweaks for next-solver tests)
 - rust-lang#122320 (Use ptradd for vtable indexing)
 - rust-lang#122328 (unix_sigpipe: Replace `inherit` with `sig_dfl` in syntax tests)
 - rust-lang#122330 (bootstrap readme: fix, improve, update)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit e3f9b2f into rust-lang:master Mar 11, 2024
11 checks passed
@rustbot rustbot added this to the 1.78.0 milestone Mar 11, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Mar 11, 2024
Rollup merge of rust-lang#122080 - Zalathar:drop-tree, r=oli-obk

Clarity improvements to `DropTree`

These changes are based on some points of confusion I had when initially trying to understand this code.

The only “functional” change is an additional assertion in `<ExitScopes as DropTreeBuilder>::link_entry_point`, checking that the dummy terminator is `TerminatorKind::UnwindResume` as expected.
@Zalathar Zalathar deleted the drop-tree branch March 11, 2024 20:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. 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.

None yet

5 participants