Don't drop partially-moved sub-places (fix #121, #122)#124
Draft
coord-e wants to merge 1 commit into
Draft
Conversation
`moved_locals` only excluded whole-local moves from the implicit-drop set, so a local with a partial field move (e.g. `move (_2.0)`) was still dropped wholesale. `Env::dropping_assumption` then walked the moved-out subtree and resolved the `&mut` prophecy it owns a second time, yielding contradictory constraints (`final = 1` and `final = 2`) under which any assertion — including false ones — verifies. Track partial field-moves per local (skipping ref-typed moves, which become reborrows and keep the source live) and skip the moved-out subtree when emitting the dropping assumption for the parent. Adds pass/fail regression tests for both the partial-move-into-local (#121) and partial-move-into-call (#122) shapes. https://claude.ai/code/session_01Fovh1a8R5EtDd98ic9EJfR
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.
Summary
Fixes #121 and #122, which share a single root cause.
moved_locals(src/analyze/basic_block/drop_point.rs) only excluded whole-local moves from the implicit-drop set. A local with a partial field move (e.g.move (_2.0)) was therefore still dropped wholesale once it became dead.Env::dropping_assumption(src/refine/env.rs) then walked the local's full type — including the part that was moved away — and resolved the&mutprophecy owned by the moved-out sub-place a second time, with a different current value. The two resolutions contradict (final = 1∧final = 2), making the clause body unsatisfiable, after which any assertion — including false ones — "verifies."Fix
drop_point.rs: addpartial_moved_places, which collects every partial field-move (move placewith a non-empty projection) per local. Reference-typed moves are skipped for the same reasonmoved_localsskips them:ReborrowVisitor/RustCallVisitorturn them into reborrows, so the source still owns its prophecy and must be dropped normally.basic_block.rs: store that map on the analyzer (recomputed wheneverbodychanges) and pass the relevant moved sub-places to the env when dropping a local.env.rs:dropping_assumptionnow takes the moved sub-places and short-circuits any subtree whose path matches a moved-out place (newPath::same_placestructural comparison).drop_local_with_movedis the new entry point;drop_localdelegates with an empty slice.Tests
New pass/fail regression tests for both shapes:
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).Each false-assertion variant now reports
Unsat(rejected) where it was previously accepted; the true-assertion companions still verify, confirming the drop is not over-suppressed.Full UI suite passes with no regressions (pcsat solver via Docker).
Note on #122's literal reproduction
The exact
Wrap { r: &'a mut i32 }example in #122 — where the moved field is itself&mut-typed — does not reach this drop on the current toolchain. In optimized MIR that field becomes a reborrow (deref_copy/copy), not an owned move, and analyzingapplypanics earlier in an unrelated, pre-existing spot (reborrowing a&mutfield out of a struct parameter). That panic is filed separately. This PR fixes the shared drop double-resolution;partial_move_field_call.rsexercises #122's intended shape using an owned aggregate field.https://claude.ai/code/session_01Fovh1a8R5EtDd98ic9EJfR
Generated by Claude Code