Skip to content

Fix enum-drop of aggregate mutable-reference fields#167

Open
coord-e wants to merge 1 commit into
mainfrom
claude/review-commit-d44c6b8-hy774d
Open

Fix enum-drop of aggregate mutable-reference fields#167
coord-e wants to merge 1 commit into
mainfrom
claude/review-commit-d44c6b8-hy774d

Conversation

@coord-e

@coord-e coord-e commented Jul 4, 2026

Copy link
Copy Markdown
Owner

Bug

Dropping a value whose enum variant holds an aggregate of mutable references panics in refine/env.rs with assert!(assumption_existentials.is_empty()). Minimum repro (panics on current main during CHC generation):

enum Pair<'a> {
    Two((&'a mut i32, &'a mut i32)),
    None,
}

#[thrust::callable]
fn check(a: &mut i32, b: &mut i32) {
    let _p = Pair::Two((a, b)); // constructed and dropped — that's all it takes
}

fn main() {}

The same shape arises naturally from split_first_mut, whose Option<(&mut T, &mut [T])> return value hits this panic when dropped.

Cause

The enum-drop path in dropping_assumption built each field's drop assumption by re-deriving it through a fresh Path::PlaceTy. But projecting an aggregate field (PlaceType::tuple_proj / deref) allocates fresh existentials via subsume_rty, so the recursive call returns an assumption whose existentials the caller never merges — the assert guarded a case the code simply didn't handle. (A field that is directly &mut T produces no new existentials, which is why only aggregate fields trip it.)

Fix

Rewrite dropping_assumption around a structural drop_prophecy helper that walks the type once:

  • tuple elements and box targets are threaded as term expressions over the value (tuple_proj / box_current), so a nested sub-value stays shared across all the mutable references it contains and needs no fresh existentials;
  • only enum fields allocate existentials (they cannot be projected without a selector), and they go into a single shared namespace tied back to the value with the matcher predicate.

This resolves every packed prophecy to identity correctly. The now-dead Path::PlaceTy variant and Path::{deref,tuple_proj} helpers are removed.

Verification

  • The repro above no longer panics.
  • New regression tests tests/ui/{pass,fail}/enum_tuple_mut_drop.rs: the pass variant verifies drop resolves both prophecies to identity (sat), the fail variant confirms the encoding is not vacuous (*a == 11 is rejected as Unsat).
  • Full tests/ui suite compared against main: identical results everywhere else (the only failures in my environment are the docker-dependent pcsat tests, equally failing on main).

Note: this is the standalone extraction of the fix from d44c6b8; the tests/lambda-chc fixture changes in that commit depend on other unmerged work and are intentionally not included here.

🤖 Generated with Claude Code

https://claude.ai/code/session_01QmZGoGNQRhg6aZsGhhSaDF


Generated by Claude Code

Dropping a value whose enum variant holds an aggregate of mutable
references (e.g. `Pair::Two((&mut i32, &mut i32))`, or the
`Option<(&mut T, &mut [T])>` returned by `split_first_mut`) panicked in
`refine/env.rs` with `assert!(assumption_existentials.is_empty())`. The
enum-drop path built each field's drop assumption by re-deriving it
through a fresh `Path`, but projecting an aggregate field
(`PlaceType::tuple_proj`/`deref`) allocates fresh existentials, so the
recursive call returned an assumption with existentials the assert
never expected. Minimum repro:

    enum Pair<'a> {
        Two((&'a mut i32, &'a mut i32)),
        None,
    }

    #[thrust::callable]
    fn check(a: &mut i32, b: &mut i32) {
        let _p = Pair::Two((a, b));
    }

Rewrite `dropping_assumption` around a structural `drop_prophecy`
helper that walks the type once, threading tuple elements and box
targets as term *expressions* over the value (so a nested sub-value
stays shared across all the mutable references it contains) and
allocating existentials into a single shared namespace only for enum
fields (which need a selector). This resolves every packed prophecy to
identity correctly.

- Removes the now-dead `Path::PlaceTy` variant and
  `Path::{deref,tuple_proj}`.
- Regression tests: tests/ui/{pass,fail}/enum_tuple_mut_drop.rs,
  verified sat/unsat respectively under the default Spacer solver.

(Extracted as a standalone bugfix from d44c6b8.)

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QmZGoGNQRhg6aZsGhhSaDF

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: bf84002f41

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/refine/env.rs
self.drop_prophecy(existentials, body, inner, term.box_current());
} else if let Some(tty) = ty.as_tuple() {
for (i, elem) in tty.elems.iter().enumerate() {
self.drop_prophecy(existentials, body, &elem.ty, term.clone().tuple_proj(i));

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve nested refinements during drop

When the dropped value has a refined aggregate element (for example a generic enum or tuple element whose type argument is {v: &mut i64 | ...}), this recurses on only elem.ty and never substitutes term.tuple_proj(i) for the element's Value in elem.refinement. The previous path_type(...).tuple_proj() path went through subsume_rty, which added those nested refinements before emitting the prophecy-equality constraints. Dropping such an aggregate now produces a weaker CHC clause and can accept bad postconditions whenever the refined field is dropped without being projected elsewhere.

Useful? React with 👍 / 👎.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants