Record the four decisions, and settle div and mod

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<T: Show>` 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<int>`) 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
@@ -200,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
@@ -208,23 +225,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 +239,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;

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 " +

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.
+     * <p>
+     * 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.
+     * <p>
+     * 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>",
+            "    T value",
+            "    construct(T value)",
+            "        this.value = value",
+            "    function get_it() returns T",
+            "        return value",
+            "class Doubler extends Holder<int>",
+            "    construct(int value)",
+            "        super(value)",
+            "    override function get_it() returns int",
+            "        return value * 2",
+            "init",
+            "    Holder<int> h = new Doubler(21)",
+            "    if h.get_it() == 42",
+            "        testSuccess()"
+        );
+    }
+
     @Test
     public void luaOutputIsDeterministicForGenericOverrideSlots() throws IOException {
         test().testLua(true).compilationUnits(genericOverrideReproUnits());