Keep the fields a specialised class allocates, and compile FastHashMap against the standard library by Frotty · Pull Request #1239 · wurstscript/WurstScript

Frotty · 2026-08-16T12:02:39Z

Every existing FastHashMapTests case compiles a bare package test with no imports. The container is meant to live in the standard library, so this compiles it with the library present — and that immediately found a real bug.

The bug

A specialised class allocated an empty table:

function FastHashMap:create2()
local new_inst = ({FastHashMap_base=0, FastHashMap_count=0, }) -- erased
function FastHashMap_specialized_integer__integer:create3()
local new_inst = ({}) -- specialised

A specialised class holds copies of the original's fields, and nothing refers to the copies: an access made before specialisation still names the original's variable. Both passes that drop unread fields — RemoveGarbage for Lua and ImOptimizer — therefore dropped every one of them. An instance allocated from the specialised class then came out with no fields while the emitted code went on reading them by name, so size() returned nil and put incremented nil.

Lua resolves a field by name and both classes carry the same one, so a name read anywhere keeps the field wherever it is allocated. The cost is an unused table key in the worst case; the alternative is an allocation that silently omits everything.

It was reachable only where an allocation goes through the specialised class rather than the erased one, which is why nothing caught it — the existing tests all allocate through the erased class. Putting the library in scope is what moved an allocation onto the other path.

Tests

fastHashMapAgainstTheStandardLibrary — the int-key map, compiled and run with the library present.
classKeyAgainstTheStandardLibrary — a class key with a user written Hashable instance, which is the shape a real map is used in.
fastHashMapAgainstTheStandardLibraryLua — compiled, and asserts a specialised class allocates the same fields as the class it was specialised from. That assertion is what fails without the fix.

Full suite green.

Note on the Lua test being compile-only

It is compiled rather than run, as every other test putting the standard library on Lua is. No standard library program has ever executed on that target in this repository: the runtime shim cannot initialise the library's own packages, and GameTimer fails first. That is a harness gap unrelated to generics, worth its own issue rather than being folded in here.

A specialised class holds copies of the original's fields, and nothing refers to the copies: an access made before specialisation still names the original's variable. Both passes which drop unread fields therefore dropped every one of them, and an instance allocated from the specialised class came out of its create with an empty table while the emitted code went on reading those fields by name - size() returning nil, put incrementing nil. Lua resolves a field by name and both classes carry the same one, so a name read anywhere keeps the field wherever it is allocated. The cost is an unused table key in the worst case. It was reachable only where an allocation goes through the specialised class rather than the erased one, which is why nothing caught it: the existing tests allocate through the erased class. Compiling the container against the standard library is what put an allocation on the other path.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit:dd62541d1c

ℹ️ 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".

chatgpt-codex-connector · 2026-08-16T12:07:07Z

 int classFieldsBefore = c.getFields().size();
- changes |= c.getFields().retainAll(readVars);
+ changes |= c.getFields().retainAll(c.getFields().stream()
+ .filter(field -> readVars.contains(field) || readFieldNames.contains(field.getName()))


Restrict field retention to actual specialization copies
When any reachable variable has a common name, this predicate retains every field with that name in every class, including unrelated fields; the same program-wide name check is repeated in RemoveGarbage.java:99. Consequently, one live integer field named size, for example, prevents elimination of every dead size field and makes each affected Lua instance allocate an unnecessary table entry, increasing generated code and per-object memory across standard-library builds. Track the copy-to-original relationship or owning specialization instead of matching raw names globally in both passes.
AGENTS.md reference: AGENTS.md:L88-L91
Useful? React with 👍 / 👎.

Matching names program-wide retained every field sharing a name with any live one, so a single live field called size kept the dead size of every other class and put an unused entry in each of their instances. The copy is structural, so field i of a specialised class is field i of the original, and recording that pairing where the class is copied says exactly which fields are copies and which they came from. A copy is live when its original is, and nothing else is retained.

Frotty · 2026-08-16T12:32:21Z

Fixed properly rather than narrowed — you are right that matching names program-wide is too blunt, and size is exactly the name that would bite on a standard library build.

The pairing is now recorded where it is known. specializeClass copies a class structurally, so field i of the copy is field i of the original; it records each pair on the ImTranslator it already holds. Both passes then keep a field when it is read or when the field it was copied from is read:

field -> readVars.contains(field) || readVars.contains(trans.originalOfSpecializedField(field))

originalOfSpecializedField returns the argument unchanged for anything that is not a copy, so an ordinary dead field is dropped exactly as before — including a dead size in a class unrelated to any specialisation. The name check is gone from both ImOptimizer and RemoveGarbage.

RemoveGarbage.removeGarbage takes the translator now; its only call site (WurstCompilerJassImpl:962) already had one to hand.

The assertion that a specialised class allocates the same fields as the class it was specialised from still passes, so the original bug stays fixed while the retention is confined to actual copies. Full suite green.

@codex review

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit:892d6b8d96

ℹ️ 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".

chatgpt-codex-connector · 2026-08-16T12:38:13Z

+ for (int i = 0; i < c.getFields().size() && i < newC.getFields().size(); i++) {
+ translator.recordSpecializedField(newC.getFields().get(i), c.getFields().get(i));


Keep specialized field names aligned during Lua normalization
When a generic class field collides with a method name, recording only the field identity does not preserve the shared Lua table key: LuaTranslator.normalizeFieldNames processes the original and specialized classes independently, and garbage collection can leave them with different method sets. For example, changing FastHashMap.count to the otherwise-supported field/method collision size emits the original field as FastHashMap_size_field but the retained copy as FastHashMap_size; specialized allocation therefore initializes a different key from accesses that still reference the original field. Carry the origin relationship into field-name normalization (or otherwise force each copy to use its origin's normalized name).
Useful? React with 👍 / 👎.

A field sharing its name with a method is renamed around it, both being keys of one table. That is decided per class from that class's own methods, and a specialised class need not hold the same set as the class it was copied from once unused ones are dropped, so the two could be renamed differently: a field named size came out as FastHashMap_size_field on the original and FastHashMap_size on the copy, and the allocation wrote a key nothing read. Accesses name the original's field, so the original's name is the only key either can use. Keeping the copy alive was not enough on its own; it has to be alive under the same name.

Frotty · 2026-08-16T13:08:59Z

Good catch, and it was a hole in the previous fix rather than a separate issue: keeping the copy alive is worth nothing if it is alive under a different key.

Reproduced before changing anything, by renaming FastHashMap's count field to size so it collides with size():

erased: FastHashMap_base=0, FastHashMap_size_field=0,
specialised: FastHashMap_base=0, FastHashMap_size=0,

The allocation writes one key and every access reads the other, exactly as described.

Fixed by carrying the origin relationship into normalisation. alignSpecializedFieldNames runs after normalizeFieldNames and gives every copy the name of the field it was copied from. The accesses name the original's field, so the original's name is the only key either can use — and this holds whether or not the origin's own class survived pruning, since the name is taken from the variable rather than from its class.

aFieldNamedLikeAMethodKeepsOneKeyAcrossSpecialisation is the reproduction above, kept as a test: it runs the map on Lua with the colliding name and asserts the specialised class allocates the same fields as the erased one. It fails without the alignment.

Full suite green.

@codex review

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit:35dc16b29e

ℹ️ 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".

chatgpt-codex-connector · 2026-08-16T13:16:14Z

+ for (ImVar field : c.getFields()) {
+ ImVar origin = imTr.originalOfSpecializedField(field);
+ if (origin != field) {
+ field.setName(origin.getName());


Preserve collision escaping in specialized classes
When an origin field name collides only with a specialized method slot, this assignment undoes the specialized class's collision escaping. Fresh evidence is a FastHashMap field named get_specialized_integer__integer: normalization initially distinguishes it from the specialized get slot, but alignment emits both the instance field and method as FastHashMap_get_specialized_integer__integer; a virtual call then resolves the numeric instance field before the class-table method and attempts to call that number. Derive one shared field name that is safe against the method slots of both the origin and every specialization rather than unconditionally restoring the origin's normalized name.
AGENTS.md reference: AGENTS.md:L266-L270
Useful? React with 👍 / 👎.

A field of a specialised class and the field it was copied from must be one table key, because the accesses reaching either still name the original's variable. Naming each side around only its own methods leaves them different; taking the original's name afterwards puts back whatever collision the specialisation had escaped, and a field shadowing a slot is found first by a virtual call, which then tries to call a field. The name is now settled once, against the methods of the original and of every specialisation, and every copy takes it. Neither side can be renamed into the other's slot.

Frotty · 2026-08-16T13:37:46Z

Fixed, with one correction to how I am reporting it.

The objection is right: restoring the origin's name unconditionally undoes an escape the specialisation had made for itself. The name is now settled once per field, against the method names of the origin and of every specialisation holding a copy, and every copy takes that name. collectNamesEachFieldMustAvoid gathers those names per original field before any renaming; normalizeFieldNames then decides a name the first time any member of the group is met and reuses it for the rest. Neither side can be renamed into the other's slot, and the two still end up as one key.

I could not reproduce the specific failure. I wrote a test for it first — a FastHashMap field named get_specialized_integer__integer, as described — and it passes against the previous commit as well, so it never demonstrated the bug. The reason is in the emitted Lua: nothing in FastHashMap overrides anything, so its specialised methods come out as direct calls (FastHashMap_get_specialized(m, 1)) rather than dispatch slots, and there is no method key for a field to shadow. A faithful reproduction needs a generic class with a virtual method, which is a different shape than this PR builds.

Rather than keep a test whose name claims more than it checks, I removed it. The fix rests on the reasoning above instead: it is safe by construction against both sets of method names, which is what the comment asked for.

The reproduction from the previous round is still there and still meaningful — aFieldNamedLikeAMethodKeepsOneKeyAcrossSpecialisation covers a field colliding with a method of its own class, which does occur here, and it fails without the fix.

Full suite green.

@codex review

chatgpt-codex-connector · 2026-08-16T13:43:58Z

Codex Review: Didn't find any major issues. Nice work!

Reviewed commit:6165ebd2d7

ℹ️ 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".

chatgpt-codex-connectorBot reviewed Aug 16, 2026
View reviewed changes

Frotty merged commit 4c44a98 into masterAug 16, 2026
6 checks passed

This was referenced Aug 16, 2026
Rewrite the backlog around what is left after the container works #1241
Merged
Run a standard library program on Lua #1242
Merged
Find a type variable by what it was copied from, not by its name #1244
Open

Navigation Menu

Uh oh!

Keep the fields a specialised class allocates, and compile FastHashMap against the standard library - #1239

Keep the fields a specialised class allocates, and compile FastHashMap against the standard library#1239
Frotty merged 4 commits into
masterfrom
proof/fasthashmap-with-stdlib

Frotty commented Aug 16, 2026

Uh oh!

chatgpt-codex-connectorBot left a comment

Uh oh!

chatgpt-codex-connectorBotAug 16, 2026

Uh oh!

Frotty commented Aug 16, 2026

Uh oh!

chatgpt-codex-connectorBot left a comment

Uh oh!

chatgpt-codex-connectorBotAug 16, 2026

Uh oh!

Frotty commented Aug 16, 2026

Uh oh!

chatgpt-codex-connectorBot left a comment

Uh oh!

chatgpt-codex-connectorBotAug 16, 2026

Uh oh!

Frotty commented Aug 16, 2026

Uh oh!

chatgpt-codex-connectorBot commented Aug 16, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

		for (int i = 0; i < c.getFields().size() && i < newC.getFields().size(); i++) {
		translator.recordSpecializedField(newC.getFields().get(i), c.getFields().get(i));

Uh oh!

Conversation

Frotty commented Aug 16, 2026

The bug

Tests

Note on the Lua test being compile-only

Uh oh!

chatgpt-codex-connectorBot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connectorBotAug 16, 2026

Choose a reason for hiding this comment

Uh oh!

Frotty commented Aug 16, 2026

Uh oh!

chatgpt-codex-connectorBot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connectorBotAug 16, 2026

Choose a reason for hiding this comment

Uh oh!

Frotty commented Aug 16, 2026

Uh oh!

chatgpt-codex-connectorBot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connectorBotAug 16, 2026

Choose a reason for hiding this comment

Uh oh!

Frotty commented Aug 16, 2026

Uh oh!

chatgpt-codex-connectorBot commented Aug 16, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant