feat(mir): admit heap-owning tuple fields in functional-record-update carry - #2916
feat(mir): admit heap-owning tuple fields in functional-record-update carry#2916gertybotbot wants to merge 7 commits into
Conversation
… carry Lifts the fail-closed NYI on carrying a heap-owning tuple field through a functional-record update. Widens the carry pre-flight's `sound_carry` predicate to admit `ty_is_heap_owning_tuple`, reusing the existing `derive_tuple_composite_drop_allowed` composite-drop prover that already sits beside the record prover. No new recursive carry spine is authored: the prover excludes the consumed base, so the base's scope-exit drop and the result's drop cannot both release the same allocation. Refs hew-lang#2207
Rebased onto upstream main. Two CI defects, both ours: - E0061 at expr.rs: upstream added a 4th param (&LifecycleRegistry) to ty_is_heap_owning_tuple after this branch was cut; supply self.lifecycle_registry. - cargo fmt --all --check failed on an over-long assert! in the new test. Verified locally: cargo check -p hew-mir rc=0, cargo fmt --all -- --check rc=0.
20f7127 to
6c14269
Compare
|
CI update at head Fixed — Remaining failure, and I do not believe it is mine: Why I read it as a timing flake:
I do not have re-run rights ( |
slepp
left a comment
There was a problem hiding this comment.
The lowering change itself held up under everything I threw at it — Guard-Malloc with scribble/guard-edges, a 100k-iteration reassign-loop carry, and 50k/100k/200k frame sweeps with zero RSS slope. Two things need addressing before this can land:
-
Admitted scope is wider than the stated scope.
ty_is_heap_owning_tupleis a pure shape predicate (Tuple(_)+ any drop obligation), so the carry now also admits a tuple containing anOptionpayload (record O { maybe: (Option<Inner>, i64), tag: string }) and a tuple containing a heap-payload user enum — both passhew checktoday, while the bare-field forms are still rejected. The description says Option/enum are untouched, and there's no fixture or soundness argument for either newly admitted family. My probes for both ran clean under the malloc oracles, so I don't have evidence of unsoundness — but they need fixtures and a stated argument, or the predicate needs narrowing to what the PR argues for. -
The soundness comment names a prover that doesn't run on this path.
derive_tuple_composite_drop_allowed's candidate set is tuple bindings; in these fixtures the carried tuple is a record field, never a binding, so that prover has no candidate. What actually protects the base isderive_owned_record_drop_allowed's escape rule — the tuple-typedRecordFieldLoaddest counts as heap-owning and its escape into the result'sRecordInitdrops the base root fromRecordInPlace. The mechanism is real; the committed explanation attributes it to the wrong prover. Please repoint the comment at the escape rule.
One smaller note: MALLOC_CHECK_/MALLOC_PERTURB_ are glibc-only, so it's worth stating the platform in the test plan — the suite's own module doc names Guard-Malloc + MallocScribble as the authoritative oracle, and that's what I verified with.
The (i64, i64) conservative-reject follow-up you propose sounds right; happy to see that as a separate PR.
|
Addressed both requested changes at
Local verification at that head: |
|
The current head is green, and the requested scope and soundness corrections are present in |
Closes the fail-closed NYI in #2207 for heap-owning tuple fields.
What changed
sound_carryin the functional-record-update carry pre-flight now admitsty_is_heap_owning_tuple. No new recursive carry spine is authored.The protection comes from
derive_owned_record_drop_allowed, not the tuple-bindingprover: the tuple-typed
RecordFieldLoaddestination is recognised as a heap-owningfield binder, and its escape into the result's
RecordInitexcludes the consumed baseroot from
RecordInPlace. The result therefore receives the tuple's complete nesteddrop obligation without the base releasing it a second time.
The functional change is 4 lines in
hew-mir/src/lower/expr.rs; the rest is tests andthe soundness comment at the carry gate.
Verification
Suites at head
42ca548ee:cargo test -p hew-cli --test funcupdate_consume_semantics— 49 passed / 0 failedcargo test -p hew-mir— 565 passed / 0 failedcargo fmt --all -- --check— rc=0Six widened shapes accept and run clean, including the full scope implied by the tuple
predicate:
(string, (string, i64))((string, i64), (string, i64))Vec(Vec<string>, i64)(Inner, i64)Optionpayload(Option<Inner>, i64)(Payload, i64)The Linux allocator checks used glibc's
MALLOC_CHECK_=3 MALLOC_PERTURB_=165; Guard Malloc +MallocScribbleis theauthoritative Darwin oracle and was independently verified in review.
No per-frame leak on the collection-bearing shape. The sweep stayed flat across
50k/100k/200k frames: 56800 / 56928 / 56976 KB maxRSS.
Bounded on both sides
Bare
Option<Inner>and bare heap-payload enum fields remain fail-closed. AnOptionor enum nested inside the admitted tuple transfer boundary is covered by explicit
fixtures. Closure and handle families also remain fail-closed.
A tuple with no heap fields (
(i64, i64)) still rejects with the NYI. That is acoverage gap, not a soundness one (a plain-scalar tuple is trivially bit-copyable), and
it is pinned by a named test. Lifting it is a separate follow-up.
Scope
This PR lifts heap-owning tuple fields, including their recursively owned payloads.
Direct/bare
Optionand enum fields, handles, and closures remain untouched.Refs #2207