Track drop points by MIR Place, splitting partially-moved locals (fix #121, #122)#155
Draft
coord-e wants to merge 2 commits into
Draft
Track drop points by MIR Place, splitting partially-moved locals (fix #121, #122)#155coord-e wants to merge 2 commits into
coord-e wants to merge 2 commits into
Conversation
This was referenced Jun 28, 2026
ecb1dc8 to
e2f735e
Compare
A local with a partial field move (e.g. `move (_2.0)`) was still dropped wholesale, so dropping it walked into the moved-out sub-place and resolved the `&mut` prophecy it owns a second time. The two resolutions contradict, making the clause body unsatisfiable, after which any assertion -- including false ones -- "verifies" (#121, #122). Produce correct drop points instead of patching the env drop-walk: - `DropPoints` stores a `PlaceSet { locals, places }` per position, so whole-local drops -- the common case -- stay as bare `Local`s and only genuinely place-granular drops (closure environments) are tracked as `Place`s. The analyzer drops through a single `drop_place_set` helper. - `Moves::collect` gathers all non-reference `move`d operands in one body traversal: whole-local moves (keyed by location, where the local also dies, for the per-statement drop-set subtraction) and the set of locals with a partial field move. A partially-moved local stays live until its remaining parts die later, so dropping it wholesale at that point would re-resolve the moved-out sub-place's prophecy; it is excluded from dropping entirely. Reference-typed moves are skipped: reborrows keep the source owning its prophecy. Adds regression tests for both shapes (partial move into a local, and a `&mut`-bearing field moved into a call). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NU1g7aHMxcSEZgeKProDr1
e2f735e to
440988e
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Supersedes #154 and #124. Fixes #121 and #122.
Motivation
Placeso implicit drops are tracked at the correct place granularity (the Track drop points by MIR Place #154 generalization).&mutborrows #121/Unsound: aggregate dropped wholesale after a partial field-move double-resolves the field's &mut prophecy #122 double-resolution bug.#154alone generalized the representation but did not fix the underlying issue: a local with a partial field move (e.g.move (_2.0)) was still dropped wholesale, so the drop walked into the moved-out sub-place and resolved the&mutprophecy it owns a second time. The two resolutions contradict (final = 1∧final = 2), making the clause body unsatisfiable, after which any assertion — including false ones — "verifies".Description
DropPointsis generic over the MIR lifetime and storesmir::Place<'tcx>inbefore_statements,after_statements,after_terminator, andafter_terminator_extra. Liveness-derivedLocalsets are converted toPlaces in the builder, and the basic-blockAnalyzerroutes all implicit-drop handling throughdrop_place/drop_after_terminatorworking onmir::Place<'tcx>.partial_movescollects the sub-places moved out of each local somewhere in the body;expand_owned_placesthen splits a to-be-dropped local along its type into the maximal still-owned sub-places, carving out moved-out subtrees while still dropping their owned siblings. A wholly-moved place expands to nothing; a place with no moved-out descendant is emitted whole (preserving prior behavior). Reference-typed moves are left in place, sinceReborrowVisitor/RustCallVisitorturn them into reborrows and the source keeps owning its prophecy.HashSetinstead ofVec, removing the manual dedup bookkeeping.Testing
partial_move_drop.rs— Unsoundness: partially-moved locals are still implicitly dropped, resolving prophecies of moved-out&mutborrows #121's partial-move-into-local (let b = s.0;).partial_move_field_call.rs— Unsound: aggregate dropped wholesale after a partial field-move double-resolves the field's &mut prophecy #122's partial-move-into-call (owned(&mut i64,)field passed to a function).Unsat(rejected) where Track drop points by MIR Place #154 alone wrongly accepted it; the true-assertion companions still verify, confirming the drop is not over-suppressed.cargo fmt --all -- --check,cargo clippy -- -D warnings, andgit diff --checkare clean.🤖 Generated with Claude Code
Generated by Claude Code