From 36146c8e8d422da7d49e414740f67012e9b0b452 Mon Sep 17 00:00:00 2001 From: Frotty Date: Mon, 17 Aug 2026 14:50:10 +0200 Subject: [PATCH 1/2] Record the four decisions, and settle div and mod Item 8 is closed rather than pending: div and mod keep returning the left operand's type, so real r = 7 div 2 compiles and is meant to. The branch in AttrExprType says so now, instead of looking like an oversight beside caseMathOperation, which collapses two literals to int precisely so real r = 1 + 1 is an error. Both tests which depend on it still pass. Item 7 records that the instantiation declares the module's type parameters only so a dispatch receiver has a name to resolve, and that they stay out of type inference - generic modules keep resolving theirs by matching the receiver type. The alternative, letting inference see them, collides with that and fails genericModuleInGenericClassGet; this is the smaller change at the price of the parameter meaning something narrower than it looks. What is left of the attempt is written down, including the widening the requirement lookup still needs. Item 23 records that Lua specialises only the paths needing a concrete type and leaves the object erased, because generated scripts should stay small. Items 6 and 13 both end there, and it removes the two-shapes-at-once arrangement which produced both field bugs in #1239. Instances for a family of types stay deferred, with why: it decides whether bounds stay a tool for new containers or become how the existing ones work. --- BACKLOG.md | 111 ++++++++---------- .../wurstscript/attributes/AttrExprType.java | 8 ++ 2 files changed, 54 insertions(+), 65 deletions(-) diff --git a/BACKLOG.md b/BACKLOG.md index aa81417ac..6f87662ea 100644 --- a/BACKLOG.md +++ b/BACKLOG.md @@ -49,54 +49,43 @@ itself, and one gap in what the suite can see. Collecting from types on the Lua path runs straight into item 23, so settle that first. -23. **The Lua erasure model, which items 6 and 13 both end at.** On Lua a generic class is erased, - and specialised copies are made only where a construction names the instantiation. Every - remaining gap on that target is one question: an object allocated from a specialised class while - its methods are bound to the erased one breaks, and an object allocated from the erased class - cannot reach a specialised method. - - Two ways out, and it is a decision rather than a patch. Either specialise only the paths which - need a concrete type and leave the object erased throughout, or stop erasing constructed generic - classes on Lua and pay the code size. - - #1239 is a reason to take it seriously rather than leave it. Both class shapes existing at once - is what let an instance be allocated with no fields at all, and then with its fields under a key - nothing read. Both are fixed; the shape which produced them is still there. - - Do not start this autonomously. - -7. **Module bounds.** `module M` is rejected with a clear message today, and - `TypeClassTests.boundOnModuleTypeParameterIsRejected` pins that. Tried and reverted; what follows - is why, because the earlier note here suggested a fix which cannot work. - - Expansion copies the module body into the user and replaces the module's type parameters - **in type positions**. A requirement is called on the parameter itself — `T.show(x)` — and that - receiver is a name, resolved by `lookupBoundedTypeParam` through `lookupType`, so the replacement - never touches it. - - Renaming the receiver to the using class's parameter, which is what "receiver rewriting during - expansion" meant, does not work. `NameResolution.nextScope` sends a `ModuleInstanciation` to - `attrModuleOrigin()` rather than to the class using it: - - if (currentScope instanceof ModuleInstanciation) { - return nextScope(moduleInstanciation.attrModuleOrigin()); - } - - That is deliberate — a module body resolves in the module's own scope so it cannot capture the - names of whoever uses it — so the renamed receiver names something that scope cannot see. The - rename itself works: with it, the error moves from the rejection to `Could not find variable K` - at the dispatch, which is this scope rule and not a mistake in the rename. - - That leaves the other half of the original note: **type parameters on `ModuleInstanciation`**. The - instantiation declares the parameter itself, bound to the argument, so the copied body keeps - saying `T` and `T` resolves without any rename. It needs `ModuleInstanciation` to carry type - parameters in the grammar, so it is a change to `wurstscript.parseq` and everything reading that - node, not a patch to the expander. - - Worth knowing before starting: an argument which is a concrete type (`use Shower`) is a - second case even then. A requirement is dispatched on a type parameter, so `int.show(x)` is not a - dispatch at all — that one has to resolve to the instance during expansion rather than resolve by - name. +23. **The Lua erasure model.** Decided: **specialise only the paths which need a concrete type and + leave the object erased throughout.** Generated scripts stay small, which is the reason for the + choice; the cost is that it is more compiler work than the alternative of not erasing at all. + + What that means in practice. An object keeps coming from the erased class, so it must never need a + specialised method - the concrete type is threaded to the places which use it rather than to the + object. Items 6 and 13 both end here: a constructor's dispatch needs the type at the construction + site, and a subclass's `super` call needs it on the call rather than on the receiver's class. + + Take it seriously rather than working around it. Two class shapes existing at once is what let an + instance be allocated with no fields at all, and then with its fields under a key nothing read + (#1239). Both are fixed; the shape which produced them is what this decision removes. + + Unblocks items 6 and 13's Lua half. Not blocking the container, which works on both targets today. + +7. **Module bounds.** Decided: the instantiation declares the module's type parameters **only so a + dispatch receiver has a name to resolve**, and they are excluded from type inference, which keeps + resolving a generic module's parameters by matching the receiver type as it does today. + + Why that way. A requirement of a bound is called on the parameter itself - `T.show(x)` - and that + receiver is a name, which the type replacement during expansion never touches. Renaming it to the + using class's parameter cannot work: `NameResolution.nextScope` sends a `ModuleInstanciation` to + `attrModuleOrigin()` rather than to the class using it, deliberately, so a module body cannot see + the names of whoever uses it. The parameter therefore has to be declared where the body can see it. + + Why only for the receiver. Declaring it and letting inference see it collides with the existing + mechanism: `GenericsModuleTests.genericModuleInGenericClassGet` fails with "Cannot infer type for + type parameter T". Two mechanisms answering one question is the cost of the alternative; this is + the smaller change, at the price of the parameter meaning something narrower than it looks. + + Started on `feat/module-instanciation-type-params`, unpushed. The grammar carries `typeParameters` + and `typeArgs` (resolved, since an argument names something only the user's scope can see), + resolution binds a declared parameter to its argument through `WurstTypeBoundTypeParam`, and + `isTypeClassDispatch` accepts a receiver which denotes a parameter through a binding. The error + chain reached "Could not find function show", which is the requirement lookup not following a + binding to the underlying parameter's bounds - the same widening, wherever a bound's functions are + surfaced. Verify before continuing that inference can be told to ignore the declared parameters. 9. **Keep `WURST_LANGUAGE.md` and `CHANGELOG.md` current** as items land — a standing practice rather than a task to finish. `WURST_LANGUAGE.md` is tracked, at @@ -208,23 +197,11 @@ itself, and one gap in what the suite can see. ## Blocked on a decision -- **8. Should `div` and `mod` keep returning the left operand's type?** Tried returning - `WurstTypeInt.instance()` to match `caseMathOperation` and reverted it: it is a user-visible - breaking change, and the suite already defines the current behaviour as correct. - - The asymmetry is real and reachable. `WurstTypeIntLiteral` is a proper subtype of both int and - real, and `caseMathOperation` collapses two literals to int precisely so `real r = 1 + 1` is an - error. `div`/`mod` return `leftType`, so `real r = 7 div 2` compiles. Changing that made exactly - one test fail — `OptimizerTests.realFormatting_consistent_fromIntOps`, which opens with - `real a = 1 div 2` — and AGENTS.md says the existing suite is the authoritative definition of - behaviour. Real maps will contain the same shape. - - So the question is the owner's: is `real r = 7 div 2` meant to compile? If yes, the branch in - `AttrExprType` wants a comment saying so, and this item closes. If no, it is a deliberate - breaking change that needs the changelog, and `realFormatting_consistent_fromIntOps` needs - rewriting to say what it actually tests, which is real formatting rather than that assignment. - `ExpressionTests.integerDivisionOfLiteralsIsStillAssignableToReal` pins the behaviour meanwhile, - so whichever way it goes is deliberate rather than accidental. +- **8. Settled: `div` and `mod` keep returning the left operand's type**, so `real r = 7 div 2` + compiles and is meant to. The branch in `AttrExprType` now says so, rather than looking like an + oversight next to `caseMathOperation`, which collapses two literals to int precisely so + `real r = 1 + 1` is an error. `ExpressionTests.integerDivisionOfLiteralsIsStillAssignableToReal` + pins it and `OptimizerTests.realFormatting_consistent_fromIntOps` depends on it. Nothing to do. - **Eliminating the remaining `castTo int`.** The motivating case is timer data attachment (`ClosureTimers.wurst`), and the containers behind it: `Table` has 81 casts, `HashList` 13, @@ -234,6 +211,10 @@ itself, and one gap in what the suite can see. question: what the syntax is, where such an instance may be declared under the orphan rule, and whether a specific instance always beats a family one. Do not start this autonomously. + Deferred deliberately, not forgotten. It decides whether type class bounds stay a tool for new + containers or become how the existing ones work, which is worth deciding when there is appetite for + the language design rather than alongside compiler work. + ## Out of scope - The stdlib itself. `de.peeeq.wurstscript/temp/WurstStdlib2` is a fetched artefact for tests; diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/attributes/AttrExprType.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/attributes/AttrExprType.java index 1a385b49b..fd15c06af 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/attributes/AttrExprType.java +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/attributes/AttrExprType.java @@ -333,6 +333,14 @@ public static WurstType calculate(final ExprBinary term) { case JASS_MOD_INT: case DIV_INT: if (leftType.isSubtypeOf(WurstTypeInt.instance(), term) && rightType.isSubtypeOf(WurstTypeInt.instance(), term)) { + // The left operand's type, deliberately, which is not what caseMathOperation does: + // that collapses two literals to int so `real r = 1 + 1` is an error. Returning it + // here means `real r = 7 div 2` compiles, and it is meant to - the division is + // integer either way and the result is then widened. + // + // Asked and settled rather than left as an accident. + // ExpressionTests.integerDivisionOfLiteralsIsStillAssignableToReal pins it, and + // OptimizerTests.realFormatting_consistent_fromIntOps opens with that assignment. return leftType; } term.addError("Operator " + term.getOp() + " is not defined for " + From 6d88a4ed9a1af821bfdd949b1d0fc91790edfe62 Mon Sep 17 00:00:00 2001 From: Frotty Date: Mon, 17 Aug 2026 16:21:31 +0200 Subject: [PATCH 2/2] Pin the dispatch bug an underscore in a method name causes A dispatch slot's name is composed from the segment after the last underscore of a method's mangled name, which is the declared name only when the declared name has no underscore in it. get_it contributes it, which is nobody's method, and the slot the call goes through is not the one the override was bound to. Found by auditing the junk-slot rule for what its name comparison does to unrelated methods, not by anyone hitting it, so it is worth pinning rather than leaving to be met by surprise. The test asserts the failure it currently produces and names what the fix has to be keyed on. Asking the declaration for the name is not that fix. declaredName already reads it off the trace, but two overloads share a declared name and mangle to Foo_bar and Foo_bar_1, so the segment after the last underscore is also what keeps their slots apart today. Pointing both composers at the declared name fixes the underscore case and collapses overloaded slots instead. What a method and its overrides share, and what tells two overloads apart, are different questions, and the mangled name is answering both at once by accident. --- BACKLOG.md | 28 ++++++++++++++ .../tests/LuaTranslationTests.java | 38 +++++++++++++++++++ 2 files changed, 66 insertions(+) diff --git a/BACKLOG.md b/BACKLOG.md index 6f87662ea..561a46df4 100644 --- a/BACKLOG.md +++ b/BACKLOG.md @@ -189,6 +189,34 @@ itself, and one gap in what the suite can see. path, and `transformGenericNewOnly` does not lift them, so the method's own parameter is counted against a list which does not include the class's. +26. **Nothing in the compiler should recover structure from a name, and one place doing it is a bug + today.** A dispatch slot's name is composed from the segment after the last underscore of a + method's mangled name, which is the declared name only when the declared name has no underscore in + it: `get_it` contributes `it`, which is nobody's method. + + That is not only untidy. `LuaTranslationTests.underscoreNamedOverrideInAGenericHierarchyIsStillBrokenOnLua` + pins the consequence - an override named `get_it` in a generic hierarchy does not dispatch on Lua, + while the same shape without the underscore does. Found by auditing the junk-slot rule for what its + name comparison does to unrelated methods, not by anyone hitting it. + + Asking the declaration instead is not the whole fix, and trying it is how the rest of this was + found. `LuaDispatchPreparation.declaredName` already reads the name off the trace, but two + overloads share a declared name: they mangle to `Foo_bar` and `Foo_bar_1`, and the segment after + the last underscore is also what currently keeps their slots apart. Pointing both composers at the + declared name fixes the underscore case and collapses overloaded slots instead - + `overloadedMethodsDoNotAliasInLuaDispatchTables` and + `moduleProvidedOverloadedOverrideDoesNotCollapseLuaSlots` both catch it. + + So the replacement is keyed on the declared signature, not the declared name: what a method and its + overrides share, and what distinguishes two overloads, are two different questions and the mangled + name is currently answering both at once by accident. Same shape as #1248 and #1249 - record it + where it is known - but it is a signature, not a rename. + + The rest of the family, for the same treatment: `ProgramState.identifyGenericStaticGlobals` takes + the longest prefix of a global's name ending at an underscore which matches a class name, which a + class whose name contains an underscore answers wrongly and silently. #1249 made the recorded owner + preferred where one exists, so this is now only the fallback. + 12. **Standing item, never finished.** When nothing above is left, find the next thing worth doing and add it here rather than stopping. Good sources, in order: a test that would have caught a bug already found; a place where two mechanisms do the same job and disagree; a diff --git a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LuaTranslationTests.java b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LuaTranslationTests.java index c64ac4144..93cbc66cd 100644 --- a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LuaTranslationTests.java +++ b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LuaTranslationTests.java @@ -1408,6 +1408,44 @@ public void genericOverrideChainBindsRootSlotToMostSpecificImplInLua() throws IO } } + /** + * An override whose declared name contains an underscore does not dispatch inside a generic + * hierarchy on Lua, and this pins that rather than leaving it to be met by surprise. + *

+ * A dispatch slot's name is composed from the segment after the last underscore of the method's + * mangled name, so {@code get_it} contributes {@code it} - which is nobody's method - and the slot + * the call goes through is not the one the override was bound to. The same shape without the + * underscore works, and so does this one outside a generic hierarchy. + *

+ * Found by auditing the junk-slot rule for what its name comparison does to unrelated methods, + * rather than by anyone hitting it. Asking the declaration for the name instead is not the whole + * fix: two overloads share a declared name and mangle to {@code Foo_bar} and {@code Foo_bar_1}, so + * the segment after the last underscore is also what currently keeps their slots apart. Backlog + * item 26 carries what the replacement has to be keyed on. + */ + @Test(expectedExceptions = Error.class, expectedExceptionsMessageRegExp = ".*Succeed function not called.*") + public void underscoreNamedOverrideInAGenericHierarchyIsStillBrokenOnLua() { + test().testLua(true).executeProg().lines( + "package test", + "native testSuccess()", + "class Holder", + " T value", + " construct(T value)", + " this.value = value", + " function get_it() returns T", + " return value", + "class Doubler extends Holder", + " construct(int value)", + " super(value)", + " override function get_it() returns int", + " return value * 2", + "init", + " Holder h = new Doubler(21)", + " if h.get_it() == 42", + " testSuccess()" + ); + } + @Test public void luaOutputIsDeterministicForGenericOverrideSlots() throws IOException { test().testLua(true).compilationUnits(genericOverrideReproUnits());