Skip to content

test: strengthen ownership and partial-match regressions - #1427

Merged
Lorenzobattistela merged 2 commits into
bendlang:mainfrom
vicmcorrea:test/retain-partial-match-lowering
Oct 9, 2026
Merged

Lorenzobattistela merged 2 commits into
bendlang:mainfrom
vicmcorrea:test/retain-partial-match-lowering

Conversation

@vicmcorrea

@vicmcorrea vicmcorrea commented Oct 8, 2026 •

Copy link
Copy Markdown
Contributor

Four regressions in #1314 could pass while ownership leaked or their target code was pruned. This extends the PR to all four fixtures and keeps their explanatory headers:

  • match_absurd_tail passes its partial-match function to opaque C/JS sinks without constructing Empty or invoking it, retaining both emitters' missing-arm lowering.
  • closure_slab_drop allocates an array from runtime input, captures it in a closure, and sends that closure to a sink that verifies and drops the actual capture.
  • closure_value_owns builds fresh trees and applies pick through the dynamic closure entry, checking both sums and native arena growth.
  • list_rebox_sum repeatedly builds, reboxes and consumes both Scalar and Pair lists, checking values and native arena growth.

The native checks warm the allocator's free lists, then allow at most one additional allocation quantum of reserved arena pages. They measure neither live bytes nor RSS. JavaScript checks values only. The existing foreign twins and test gate need no changes.

Regression sensitivity was checked against targeted broken behavior:

  • Reversing the current sum-arm spare isolation corresponding to de63a6f leaves the original 7/3 test passing; the new reboxing test fails its arena guard.
  • Restoring borrowed argument ownership at a def-value eta closure entry leaves the original 41000000 test passing; the new closure test fails its arena guard.
  • Skipping only the array capture during closure disposal fails the slab guard. Refusing C or JS Efq lowering leaves the original partial-match fixture emitting, while the retained fixture fails emission.

I could not resolve historical commit b3f448e in this repository. The closure control is a reviewed current-equivalent borrowing reversal, not a literal historical revert.

Validation passed for source execution, emitted JS, native default/one/four threads, namespace imports, combined --checkup, native UBSan, and the final fixtures against current main (3f80b06). Two independent reviews found no actionable issues. Repository gate: 54/54 with the CI-pinned ttok 0.3. Cluster/GPU checks could not run because the configured cluster hostname does not resolve.

Addresses #1314.

@Lorenzobattistela Lorenzobattistela self-assigned this Oct 9, 2026
@Lorenzobattistela

Copy link
Copy Markdown
Collaborator

Thanks @vicmcorrea. The match_absurd_tail change works. On main, absurd.arm was pruned and never reached the emitters. With this PR, absurd.arm now shows up in both the C and JS output, and the test prints 1 and ok on the interpreter, JS and C.

But #1314 lists four weak tests, and this PR fixes only one of them. The other three still don't check what they claim to:

  • closure_slab_drop: g is never used, so it gets pruned and the test never reaches the code it's meant to cover.
  • closure_value_owns: the header promises a memory check, but the test only checks the printed value. The leak it was written for could come back without a failure.
  • list_rebox_sum: same problem. It only checks the printed 7 and 3, so the leak can't be seen.

Could you extend this PR to cover all of #1314, so the issue can be closed in one go? For the two memory tests, revert the fix each one guards (b3f448e and de63a6f) and check that the test fails.

Also, this PR deletes the header of match_absurd_tail.bend. Please keep it, updated to explain why keep is there.

@vicmcorrea vicmcorrea changed the title test(compiler): retain partial-match lowering coverage test: strengthen ownership and partial-match regressions Oct 9, 2026
@Lorenzobattistela
Lorenzobattistela merged commit 5e70819 into bendlang:main Oct 9, 2026
1 check passed
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