Skip to content

Deref is not a projection: VarDebugInfo #146710

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

Deref is not a projection: VarDebugInfo #146710

wants to merge 7 commits into from

Conversation

beepster4096
Copy link
Contributor

@beepster4096 beepster4096 commented Sep 18, 2025 •
edited
Loading

See rust-lang/compiler-team#532

The Derefer MIR pass ensures that all places' projections are an optional deref followed by a direct projection (no derefs, points to the same region of memory), except in VarDebugInfo. There is no way to represent debug info's places in general like this.

To solve this, this PR introduces CompoundPlace and CompoundPlaceRef to for places with multiple derefs in arbitrary positions by instead representing them as multiple direct projections seperated by derefs. These types are also necessary for moving Derefer before borrowck because having to deal with these split-up single-deref places in borrowck would be a nightmare (I tried already).

Couple points of uncertainty:

  • I put them in rustc_middle::mir::statement along side Place's methods and PlaceRef, not sure if that's the best place
  • CompoundPlace is just turned into a regular Place in stable MIR. Should it?
  • Visitor changes feel like a big hack

r? oli-obk (not sure if they're still on vacation)

@rustbot
Copy link
Collaborator

rustbot commented Sep 18, 2025

Some changes occurred in compiler/rustc_codegen_ssa

cc @WaffleLapkin

This PR changes rustc_public

cc @oli-obk, @celinval, @ouz-a

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 Sep 18, 2025
@rustbot
Copy link
Collaborator

rustbot commented Sep 18, 2025

oli-obk is not on the review rotation at the moment.
They may take a while to respond.

@beepster4096 beepster4096 force-pushed the did_you_think_you_could_escape_derefer_just_because_you_arent_in_the_basic_blocks branch from 5c72b0b to bc84a6c Compare September 18, 2025 09:07
@rustbot

This comment has been minimized.

Sign in to view
@beepster4096
Copy link
Contributor Author

r? mir

@rustbot rustbot assigned saethlin and unassigned oli-obk Sep 18, 2025
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@beepster4096 beepster4096 force-pushed the did_you_think_you_could_escape_derefer_just_because_you_arent_in_the_basic_blocks branch from bc84a6c to 1101f1b Compare September 18, 2025 20:11
@cjgillot
Copy link
Contributor

Do you mind extracting the changes to ref_prop into a standalone PR?

@cjgillot cjgillot self-assigned this Sep 19, 2025
@beepster4096 beepster4096 mentioned this pull request Sep 19, 2025
Merged
bors added a commit that referenced this pull request Sep 20, 2025
@bors
Auto merge of #146744 - beepster4096:derefs_in_ref_prop, r=cjgillot
a213de7 
Deref related cleanups in ref_prop

Cherry picked from #146710

r? cjgillot
tgross35 added a commit to tgross35/rust that referenced this pull request Sep 20, 2025
@tgross35
Rollup merge of rust-lang#146744 - beepster4096:derefs_in_ref_prop, r…
e4a5c3a 
…=cjgillot

Deref related cleanups in ref_prop

Cherry picked from rust-lang#146710

r? cjgillot
tgross35 added a commit to tgross35/rust that referenced this pull request Sep 20, 2025
@tgross35
Rollup merge of rust-lang#146744 - beepster4096:derefs_in_ref_prop, r…
9f73b06 
…=cjgillot

Deref related cleanups in ref_prop

Cherry picked from rust-lang#146710

r? cjgillot
Zalathar added a commit to Zalathar/rust that referenced this pull request Sep 21, 2025
@Zalathar
Rollup merge of rust-lang#146744 - beepster4096:derefs_in_ref_prop, r…
5c1c479 
…=cjgillot

Deref related cleanups in ref_prop

Cherry picked from rust-lang#146710

r? cjgillot
rust-timer added a commit that referenced this pull request Sep 21, 2025
@rust-timer
Unrolled build for #146744
c89077e 
Rollup merge of #146744 - beepster4096:derefs_in_ref_prop, r=cjgillot

Deref related cleanups in ref_prop

Cherry picked from #146710

r? cjgillot
@beepster4096 beepster4096 force-pushed the did_you_think_you_could_escape_derefer_just_because_you_arent_in_the_basic_blocks branch from 1101f1b to d32ca9b Compare September 22, 2025 04:33
@rustbot
Copy link
Collaborator

rustbot commented Sep 22, 2025

This PR was rebased onto a different master commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

@celinval
Copy link
Contributor 

* `CompoundPlace` is just turned into a regular `Place` in stable MIR. Should it?

The stable Mir currently supports the after analysis MIR only. Is there any semantic difference at that level between CompoundPlace and regular Place? If not, you can just translate it to Place. Otherwise, please add a CompoundPlace. Thanks

Muscraft pushed a commit to Muscraft/rust that referenced this pull request Sep 24, 2025
@Zalathar @Muscraft
Rollup merge of rust-lang#146744 - beepster4096:derefs_in_ref_prop, r…
5c2d889 
…=cjgillot

Deref related cleanups in ref_prop

Cherry picked from rust-lang#146710

r? cjgillot
@beepster4096
Copy link
Contributor Author

Yes, it probably makes sense to add CompoundPlace to stable MIR then. It would need it eventually when ProjectionElem::Deref is fully removed anyway.

@bors
Copy link
Collaborator

bors commented Oct 3, 2025

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

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

@cjgillot cjgillot

@saethlin saethlin

Labels
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.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@beepster4096 @rustbot @rust-log-analyzer @cjgillot @celinval @bors @oli-obk @saethlin