feat(ir): collection names declared in std are ordinary types #474
Loading…
Reference in a new issue
No description provided.
Delete branch "loop/468-collection-names"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Closes #468
What
ir/instantiate.trb: the six collection names are runtime primitives only whilestd/declares no type of that name; a declared generic type is built likefilledListbuilds it today.ir/elements.trb:isListSetteralso recognizes the wrapper shape of the record'sArrayList.set.Buffer.holdsClose: lowered constant per item type (mayHoldClose), K5 list + manifest row (docs/design/NATIVES.mdcounts: kernel 156, total 537).ArrayListoverBuffer(compiler/tests/lower-collections.test.trb), the manifest row (natives.test.trb), the conformance programtests/conformance/buffer-holds-close.trb.isListSetteris public, andcompiler/tests/elements.test.trbpins it on hand-built IR: the runtime'storb_list_set, the old wrapper of anative fnand thesetof a declared list (receiver = theitemsfield ofself, every other parameter in its own position) are setters; a body with an instruction before the call, a reordered index/value, a witness table, a missing parameter, a receiver that is no direct field ofself, or a callee that is another runtime function are not. Hand-built, because a source program can only write the shape the lowering gives it. What this does not assert: a source-levelvalues[i].f = von the declared list as an element step.elementTypeOfadmits only a container of the runtime's list type, so a record-typed container keeps its take-out and put-back until the follow-up commit (which movesstd/) teaches it; this commit teaches the setter shape only.tests/conformance/declared-list-shape.trbrunsPile, a program-localIterate/Length/MutableIndextype over aBufferin the shape of the declaredArrayList(withCapacity,appendin written order,forthroughiterate,a[i],a[i] = v,a[i].field = v, a copy is a value). The program cannot declare the collection name itself:addCollectionlooks the name up as the symbolstd/declares, andstd/still declaresArrayListnative, so a program-localArrayListis just another type and literals keep the runtime's list. The name resolution through the literal andforpaths stays pinned by the IR assertions oflower-collections.test.trb; the first executed run of the declaredArrayListitself is the follow-up commit that movesstd/. Output checked in the VM; the allocation budget (30) is an upper bound set without a C compiler on the authoring machine and is to be lowered to the count CI reports.Bufferblock ofdocs/standard-library/core.mdlistsholdsClose. Skills: the same line in the skills mirror (skills/torbscript-standard-library/references/standard-library/core.md), regenerated by hand to match the output oftorb docs skill.Baseline on the primitive
The table below was measured at head
fff3d176; nothing incompiler/src,std/orruntime/changed behaviour since, so it is the baseline for the follow-up.collection-cost.sh(tier B), fastest of 9, µs net of the floor, merge base861b495bvs headfff3d176. Every row still runs on the primitive (nothing instd/declares a list yet), so these are the baselines for the follow-up commit that movesArrayListoverBuffer.bytes-pass(fastest of 9):Buffer<UInt8>base 0.0421 s, head 0.0382 s;List<UInt8>base 0.0454 s, head 0.0471 s (gate: ratio 0.890, bound 1.10). Allocation counts are the same in base and head.Test
CI on this branch (forge workflow): bootstrap, tier A (compiler tests,
format --check,docs declarations --check, conformance gate), tier B (collection-cost,bytes-pass), commit messages, release-sync/registry, pdisc. Result: see the checks of the head commit.Seed impact
Yes (two-commit rule, seed refresh stays with the owner).
Buffer.holdsClose(step 2 of #55, commit 1 of 2) #46868243a6d80c704135566Buffer.holdsClose(step 2 of #55, commit 1 of 2) #468Buffer.holdsClose(step 2 of #55, commit 1 of 2) #468forpass over a flat list (step 2 of #58) #470Buffer.holdsClose(step 2 of #55, commit 1 of 2) #468forpass over a flat list (step 2 of #58) #470Buffer.holdsClose(step 2 of #55, commit 1 of 2) #468Buffer.holdsClose(step 2 of #55, commit 1 of 2) #468forpass over a flat list (step 2 of #58) #470forpass over a flat list (step 2 of #58) #470Buffer.holdsClose(step 2 of #55, commit 1 of 2) #468ada4eef04b8891a22727Buffer.holdsClose(step 2 of #55, commit 1 of 2) #468Buffer.holdsClose(step 2 of #55, commit 1 of 2) #468CI red on
8891a22:ci / tier A (linux-x64)failed after 26m40s (ci / gatesred as a consequence). Tier B, bootstrap, release-sync, pdisc and commit messages are green and main is green, so the failure comes from this head. The job log is not readable with the reviewer token, so I cannot name the failing step. Please fix or re-run; I will review once CI is green. No other blockers raised yet.Tier A failure on
8891a22was theformat --checkgate:compiler/tests/lower-collections.test.trb(one line, closure call layout). Fixed in9ec232fwithtorb format; no semantic change. Review re-requested.Review of
9ec232f. CI is green for this head (tier A/B, gates, pdisc, commit messages). Scope matches #468 and nothing instd/changes behaviour (only theholdsClosedeclaration is added). Checklist: Security/secrets ok, Data ok, Workflows ok, Docs ok (core.md, skills mirror, NATIVES.md counts, INDEX/sidecar regenerated), Architecture ok (design record cited), Correctness ofaddCollection,emitWithCapacityandlowerHoldsClosereads fine. Two acceptance gaps remain:[blocker]
isListSetterchange has no test.isStorageSetter/fieldForwardedCalleeincompiler/src/ir/elements.trbare new logic, and #468's second criterion is thata[i].f = vstays an element step through the wrapper shape of a declaredArrayList.set. Nothing in the diff exercises it: the two new lowering tests cover only the literal (withCapacity+append) andfor(iterate). Please add a lowering test (thedeclaredListFiles()harness already has thevar fn setwrapper) that writesvalues[i].field = vor similar on a declaredArrayListof records and asserts the element step is kept (no copy of the item out and back), plus one negative case where the wrapper is not a plain field forward (extra instruction, reordered argument, or a witness) andisListSetterreturns false.[blocker] The conformance program for the declared type is missing. #468 asks for "one for a test-local declared
ArrayList-shaped type resolved through the literal andforpaths" among the conformance programs; the PR addsbuffer-holds-closeandslice-close-genericbut the declared-type case exists only as IR-text assertions incompiler/tests/lower-collections.test.trb, which never run the generated code (order ofappends, capacity, aforresult). Either add the program (a program-local type that is the declared list, run through the conformance gate), or state in the PR body why a conformance program cannot declare a collection name whilestd/still has thenativeone, and give an executed test (compile and run, with.expected) as the substitute. The same executed test can cover finding 1 if it writes throughvalues[i].field = vand prints the result.Non-blocking:
[suggestion]The PR body table is for headfff3d176, not the current head; harmless as it is a baseline on the primitive, but say so if the numbers are quoted in the follow-up.9ec232f14744a359160dBoth blockers addressed in
44a3591(rebased on current main; two commits, no change undercompiler/srcbeyondisListSetterbecomingpublic).isListSettertest:compiler/tests/elements.test.trb, group "a callee that is a list setter", on hand-built IR. Positive:torb_list_setitself, the oldnative fnwrapper, and the declared list'sset(receiver =itemsfield ofself, other parameters in place). Negative: an instruction before the call, reordered index/value, a witness table, a missing parameter, a receiver that is a field of a field or of another slot, and a callee that is another runtime function. Not asserted: a source-levelvalues[i].f = vas an element step on the declared list.elementTypeOfonly admits a container of the runtime's list type, so a record-typed container keeps its take-out/put-back until the follow-up commit teaches it; this commit teaches the setter shape only (stated in the PR body).tests/conformance/declared-list-shape.trb, with.expectedand.allocations. A program cannot declare the collection name (addCollectionlooks up the symbolstd/declares, and that is stillnative), so it runsPile, a program-local type overBufferin the shape of the declaredArrayList(append order,forviaiterate,a[i],a[i] = v,a[i].field = v, copy as a value). Output checked in the VM. The allocation budget (30) is an upper bound: there is no C compiler on my machine to count it, so it should be lowered to the count CI reports. The declaredArrayListitself stays covered by the IR assertions oflower-collections.test.trbuntil the follow-up movesstd/.Suggestion taken: the PR body now says the baseline table is for
fff3d176.Re-review of
44a3591(earlier blockers on9ec232f+ changed lines). CI is green for this head (tier A/B, gates, pdisc, commit messages, cross-host C).Earlier findings:
isListSettertest - resolved.compiler/tests/elements.test.trbpins it on hand-built IR: positives (torb_list_set, the oldnative fnwrapper, a declared list'ssetforwarding theitemsfield ofself) and negatives (instruction before the call, reordered index/value, witness table, missing parameter, receiver a field of a field or of another slot, callee another runtime function). The stated limit (a source-levelvalues[i].f = von a declared list stays take-out/put-back until the follow-up teacheselementTypeOf) is explained and matches the issue goal of recognizing the setter shape; accepted on merit.tests/conformance/declared-list-shape.trbruns a program-localPileoverBuffer(append order,forviaiterate,a[i],a[i] = v,a[i].field = v, copy as value) with.expected/.allocations; the reason the declared name itself cannot be exercised beforestd/moves is given in the body and the file header, and the conformance gate passed.Checklist: Tests ok, Correctness ok, Scope = #468 (nothing in
std/changes behaviour), Security/secrets ok, Data ok, Docs ok (core.md, skills mirror, NATIVES.md, INDEX/sidecar), Architecture ok (design record cited), Release: seed impact stated, refresh stays with the owner.Non-blocking:
declared-list-shape.allocationsbudget (30) is an upper bound set without a C compiler; lower it to the count CI reports in the follow-up commit.ArrayListis a type overBuffer(step 3 of #55, commit 2 of 2) #526ArrayListis a type overBuffer(step 3 of #55, commit 2 of 2) #526ArrayList, the hash tables and the tries onBuffer#55ArrayList, the hash tables and the tries onBuffer#55