Skip to content

Resolve a type parameter's default where the binding is known - #1234

Merged
Frotty merged 29 commits into
masterfrom
interpreter/type-parameter-defaults
Aug 16, 2026
Merged

Resolve a type parameter's default where the binding is known#1234
Frotty merged 29 commits into
masterfrom
interpreter/type-parameter-defaults

Conversation

@Frotty

@Frotty Frotty commented Aug 15, 2026

Copy link
Copy Markdown
Member

Stacked on #1233.

Reading a slot of a T array that was never written gave a stand-in rather than the default of what T stands for, and the stand-in compares equal only to another stand-in. So Box<int>.first() == 0 was quietly false on the interpreter, while both backends had it right. Wrong answer, no error.

class Box<T:>
    private static T array none
    static function first() returns T
        return none[0]
init
    if Box<int>.first() == 0
        testSuccess()

The stand-in is produced by a static attribute, which cannot see the stack frames that know the type argument. ProgramState can, so the value is resolved where it is produced — at the array read and the member read — rather than at the comparison, which is only where the symptom shows.

Comparing an unresolved one is now an error naming the type parameter, instead of answering "not equal". The whole suite passes with it throwing, which says nothing reachable produces one any more; if something starts to, it says so rather than returning a wrong answer.

Also carries a note recording why the junk dispatch slot per specialised class stays for now: both available name sources are wrong in opposite directions, and the slot is unused, so the cost of leaving it is one table key per specialised class while the cost of getting the alias wrong is a real mis-binding.

Found while building the FastHashMap tombstone fixture, which needed a "no value" for V.

Full test suite green.

Frotty added 14 commits August 15, 2026 03:39
Exercises type class bounds through the container they were added for.
Six of the seven cases pass on both backends with no compiler change:
int keys, tuple keys on Jass, a user class key, two specialisations
coexisting, and two instances of one specialisation.

tupleKeyLua fails and is a real, pre-existing backend bug. Method names
become Lua table keys, but luaMethod.initFor passes the name through raw
while every sibling (luaVar, luaFunc, luaClassVar) sanitises via
uniqueName. Names are valid identifiers in ordinary code, so nothing hit
it until a class method was specialised for Lua with more than one type
argument: specializeMethod builds name + "_specialized_" +
generics.makeName(), and makeName joins arguments with ", ". Two simple
arguments give "get_specialized_integer, integer", which emits
"Class.get_specialized_integer, integer = impl" -- valid Lua that assigns
to two targets and quietly writes a junk global. A tuple argument gives
"⦅integer, integer⦆" and fails the syntax check outright.
Method names become Lua table keys, so they must be identifiers. A method
specialised with two type arguments was named after them, commas included,
and emitted `Class.get_specialized_integer, integer = impl` - valid Lua that
quietly assigns to two targets; a tuple argument produced characters luac
rejects outright.

normalizeMethodNames is the pass that gives one name to a whole dispatch
group, so it sanitises before uniquing: two names that differed only in
characters Lua has no place for still get a slot each. The backend maps every
slot key and every LuaMethod name through the same function, so call sites and
class tables keep agreeing. Lua's identifier rule now has one home.

The luac check never caught this, because the broken output parses. Assert
instead on the names themselves: every emitted function, method, variable,
field and call-by-name must be an identifier, checked for every testLua
compile.
The execution path read the spawned process's stderr to EOF before touching
stdout, and never bounded the wait. A program that fills the stdout pipe blocks
writing while the harness blocks reading stderr, and the suite stops with no
output, no timeout and no failing test - a stray worker JVM was still sitting on
the build directory twenty minutes later.

checkLuaSyntax, in the same file, already drained both pipes on their own
threads and waited with a timeout. The execution path now uses the same helper,
so a program that does not terminate fails its test instead of the run.

Also records what building a repro turned up: a bounded generic class cannot be
subclassed on either backend, and the div/mod result type asymmetry is reachable
through integer literals rather than harmless.
A method claims the slots of every method it might be an override of, and
sharesSemanticName accepted a match on either the declared name or the segment
after the last underscore of the mangled one. For a specialised method that
segment is a fragment of the type argument: get and slotFor of
FastHashMap<int, int> both read as 'integer', and since specialisation gave them
the same signature too, get claimed slotFor's slot and the class table bound
FastHashMap_slotFor_specialized_integer__integer to FastHashMap_get_specialized.

Declared names now settle it whenever both methods have one. The segment stays
as the fallback for closures and bridges, which have no declaration to ask -
that is the case it was there for.

The emitted Lua is the only record of which implementation a slot should hold,
so the test asserts it there: every slot named after a method must bind an
implementation named after the same method.
remove leaves a tombstone rather than an empty slot, because a probe that
stopped at one would miss keys put down beyond it. slotFor passes tombstones
over when searching and returns the first one when putting, so removing and
re-putting a key reuses its slot instead of lengthening the run. The probe is
bounded by capacity, so a table full of tombstones cannot spin.

The cost claim is asserted on the least optimised configuration, because it has
to hold by construction rather than because the inliner removed it: storage is
four plain Jass arrays, no hashtable native is reached for, slotFor takes
nothing beyond the receiver and the key, and both requirements are direct calls
rather than ExecuteFunc or a dispatch wrapper. With optimisation on, hash(key)
becomes key and equals(a, key) becomes a != key.

The dispatch_ functions in the output are the nullpointer check every class
method gets, not type class dispatch, so the test looks at the real function
underneath rather than asserting they are absent.

Building the fixture also turned up a silent wrong result in the interpreter,
recorded as item 16: a never-written array of a type parameter compares unequal
to the type argument's default.
Lua specialises what it can reach from a concrete type, and it finds that type
on calls that carry type arguments. A closure has no such call: it is reached
through the interface it implements, which is not generic, so nothing at the
call site says what the body dispatches on. Only the construction knows.

Three pieces were missing, each of which already exists for the Jass path: the
instantiation is now collected from the allocation, the member access is
collected so the write that captures the environment lands on the specialised
field rather than being dropped as a dead store, and the specialised methods are
bound to the roots their originals were submethods of. Registering the original
implementation as specialised lets the dispatch left in it settle rather than
reaching a backend with no way to express it.

All three are gated on the class being closure-generated, which is the case
where no call names the instantiation. Ungated, the object comes from the
specialised class while its methods were bound to the erased one, and every
FastHashMap Lua test fails - the disagreement AGENTS.md section 9 describes.

dispatchInsideClosureIsRejectedForLua pinned the old diagnostic and becomes
dispatchInsideClosureLua. The language doc and changelog said Jass only.
A bound dispatched from a constructor works on Jass and is rejected on Lua.
dispatchInsideConstructor covers the working target, and
dispatchInsideConstructorIsRejectedForLua pins that the rejection is a clear
diagnostic rather than a mistranslation.

Not the same gap as the closure case, and the fix from it does not reach. A
constructor belongs to its class rather than to a generic function of its own,
so the call that runs it carries no type arguments at all: the intermediate
language has b = new_Box(21) with b typed Box<integer{show}> and new_Box still
generic. The calls inside it do carry the class's type variable, but nothing
gives the outermost one a concrete argument, and collection requires non-empty
type arguments to start. The backlog records where the instantiation could come
from instead.
translateAndTestLua never set the environment label, so a Lua failure was
reported under whatever Jass configuration ran last: "With Inlining,
Optimizations and Stacktraces: Succeed function not called" for a program that
had already passed every Jass configuration. This run went looking for a Jass
regression that did not exist.

Also pins the div/mod result type from both sides rather than changing it.
Returning the left operand's type means an integer literal stays assignable to a
real through div and mod, where addition collapses two literals to int and does
not. That asymmetry is reachable, but OptimizerTests.realFormatting_consistent_
fromIntOps opens with `real a = 1 div 2`, so the suite already defines the
current behaviour as correct and tightening it would reject existing code. The
backlog carries the question; the tests make sure the answer is deliberate.
Reading a slot of a T array that was never written gave a stand-in rather than
the default of what T stands for, and the stand-in compares equal only to
another stand-in - so Box<int>.first() == 0 was quietly false on the
interpreter, while both backends had it right.

The stand-in is produced by a static attribute, which cannot see the stack
frames that know the type argument. ProgramState can, so the value is resolved
where it is produced - at the array read and the member read - rather than at
the comparison, which is only where the symptom shows.
The stand-in for the default of a type parameter answered "equal" only for
another stand-in, so a comparison against a concrete value was quietly false.
The previous commit resolves it where a value is read, which is the path that
reached a program, but the stand-in is produced by a static attribute and could
surface anywhere - and silence was the part worth removing.

It now throws when compared to a real value, naming the type parameter. The
whole suite passes with it, so nothing reachable produces one any more; if
something starts to, it says so rather than returning a wrong answer.
Tried building the class-qualified dispatch alias from the declared name
instead of the trailing segment of the mangled one, to stop every method of a
specialised class claiming the same slot. The suite rejected it: overloads share
a declared name, so setup(int) and setup(string) collapse into one slot, which
is what two LuaTranslationTests cases exist to prevent.

Both sources are wrong in opposite directions - the mangled segment collides
across the siblings of one specialisation, the declared name across overloads.
The junk slot is unused, so leaving it costs one table key per specialised
class, while getting the alias wrong costs a real mis-binding. The backlog says
what a fix would have to separate.

@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: 9d6b00579a

ℹ️ 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 on lines +31 to +32
public boolean isEqualTo(ILconst other) {
return other instanceof ILconstUnsafeDefault;
if (other instanceof ILconstUnsafeDefault) {

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 Reject unresolved defaults regardless of operand order

WurstOperator.EQ and NOTEQ evaluate left.equals(right), so this override runs only when the unresolved default is the left operand. Concrete constants such as ILconstInt and ILconstNull still return false when the unresolved value is on the right, making unsafeDefault == 0 throw while 0 == unsafeDefault silently evaluates false. Detect the placeholder symmetrically in the operator/equality path so the new diagnostic cannot depend on expression order.

AGENTS.md reference: AGENTS.md:L82-L85

Useful? React with 👍 / 👎.

Frotty added 4 commits August 16, 2026 08:37
Wurst and Lua reserve different words, so a method can be declared repeat or
goto and reach the backend under that name. Method names survive it, because
the pass that assigns them uniques against the reserved set. A closure does not:
it adds the name it implements as a dispatch alias directly, so the alias
arrives as a bare keyword and is emitted as a table key.

luac rejects that, so it was loud rather than wrong, but the check added
alongside it accepted the name - and catching this before the syntax check is
the whole point of having it. isValid now rejects keywords and toIdentifier maps
them out of the way.

Underscores rather than a counter, so a keyword maps to the same name wherever
it is derived: call sites and class tables have to agree without consulting each
other.
Two tests for the runner itself, both Lua only: run through the Jass
configurations the non-terminating one would hang the interpreter instead, which
is the same problem somewhere this fix does not reach.

  - a program that loops forever fails its own test rather than the run
  - a program that prints twenty thousand lines still finishes

The second is the deadlock this change was for: with the streams read one after
the other, the program blocks writing stdout while the runner blocks reading
stderr. The timeout is overridable so these take seconds rather than a minute.

Output kept for the failure message is capped. Draining still never stops, since
stopping is what blocks the process, but a program that loops while printing
would otherwise exhaust the worker before the timeout fires.

Capping meant success could no longer be read back out of the retained text -
testSuccess prints last, well past the limit - so the line is recognised while
draining. Which is better regardless: whether a program succeeded no longer
depends on how much of its output was kept.
slotFor says a key has nowhere to go by returning a slot below the instance's
section, and get, has and remove all check for it. put did not, so a ninth
distinct key wrote through the sentinel: an entry nothing can reach, a count of
nine, and a negative array index on targets less forgiving than Lua.

The test fills the map exactly - keys one to eight hash to the eight slots - and
puts a ninth. It does not read the absent key back, because reading one of a
type parameter returns a stand-in the interpreter cannot compare; that is a
separate bug, fixed later in this stack.
Frotty added 4 commits August 16, 2026 08:48
The gate asked whether a dispatch sits in the closure itself. A closure whose
body is () -> helper(x) has none - the dispatch is in helper - so the
construction was skipped, and the bound reached the backend unresolved even
though this is exactly the shape the change was for.

It now uses the call-aware check that already exists for functions.
classNeedsSpecialization is left as it was, because widening that would change
what gets specialised on paths which have nothing to do with closures.

Specialising the closure class then surfaced a second thing: the member access
collector ran again for the copy, and a receiver still typed by the generic
class cannot adapt to the specialised one, which is not a superclass of it. A
class with no type variables left has nothing to select, so it is skipped.
…parameter-defaults

# Conflicts:
#	de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/TypeClassTests.java
Equality asks the left operand whether it equals the right, so putting the
refusal in the stand-in only covered half of it: unresolved == 0 complained
while 0 == unresolved went quietly false, which is the failure the refusal was
added to remove.

The check moves to the operator, where both operands are in hand, and the
stand-in goes back to answering plainly. Two stand-ins still compare equal to
each other; only a comparison against a real value is refused.
@Frotty

Frotty commented Aug 16, 2026

Copy link
Copy Markdown
Member Author

Fixed. You were right that it was half a check: EQ and NOTEQ ask the left operand, so unresolved == 0 complained while 0 == unresolved went quietly false — which is exactly the failure mode the refusal was added to remove.

Moved it into WurstOperator, where both operands are in hand, and ILconstUnsafeDefault.isEqualTo goes back to answering plainly. Two stand-ins still compare equal to each other; only a comparison against a real value is refused. unwrittenArrayOfATypeParameterReadsAsItsDefaultReversed covers the reversed order.

Also merged the updated #1233 through, keeping both tests that landed at the same spot in TypeClassTests.

TypeClassTests, InterpreterTests and CompiletimeTests pass; the full suite is running and I will report here if it disagrees, since this sits on the path every interpreter comparison takes.

@codex review

Frotty added 3 commits August 16, 2026 09:05
Neither of these belongs in a branch that gets merged. The loop configuration
is machine-local state - an active session id, an iteration count, a path and a
branch name from one checkout - and another checkout picking it up would inherit
an automation session that has nothing to do with it. The brief beside it is
one run's instructions, not documentation of anything.

Both came in with the branch this work was based on rather than with the work.
Ignored as well, so they do not come back the next time either is written.
@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Keep it up!

Reviewed commit: 7582ade618

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

Frotty added 3 commits August 16, 2026 09:06
…dispatch

# Conflicts:
#	de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/TypeClassTests.java
…parameter-defaults

# Conflicts:
#	de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/TypeClassTests.java
Base automatically changed from lua/closure-type-class-dispatch to master August 16, 2026 08:06
…rameter-defaults

# Conflicts:
#	BACKLOG.md
#	de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/EliminateGenerics.java
#	de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LuaRunnerTests.java
#	de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/TypeClassTests.java
#	de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/WurstScriptTest.java
@Frotty

Frotty commented Aug 16, 2026

Copy link
Copy Markdown
Member Author

Updated from master (#1230 through #1233 are in). No new findings since the last round, and the operand-order fix is unchanged by the merge: the refusal lives in WurstOperator where both operands are in hand, and ILconstUnsafeDefault.isEqualTo answers plainly.

Merge resolution, since several files needed a side picked: took master's EliminateGenerics, LuaRunnerTests and WurstScriptTest (they carry the merged #1231 and #1233 work), kept ours for BACKLOG.md, and rebuilt TypeClassTests as master's version plus this PR's two unwrittenArrayOfATypeParameter tests so nothing from either side was dropped.

TypeClassTests, InterpreterTests and CompiletimeTests pass.

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Chef's kiss.

Reviewed commit: 2290bc1749

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

@Frotty
Frotty merged commit 5ccc791 into master Aug 16, 2026
4 checks 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.

1 participant