From 7a23c8f36f885b76e61003dfcd8fbd640c888d62 Mon Sep 17 00:00:00 2001 From: Frotty Date: Mon, 17 Aug 2026 18:06:42 +0200 Subject: [PATCH 01/12] Pin what recovering a dispatch slot name from another name costs The slot name is composed by cutting a method's name at its last underscore and taking the tail. That tail is the declared name only when the declared name has no underscore in it and the method is not a specialised copy, and every way that assumption fails has now produced a bug. Three tests, one per shape that matters. An override named get_it in a generic hierarchy does not dispatch, which is asserted as the failure it currently produces rather than left to be met by surprise. An override of a numbered overload is reached through its base, which the strict version of this rule broke and which passes here. A type argument named like a numbered overload does not steal a slot, which was raised against a tolerance for that number and holds for a structural reason - the type argument is part of the owning class's name, not the tail of the method's. The backlog entry carries why the two obvious fixes do not work, since each was tried: asking the declaration alone collapses overloads, and using the method's name whole breaks cross-class matching because at that point the name is still class-prefixed. The name has to arrive as data, which is what this branch does next. --- BACKLOG.md | 142 ++++++++++-------- .../tests/LuaTranslationTests.java | 96 ++++++++++++ 2 files changed, 173 insertions(+), 65 deletions(-) diff --git a/BACKLOG.md b/BACKLOG.md index aa81417ac..fc65a0738 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 @@ -200,6 +189,37 @@ 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. **A dispatch slot's name is recovered from another name instead of being asked for, and that is + where four bugs came from.** The slot name is composed by cutting a method's name at its last + underscore and taking the tail. That tail is the declared name only when the declared name has no + underscore in it and the method is not a specialised copy, so: + + - `get_it` contributes `it`, which is nobody's method, and an override named `get_it` in a generic + hierarchy does not dispatch on Lua. Pinned by + `LuaTranslationTests.underscoreNamedOverrideInAGenericHierarchyIsStillBrokenOnLua`. + - a specialised method's tail is the type argument, composing a slot named after a type rather than + a method - the case `dea459b45` stopped by refusing the composition. + - an overload numbered by the translation carries the number in the tail, so requiring the tail to + equal the declared name loses the slot an override of that overload has to replace. Covered by + `overloadedOverrideOnAGenericBaseIsReachedThroughTheBase`. + - a type argument named like a numbered overload could in principle collide with a tolerance for + that number. Not reachable, and covered by `aTypeNamedLikeAnOverloadNumberDoesNotStealTheSlot`. + + Two attempts at the obvious fix both fail, and both failures say what the real one has to be. + Asking `declaredName` alone collapses overloads, because two overloads share a declared name - + `overloadedMethodsDoNotAliasInLuaDispatchTables` catches it. Using the method's name whole instead + of its tail breaks cross-class matching, because at the point slots are composed a method's name is + still class-prefixed: `GlobalCheckState_update`, where the ancestor's slot is `State_update`. The + tail is load-bearing precisely because the prefix is there. + + So the name has to arrive as data rather than be recovered from a string: `ImMethod` carries its + declared name and the index the translation gave it among its overloads, recorded where the + translation assigns them, and slots compose from that pair. `luaDispatchGroupKey` is already a + recorded field on the method, so the shape exists. Then the cut is deleted rather than tolerated, + both composers ask one question, the pin above starts passing, and + `ProgramState.identifyGenericStaticGlobals` - which takes the longest prefix of a global's name + ending at an underscore that matches a class name - gets the same treatment. + 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 +228,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 +242,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/test/java/tests/wurstscript/tests/LuaTranslationTests.java b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LuaTranslationTests.java index c64ac4144..eb436a5ec 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,102 @@ public void genericOverrideChainBindsRootSlotToMostSpecificImplInLua() throws IO } } + @Test + public void overloadedOverrideOnAGenericBaseIsReachedThroughTheBase() { + test().testLua(true).executeProg().lines( + "package test", + "native testSuccess()", + "class Base", + " function route(T t) returns int", + " return 1", + " function route(T t, int extra) returns int", + " return 2", + "class Child extends Base", + " override function route(int t) returns int", + " return 10", + " override function route(int t, int extra) returns int", + " return 20", + "init", + " Base b = new Child()", + " if b.route(1) == 10 and b.route(1, 2) == 20", + " testSuccess()" + ); + } + + /** + * A type argument whose name is a method's name followed by a number, which is what the rule + * allowing an overload number could in principle be fooled by. + *

+ * It holds, because the type argument is part of the owning class's name rather than the tail of the + * method's: the segment a slot name is composed from is {@code route} for {@code route} and + * {@code route1} for {@code route1}, and neither needs the number tolerated. The case is kept + * because it was raised against that rule and reasoning about which segment carries the type is + * exactly the kind of thing to check rather than argue about. + */ + /** + * A type argument whose name is a method's name followed by a number, which is what the rule + * allowing an overload number could in principle be fooled by. + *

+ * It holds, because the type argument is part of the owning class's name rather than the tail of the + * method's: the segment a slot name is composed from is {@code route} for {@code route} and + * {@code route1} for {@code route1}, and neither needs the number tolerated. The case is kept + * because it was raised against that rule, and which segment carries the type argument is exactly + * the kind of thing to check rather than argue about. + */ + @Test + public void aTypeNamedLikeAnOverloadNumberDoesNotStealTheSlot() { + test().testLua(true).executeProg().lines( + "package test", + "native testSuccess()", + "class route1", + " int v = 3", + "class Holder", + " T item", + " construct(T item)", + " this.item = item", + " function route() returns int", + " return 1", + " function route1() returns int", + " return 2", + "init", + " let h = new Holder(new route1())", + " if h.route() == 1 and h.route1() == 2", + " testSuccess()" + ); + } + + /** + * 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 by cutting the method's name at its last underscore and taking + * the tail, 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. + */ + @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()); From 375d18511c795e2102db40aa272f182442ccf0f8 Mon Sep 17 00:00:00 2001 From: Frotty Date: Mon, 17 Aug 2026 18:36:15 +0200 Subject: [PATCH 02/12] Record the attempt which moved the pin, and why it still fails Stripping the owner's name as a known prefix instead of searching for the last underscore does fix the underscore case - the pinned test flipped to passing, the first time anything has moved it - and breaks two override-chain tests instead. normalizeMethodNames assigns one name per dispatch group, derived from the first member's already class-prefixed name, and sets it on every member. So the prefix a method's name carries is not necessarily its own owner's: an ancestor's method can be named after a descendant's class. No prefix known locally is the right anchor, and cutting at the last underscore survives that by accident. Which settles where the recording has to happen rather than leaving it open: the one point holding both the group and the name assigned to it. --- BACKLOG.md | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/BACKLOG.md b/BACKLOG.md index fc65a0738..c7de6e88a 100644 --- a/BACKLOG.md +++ b/BACKLOG.md @@ -212,6 +212,17 @@ itself, and one gap in what the suite can see. still class-prefixed: `GlobalCheckState_update`, where the ancestor's slot is `State_update`. The tail is load-bearing precisely because the prefix is there. + A third attempt gets closest and shows why none of these can work. Stripping the owner's name as a + known prefix - the boundary is not a guess, the owner is right there to be asked - does fix the + underscore case, and the pin above flipped to passing, the first time anything has moved it. It + breaks `genericOverrideChainBindsRootSlotToMostSpecificImplInLua` and + `genericOverrideChainBindsGlobalStateSlotToMostSpecificImplInLua` instead, because + `normalizeMethodNames` assigns one name per dispatch group derived from the first member's already + class-prefixed name and sets it on every member. The prefix a method's name carries is therefore not + necessarily its own owner's - an ancestor's method can be named after a descendant's class - so no + prefix known locally is the right anchor. Cutting at the last underscore survives that by accident, + which is the whole reason it is still here. + So the name has to arrive as data rather than be recovered from a string: `ImMethod` carries its declared name and the index the translation gave it among its overloads, recorded where the translation assigns them, and slots compose from that pair. `luaDispatchGroupKey` is already a From f44a32d280d98e5b41455dc6669536a6b4cd9461 Mon Sep 17 00:00:00 2001 From: Frotty Date: Mon, 17 Aug 2026 18:40:15 +0200 Subject: [PATCH 03/12] Record the segment where it is assigned, not where it could be rebuilt The plan said to record the declared name and an overload index where the translation assigns method names. That is not equivalent to the segment the slot name is composed from: normalizeMethodNames may derive a group's name from a different member of the group, and it sanitises the name into a Lua identifier and uniques it against everything already taken. A pair recorded earlier would have to be matched back to whatever came out of that, which is the recovery problem again under a new name. The authoritative segment is the one normalizeMethodNames produced, so the field is filled there, where the group and its assigned name are both in hand. --- BACKLOG.md | 26 +++++++++++++++++++------- 1 file changed, 19 insertions(+), 7 deletions(-) diff --git a/BACKLOG.md b/BACKLOG.md index c7de6e88a..1e7910b63 100644 --- a/BACKLOG.md +++ b/BACKLOG.md @@ -223,13 +223,25 @@ itself, and one gap in what the suite can see. prefix known locally is the right anchor. Cutting at the last underscore survives that by accident, which is the whole reason it is still here. - So the name has to arrive as data rather than be recovered from a string: `ImMethod` carries its - declared name and the index the translation gave it among its overloads, recorded where the - translation assigns them, and slots compose from that pair. `luaDispatchGroupKey` is already a - recorded field on the method, so the shape exists. Then the cut is deleted rather than tolerated, - both composers ask one question, the pin above starts passing, and - `ProgramState.identifyGenericStaticGlobals` - which takes the longest prefix of a global's name - ending at an underscore that matches a class name - gets the same treatment. + So the segment has to arrive as data, recorded at the one point which knows it: + `LuaDispatchPreparation.normalizeMethodNames`. That is where a dispatch group is given its name - + sanitised into a Lua identifier and uniqued against everything already taken - and where the group + is in hand to strip its own prefix from it. `ImMethod` carries the result in a field beside + `luaDispatchGroupKey`, so a grammar change and `genAst`, and both composers then read the field + instead of cutting a string. + + Recording the source declaration and an overload index earlier in translation looks equivalent and + is not: the assigned name may be derived from a different member of the group, and sanitising and + uniquing can change it. A declaration-derived pair would have to be matched back to it, which is the + same recovery problem again under a new name. The authoritative segment is the one + `normalizeMethodNames` produced, so that is the one to keep. + + The rest of the family, for the same treatment once this exists: + `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 From 929235081c71b7066f723568d69743751639d74e Mon Sep 17 00:00:00 2001 From: Frotty Date: Mon, 17 Aug 2026 18:47:01 +0200 Subject: [PATCH 04/12] Say in the code why div and mod keep the left operand's type The backlog claimed this branch explains itself and it did not. Returning the left operand's type is what makes real r = 7 div 2 compile, and caseMathOperation does the opposite a few lines down - collapsing two int literals to int so that real r = 1 + 1 is an error - which makes the difference easy to read as an oversight. It is not one: these operators are integer-only, an int literal is a subtype of real, and narrowing the result would break assignments which compile today. The two tests holding that are named where someone changing this will see them. --- .../java/de/peeeq/wurstscript/attributes/AttrExprType.java | 7 +++++++ 1 file changed, 7 insertions(+) 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..d0d065e87 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 @@ -332,6 +332,13 @@ public static WurstType calculate(final ExprBinary term) { case MOD_INT: case JASS_MOD_INT: case DIV_INT: + // The left operand's type is returned deliberately, so that `real r = 7 div 2` compiles. + // caseMathOperation below does the opposite for + - * /, collapsing two int literals to + // int precisely so that `real r = 1 + 1` is an error, and the difference between the two + // is easy to read as an oversight here. It is not: these operators are integer-only, an + // int literal is a subtype of real, and narrowing the result would break assignments + // which compile today. ExpressionTests.integerDivisionOfLiteralsIsStillAssignableToReal + // pins it, and OptimizerTests.realFormatting_consistent_fromIntOps depends on it. if (leftType.isSubtypeOf(WurstTypeInt.instance(), term) && rightType.isSubtypeOf(WurstTypeInt.instance(), term)) { return leftType; } From 785115580275451c41b484e4bdcb01f80a9c8e4b Mon Sep 17 00:00:00 2001 From: Frotty Date: Mon, 17 Aug 2026 19:03:22 +0200 Subject: [PATCH 05/12] Record a dispatch slot's segment instead of cutting it out of a name The segment a slot name is composed from was found by cutting a method's name at its last underscore, which is the right answer only when the rest contains no underscore of its own and the method is not a specialised copy. A method declared get_it composed a slot called it, which is nobody's method, so the override and the call it should answer went through different slots. normalizeMethodNames names a dispatch group after one of its members, sanitises that name into a Lua identifier and uniques it against every name taken. It now strips the naming member's class from the result and records the segment on the translator for every member of the group, and both composers read the record. semanticNameFromMethodName is deleted from both. Nothing else can compute this. A method cannot strip its own class, because the group is named after one member and an ancestor's method can carry a descendant's class in its name - that attempt fixes the underscore case and breaks override chains. The declaration cannot supply it either, because overloads share a declared name and because sanitising and uniquing may change what the group ended up called. The pinned failure is now a positive test. --- BACKLOG.md | 76 ++++++----------- .../peeeq/wurstio/WurstCompilerJassImpl.java | 2 +- .../imtranslation/ImTranslator.java | 24 ++++++ .../imtranslation/LuaDispatchPreparation.java | 82 ++++++++++--------- .../lua/translation/LuaTranslator.java | 15 +--- .../tests/LuaTranslationTests.java | 15 ++-- 6 files changed, 101 insertions(+), 113 deletions(-) diff --git a/BACKLOG.md b/BACKLOG.md index 1e7910b63..371b6d7e7 100644 --- a/BACKLOG.md +++ b/BACKLOG.md @@ -189,59 +189,29 @@ 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. **A dispatch slot's name is recovered from another name instead of being asked for, and that is - where four bugs came from.** The slot name is composed by cutting a method's name at its last - underscore and taking the tail. That tail is the declared name only when the declared name has no - underscore in it and the method is not a specialised copy, so: - - - `get_it` contributes `it`, which is nobody's method, and an override named `get_it` in a generic - hierarchy does not dispatch on Lua. Pinned by - `LuaTranslationTests.underscoreNamedOverrideInAGenericHierarchyIsStillBrokenOnLua`. - - a specialised method's tail is the type argument, composing a slot named after a type rather than - a method - the case `dea459b45` stopped by refusing the composition. - - an overload numbered by the translation carries the number in the tail, so requiring the tail to - equal the declared name loses the slot an override of that overload has to replace. Covered by - `overloadedOverrideOnAGenericBaseIsReachedThroughTheBase`. - - a type argument named like a numbered overload could in principle collide with a tolerance for - that number. Not reachable, and covered by `aTypeNamedLikeAnOverloadNumberDoesNotStealTheSlot`. - - Two attempts at the obvious fix both fail, and both failures say what the real one has to be. - Asking `declaredName` alone collapses overloads, because two overloads share a declared name - - `overloadedMethodsDoNotAliasInLuaDispatchTables` catches it. Using the method's name whole instead - of its tail breaks cross-class matching, because at the point slots are composed a method's name is - still class-prefixed: `GlobalCheckState_update`, where the ancestor's slot is `State_update`. The - tail is load-bearing precisely because the prefix is there. - - A third attempt gets closest and shows why none of these can work. Stripping the owner's name as a - known prefix - the boundary is not a guess, the owner is right there to be asked - does fix the - underscore case, and the pin above flipped to passing, the first time anything has moved it. It - breaks `genericOverrideChainBindsRootSlotToMostSpecificImplInLua` and - `genericOverrideChainBindsGlobalStateSlotToMostSpecificImplInLua` instead, because - `normalizeMethodNames` assigns one name per dispatch group derived from the first member's already - class-prefixed name and sets it on every member. The prefix a method's name carries is therefore not - necessarily its own owner's - an ancestor's method can be named after a descendant's class - so no - prefix known locally is the right anchor. Cutting at the last underscore survives that by accident, - which is the whole reason it is still here. - - So the segment has to arrive as data, recorded at the one point which knows it: - `LuaDispatchPreparation.normalizeMethodNames`. That is where a dispatch group is given its name - - sanitised into a Lua identifier and uniqued against everything already taken - and where the group - is in hand to strip its own prefix from it. `ImMethod` carries the result in a field beside - `luaDispatchGroupKey`, so a grammar change and `genAst`, and both composers then read the field - instead of cutting a string. - - Recording the source declaration and an overload index earlier in translation looks equivalent and - is not: the assigned name may be derived from a different member of the group, and sanitising and - uniquing can change it. A declaration-derived pair would have to be matched back to it, which is the - same recovery problem again under a new name. The authoritative segment is the one - `normalizeMethodNames` produced, so that is the one to keep. - - The rest of the family, for the same treatment once this exists: - `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. - +26. **Done. A dispatch slot's segment is recorded where it is assigned, not recovered from a name.** + The segment used to be found by cutting a method's name at its last underscore, which is the right + answer only when the rest contains no underscore and the method is not a specialised copy. Four bugs + came out of that: a method declared `get_it` composed a slot called `it` and its override never + reached it; a specialised method composed a slot named after its type argument; requiring the cut to + equal the declared name lost the slot an override of a numbered overload has to replace. + + `LuaDispatchPreparation.normalizeMethodNames` names a dispatch group after one member, sanitises + that name into a Lua identifier and uniques it, and now strips the naming member's class from it and + records the result on `ImTranslator` for every member of the group. Both composers read the record. + `semanticNameFromMethodName` is gone from both. + + Three earlier attempts are worth remembering, because each looked equivalent and was not. Asking the + declaration alone collapses overloads, which share a declared name. Using a method's name whole + breaks cross-class matching, since the name is class-prefixed. Stripping each method's *own* owner + fixes the underscore case and breaks override chains, because a group is named after one member and + an ancestor's method can carry a descendant's class in its name - which is exactly why no method can + work its segment out for itself, and why the recording happens where the group is in hand. + + `LuaTranslationTests.underscoreNamedOverrideDispatchesInAGenericHierarchy` is now a positive test. + Still in the same family: `ProgramState.identifyGenericStaticGlobals` takes the longest prefix of a + global's name ending at an underscore which matches a class name. #1249 made the recorded owner + preferred where one exists, so this is only the fallback, and it should stop being reachable. 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 diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstio/WurstCompilerJassImpl.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstio/WurstCompilerJassImpl.java index 1a59dd5bc..de2cd9c1d 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstio/WurstCompilerJassImpl.java +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstio/WurstCompilerJassImpl.java @@ -964,7 +964,7 @@ public LuaCompilationUnit transformProgToLua() { timeTaker.endPhase(); beginPhase(13, "prepare lua dispatch"); - LuaDispatchPreparation.prepare(imProg); + LuaDispatchPreparation.prepare(imProg, imTranslator); timeTaker.endPhase(); beginPhase(14, "translate to lua"); diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/ImTranslator.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/ImTranslator.java index 455aedeb5..248d20691 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/ImTranslator.java +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/ImTranslator.java @@ -2453,4 +2453,28 @@ int getCompiletimeExpressionsOrder(FunctionCall fc) { public RunArgs getRunArgs() { return runArgs; } + + private final Map dispatchSegments = new IdentityHashMap<>(); + + /** + * The part of a dispatch group's assigned name which identifies the method rather than a class. + *

+ * Recorded by {@code LuaDispatchPreparation.normalizeMethodNames}, the only place which knows it: it + * names a whole group after one member's already class-prefixed name, sanitises that into a Lua + * identifier, and uniques it against every name taken. Reconstructing the segment afterwards means + * cutting the result at a boundary nobody recorded, which is where four dispatch bugs came from - + * most memorably a method declared {@code get_it} composing a slot called {@code it}. + */ + public void recordDispatchSegment(ImMethod method, String segment) { + dispatchSegments.put(method, segment); + } + + /** The recorded segment, or the method's whole name when nothing recorded one - never a cut. */ + public String dispatchSegmentOf(ImMethod method) { + if (method == null) { + return ""; + } + String segment = dispatchSegments.get(method); + return segment != null ? segment : method.getName(); + } } diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/LuaDispatchPreparation.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/LuaDispatchPreparation.java index 4891947a2..1deaf553b 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/LuaDispatchPreparation.java +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/LuaDispatchPreparation.java @@ -34,11 +34,11 @@ public final class LuaDispatchPreparation { private LuaDispatchPreparation() { } - public static void prepare(ImProg prog) { + public static void prepare(ImProg prog, ImTranslator tr) { List allMethods = collectAllMethods(prog); assignDispatchGroupKeys(allMethods); - normalizeMethodNames(prog, allMethods); - assignDispatchAliases(prog, allMethods); + normalizeMethodNames(prog, allMethods, tr); + assignDispatchAliases(prog, allMethods, tr); } private static List collectAllMethods(ImProg prog) { @@ -87,7 +87,7 @@ private static void assignDispatchGroupKeys(List allMethods) { } } - private static void normalizeMethodNames(ImProg prog, List allMethods) { + private static void normalizeMethodNames(ImProg prog, List allMethods, ImTranslator tr) { Set usedNames = new HashSet<>(LUA_RESERVED_NAMES); collectPredefinedNames(prog, usedNames); @@ -105,24 +105,31 @@ private static void normalizeMethodNames(ImProg prog, List allMethods) // The name is about to become a Lua table key. Sanitising before uniquing means two // names that only differed in characters Lua has no place for still get one slot each. String name = uniqueName(LuaIdentifiers.toIdentifier(group.get(0).getName()), usedNames); + // The group is named after one member, whose name is that member's own class and then + // the method, so stripping the class here is the one place the boundary is known rather + // than guessed at. Every member shares the segment, including members of other classes + // whose own name appears nowhere in it - which is why no method can work this out for + // itself afterwards. + String segment = segmentOf(name, group.get(0)); for (ImMethod method : group) { method.setName(name); + tr.recordDispatchSegment(method, segment); } } } - private static void assignDispatchAliases(ImProg prog, List allMethods) { + private static void assignDispatchAliases(ImProg prog, List allMethods, ImTranslator tr) { Map> sortedMethodsByClass = new HashMap<>(); Map> closureFamilyAnchorsCache = new HashMap<>(); Map> closureFamilyClassesByAnchor = new HashMap<>(); - Set ambiguousDirectAliases = ambiguousDirectAliases(allMethods); + Set ambiguousDirectAliases = ambiguousDirectAliases(allMethods, tr); for (ImMethod method : allMethods) { TreeSet aliases = new TreeSet<>(); - addDirectAliases(method, aliases, ambiguousDirectAliases); - addHierarchyAliases(method, aliases, sortedMethodsByClass); - addClosureFamilyAliases(prog, method, aliases, sortedMethodsByClass, closureFamilyAnchorsCache, closureFamilyClassesByAnchor); + addDirectAliases(method, aliases, ambiguousDirectAliases, tr); + addHierarchyAliases(method, aliases, sortedMethodsByClass, tr); + addClosureFamilyAliases(prog, method, aliases, sortedMethodsByClass, closureFamilyAnchorsCache, closureFamilyClassesByAnchor, tr); method.setLuaMethodDispatchAliases(new ArrayList<>(aliases)); } } @@ -160,11 +167,11 @@ private static String uniqueName(String name, Set usedNames) { * arbitrarily" is worse than a name meaning nothing. {@code LuaTranslator} skips composing the * matching slot for the same reason. */ - private static Set ambiguousDirectAliases(List allMethods) { + private static Set ambiguousDirectAliases(List allMethods, ImTranslator tr) { Map claimedBy = new LinkedHashMap<>(); Set ambiguous = new HashSet<>(); for (ImMethod method : allMethods) { - String composed = directAliasFor(method); + String composed = directAliasFor(method, tr); if (composed == null) { continue; } @@ -186,12 +193,12 @@ private static Set ambiguousDirectAliases(List allMethods) { return ambiguous; } - private static @Nullable String directAliasFor(ImMethod method) { + private static @Nullable String directAliasFor(ImMethod method, ImTranslator tr) { if (method == null) { return null; } ImClass owner = method.attrClass(); - String semanticName = semanticNameFromMethodName(method.getName()); + String semanticName = tr.dispatchSegmentOf(method); if (owner == null || semanticName.isEmpty()) { return null; } @@ -199,7 +206,7 @@ private static Set ambiguousDirectAliases(List allMethods) { } private static void addDirectAliases(ImMethod method, Set aliases, - Set ambiguousDirectAliases) { + Set ambiguousDirectAliases, ImTranslator tr) { if (method == null) { return; } @@ -208,7 +215,7 @@ private static void addDirectAliases(ImMethod method, Set aliases, aliases.add(methodName); } ImClass owner = method.attrClass(); - String composed = directAliasFor(method); + String composed = directAliasFor(method, tr); if (composed != null && !ambiguousDirectAliases.contains(composed)) { aliases.add(composed); } @@ -219,21 +226,21 @@ private static void addDirectAliases(ImMethod method, Set aliases, } } - private static void addHierarchyAliases(ImMethod method, Set aliases, Map> sortedMethodsByClass) { + private static void addHierarchyAliases(ImMethod method, Set aliases, Map> sortedMethodsByClass, ImTranslator tr) { ImClass owner = method.attrClass(); if (owner == null) { return; } - Set semanticNames = semanticNames(method); + Set semanticNames = semanticNames(method, tr); if (semanticNames.isEmpty()) { return; } String dispatchKey = dispatchSignatureKey(method); - collectHierarchyAliases(owner, method, dispatchKey, semanticNames, aliases, sortedMethodsByClass, new HashSet<>()); + collectHierarchyAliases(owner, method, dispatchKey, semanticNames, aliases, sortedMethodsByClass, new HashSet<>(), tr); } private static void collectHierarchyAliases(ImClass c, ImMethod method, String dispatchKey, Set semanticNames, Set aliases, - Map> sortedMethodsByClass, Set visited) { + Map> sortedMethodsByClass, Set visited, ImTranslator tr) { if (c == null || !visited.add(c)) { return; } @@ -241,7 +248,7 @@ private static void collectHierarchyAliases(ImClass c, ImMethod method, String d if (!dispatchKey.equals(dispatchSignatureKey(candidate))) { continue; } - if (!sharesSemanticName(method, candidate, semanticNames)) { + if (!sharesSemanticName(method, candidate, semanticNames, tr)) { continue; } String candidateName = candidate.getName(); @@ -251,19 +258,19 @@ private static void collectHierarchyAliases(ImClass c, ImMethod method, String d } } for (ImClassType sc : c.getSuperClasses()) { - collectHierarchyAliases(sc.getClassDef(), method, dispatchKey, semanticNames, aliases, sortedMethodsByClass, visited); + collectHierarchyAliases(sc.getClassDef(), method, dispatchKey, semanticNames, aliases, sortedMethodsByClass, visited, tr); } } private static void addClosureFamilyAliases(ImProg prog, ImMethod method, Set aliases, Map> sortedMethodsByClass, Map> closureFamilyAnchorsCache, - Map> closureFamilyClassesByAnchor) { + Map> closureFamilyClassesByAnchor, ImTranslator tr) { ImClass owner = method.attrClass(); if (owner == null || !isClosureGeneratedClass(owner)) { return; } - Set semanticNames = semanticNames(method); + Set semanticNames = semanticNames(method, tr); if (semanticNames.isEmpty()) { return; } @@ -274,7 +281,7 @@ private static void addClosureFamilyAliases(ImProg prog, ImMethod method, Set semanticNames(ImMethod method) { + private static Set semanticNames(ImMethod method, ImTranslator tr) { Set names = new HashSet<>(); - String semanticName = semanticNameFromMethodName(method.getName()); + String semanticName = tr.dispatchSegmentOf(method); if (!semanticName.isEmpty()) { names.add(semanticName); } @@ -309,20 +316,20 @@ private static Set semanticNames(ImMethod method) { * is a fragment of the type argument: two unrelated methods of one specialisation both end in * {@code integer} and would otherwise be taken for one another. */ - private static boolean sharesSemanticName(ImMethod method, ImMethod candidate, Set semanticNames) { + private static boolean sharesSemanticName(ImMethod method, ImMethod candidate, Set semanticNames, ImTranslator tr) { String declared = declaredName(method); String candidateDeclared = declaredName(candidate); if (!declared.isEmpty() && !candidateDeclared.isEmpty()) { return declared.equals(candidateDeclared); } - return sharesSemanticName(candidate, semanticNames); + return sharesSemanticName(candidate, semanticNames, tr); } - private static boolean sharesSemanticName(ImMethod method, Set semanticNames) { + private static boolean sharesSemanticName(ImMethod method, Set semanticNames, ImTranslator tr) { if (semanticNames.isEmpty()) { return false; } - return semanticNames.contains(semanticNameFromMethodName(method.getName())) + return semanticNames.contains(tr.dispatchSegmentOf(method)) || semanticNames.contains(sourceSemanticName(method)); } @@ -458,15 +465,14 @@ private static String classSortKey(ImClass c) { return c == null ? "" : c.getName(); } - private static String semanticNameFromMethodName(String methodName) { - if (methodName == null || methodName.isEmpty()) { - return ""; - } - int lastUnderscore = methodName.lastIndexOf('_'); - if (lastUnderscore >= 0 && lastUnderscore + 1 < methodName.length()) { - return methodName.substring(lastUnderscore + 1); + /** The assigned name without the prefix naming the class whose member the group was named after. */ + private static String segmentOf(String assignedName, ImMethod namedAfter) { + ImClass owner = namedAfter == null ? null : namedAfter.attrClass(); + if (owner == null) { + return assignedName; } - return methodName; + String prefix = LuaIdentifiers.toIdentifier(owner.getName()) + "_"; + return assignedName.startsWith(prefix) ? assignedName.substring(prefix.length()) : assignedName; } private static boolean isClosureGeneratedClass(ImClass c) { diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/lua/translation/LuaTranslator.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/lua/translation/LuaTranslator.java index 05915f337..8f168dcb8 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/lua/translation/LuaTranslator.java +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/lua/translation/LuaTranslator.java @@ -1020,7 +1020,7 @@ private Set collectDispatchSlotNames(ImClass receiverClass, List ambiguousSemanticNames(ImClass c) { if (m == null) { continue; } - String semanticName = semanticNameFromMethodName(m.getName()); + String semanticName = imTr.dispatchSegmentOf(m); if (semanticName.isEmpty()) { continue; } @@ -1240,17 +1240,6 @@ private String dispatchGroupKey(ImMethod method) { return key == null || key.isEmpty() ? methodSortKey(method) : key; } - private String semanticNameFromMethodName(String methodName) { - if (methodName == null || methodName.isEmpty()) { - return ""; - } - int lastUnderscore = methodName.lastIndexOf('_'); - if (lastUnderscore >= 0 && lastUnderscore + 1 < methodName.length()) { - return methodName.substring(lastUnderscore + 1); - } - return methodName; - } - private String sourceSemanticName(ImMethod method) { if (method == null) { return ""; 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 eb436a5ec..0b7ccddae 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 @@ -1473,16 +1473,15 @@ public void aTypeNamedLikeAnOverloadNumberDoesNotStealTheSlot() { } /** - * 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. + * An override whose declared name contains an underscore dispatches inside a generic hierarchy. *

- * A dispatch slot's name is composed by cutting the method's name at its last underscore and taking - * the tail, 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. + * It did not until the segment a slot name is composed from stopped being recovered by cutting a + * method's name at its last underscore: {@code get_it} yielded {@code it}, which is nobody's method, + * so the override and the call it should answer composed different slots. The segment is now + * recorded where a dispatch group is named, and this is the case that proves it. */ - @Test(expectedExceptions = Error.class, expectedExceptionsMessageRegExp = ".*Succeed function not called.*") - public void underscoreNamedOverrideInAGenericHierarchyIsStillBrokenOnLua() { + @Test + public void underscoreNamedOverrideDispatchesInAGenericHierarchy() { test().testLua(true).executeProg().lines( "package test", "native testSuccess()", From 2d8a3870c83282d3990240a4817b26283cafca2c Mon Sep 17 00:00:00 2001 From: Frotty Date: Mon, 17 Aug 2026 19:44:20 +0200 Subject: [PATCH 06/12] Let a module's type parameter carry a type class bound Using a module copies its body into the class and replaces the module's type parameters wherever they are used as types. A requirement of a bound is called on the parameter itself, T.show(x), where T is a name and not a type, so the replacement never reaches it. Renaming it to the using class's parameter is not open either: a module body resolves names in the module's own scope by design, so that name is one the scope deliberately cannot see. The bound was rejected outright rather than mistranslated. The instantiation now declares the module's parameters and records the arguments chosen for them. The body keeps saying T, that name resolves to the instantiation's own declaration, and the argument is right there to say what it stands for. The arguments are recorded resolved rather than copied, because an argument names something only the user's scope can see. Declaring them is all it does. A module instantiation is not an AstElementWithTypeParameters: these are names to look up, not variables for a call to infer, and making them inferable made every method of a generic module's instantiation ask a caller to infer a parameter its signature never mentions. Generic modules keep resolving their parameters by matching the receiver type. A receiver written on such a parameter denotes the argument bound to it. The requirements it offers are the ones the parameter declared, so a module cannot reach a bound it did not ask for, while their parameter and return types are the argument's, which is what the copied body speaks in. Dispatch follows the argument: on the using class's type variable when the argument is itself a parameter, and straight to the instance otherwise, since a module used with a concrete argument leaves no variable for generic elimination to substitute. The bound is checked at the use, which is the only place that sees both the parameter and the argument chosen for it. --- .../parserspec/wurstscript.parseq | 6 + .../de/peeeq/wurstscript/ModuleExpander.java | 17 +++ .../attributes/AttrImplicitParameter.java | 15 ++- .../wurstscript/attributes/AttrNameDef.java | 31 ++++- .../attributes/names/TypeNameLinks.java | 27 +++- .../imtranslation/ExprTranslation.java | 33 ++++- .../types/TypeClassConstraints.java | 78 +++++++++++ .../types/WurstTypeBoundTypeParam.java | 34 ++++- .../wurstscript/types/WurstTypeTypeParam.java | 62 +-------- .../validation/WurstValidator.java | 26 ++-- .../wurstscript/tests/TypeClassTests.java | 122 ++++++++++++++---- 11 files changed, 340 insertions(+), 111 deletions(-) diff --git a/de.peeeq.wurstscript/parserspec/wurstscript.parseq b/de.peeeq.wurstscript/parserspec/wurstscript.parseq index 8d6c33bce..a13dbcbd2 100644 --- a/de.peeeq.wurstscript/parserspec/wurstscript.parseq +++ b/de.peeeq.wurstscript/parserspec/wurstscript.parseq @@ -82,7 +82,13 @@ ClassSlot = ConstructorDef(@ignoreForEquality de.peeeq.wurstscript.parser.WPos source, Modifiers modifiers, WParameters parameters, SuperConstructorCall superConstructorCall, WStatements body) | OnDestroyDef(@ignoreForEquality de.peeeq.wurstscript.parser.WPos source, WStatements body) | ModuleUse(@ignoreForEquality de.peeeq.wurstscript.parser.WPos source, Identifier moduleNameId, TypeExprList typeArgs) + // Carries the module's type parameters, and the arguments chosen for them, so a name inside the + // copied body still resolves: a requirement of a bound is called on the parameter itself, which + // is a name rather than a type, and a module body resolves names in the module's own scope + // rather than the user's. They are declarations, not parameters left to infer, which is why the + // instantiation is not an AstElementWithTypeParameters. | ModuleInstanciation(@ignoreForEquality de.peeeq.wurstscript.parser.WPos source, Modifiers modifiers, Identifier nameId, + TypeParamDefs typeParameters, TypeExprList typeArgs, ClassDefs innerClasses, FuncDefs methods, GlobalVarDefs vars, ConstructorDefs constructors, ModuleInstanciations p_moduleInstanciations, ModuleUses moduleUses, OnDestroyDef onDestroy) | ClassMember diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/ModuleExpander.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/ModuleExpander.java index e133a66a9..49b75b390 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/ModuleExpander.java +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/ModuleExpander.java @@ -75,8 +75,25 @@ private static ModuleInstanciations expandModules(ClassOrModule m, List + * A parameter declared on a module instantiation stands for the argument the user supplied, so it + * denotes that type bound to this parameter: the requirement's own parameter types then substitute + * to the argument rather than to a name only the module can see. Everywhere else the parameter + * stands for itself. + */ + private static WurstType staticRefTypeFor(TypeParamDef tp, NameRef node) { + WurstType argument = moduleInstanciationArgument(tp); + if (argument != null) { + return new WurstTypeBoundTypeParam(tp, argument, node).asStaticRef(); + } + return new WurstTypeTypeParam(tp).asStaticRef(); + } + + /** The argument a module instantiation supplied for this parameter, or null if it is not one. */ + private static @Nullable WurstType moduleInstanciationArgument(TypeParamDef tp) { + if (!(tp.getParent() != null && tp.getParent().getParent() instanceof ModuleInstanciation mi)) { + return null; + } + int index = mi.getTypeParameters().indexOf(tp); + if (index < 0 || index >= mi.getTypeArgs().size()) { + return null; + } + return mi.getTypeArgs().get(index).attrTyp(); + } + private static @Nullable NameLink lookupImplicitClosureSelf(NameRef node, boolean showErrors) { ExprClosure closure = node.attrNearestExprClosure(); if (closure == null) { diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/attributes/names/TypeNameLinks.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/attributes/names/TypeNameLinks.java index 341ac30b6..e72f7c76a 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/attributes/names/TypeNameLinks.java +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/attributes/names/TypeNameLinks.java @@ -4,6 +4,9 @@ import com.google.common.collect.ImmutableSetMultimap; import de.peeeq.wurstscript.ast.*; +import java.util.Collections; +import java.util.List; + public class TypeNameLinks { public static ImmutableMultimap calculate(ClassOrModuleOrModuleInstanciation c) { @@ -94,13 +97,27 @@ public static ImmutableMultimap calculate(WStatements statemen } private static void addTypeParametersIfAny(ImmutableMultimap.Builder result, WScope c) { - if (c instanceof AstElementWithTypeParameters) { - AstElementWithTypeParameters wtp = (AstElementWithTypeParameters) c; - for (TypeParamDef i : wtp.getTypeParameters()) { - result.put(i.getName(), TypeLink.create(i, c)); - } + for (TypeParamDef i : declaredTypeParameters(c)) { + result.put(i.getName(), TypeLink.create(i, c)); } + } + /** + * The type parameter names a scope introduces. + *

+ * A module instantiation declares the module's parameters so that a receiver written on one + * still resolves once the body has been copied out of the module's scope. It is not an + * {@link AstElementWithTypeParameters}: the arguments are recorded on the instantiation, so + * these are names to look up rather than variables for a call to infer. + */ + private static List declaredTypeParameters(WScope c) { + if (c instanceof AstElementWithTypeParameters wtp) { + return wtp.getTypeParameters(); + } + if (c instanceof ModuleInstanciation mi) { + return mi.getTypeParameters(); + } + return Collections.emptyList(); } private static void addJassTypes(ImmutableMultimap.Builder result, CompilationUnit cu) { diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/ExprTranslation.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/ExprTranslation.java index 6d30244e8..c5e3e2e70 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/ExprTranslation.java +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/ExprTranslation.java @@ -521,7 +521,7 @@ public static ImExpr translateIntern(FunctionCall e, ImTranslator t, ImFunction * function supplied by the instance chosen for the substituted type. */ private static ImExpr translateTypeClassDispatch(FunctionCall e, ImTranslator t, ImFunction f) { - WurstTypeTypeParam receiver = (WurstTypeTypeParam) ((HasReceiver) e).getLeft().attrTyp(); + WurstType receiver = ((HasReceiver) e).getLeft().attrTyp(); FunctionDefinition called = e.attrFuncDef(); if (!(called instanceof FuncDef method)) { throw new CompileError(e.attrSource(), @@ -531,7 +531,36 @@ private static ImExpr translateTypeClassDispatch(FunctionCall e, ImTranslator t, for (Expr arg : e.getArgs()) { args.add(arg.imTranslateExpr(t, f)); } - return JassIm.ImTypeVarDispatch(e, t.getTypeClassFunc(method), args, t.getTypeVar(receiver.getDef())); + ImTypeClassFunc requirement = t.getTypeClassFunc(method); + if (receiver instanceof WurstTypeBoundTypeParam bound) { + return translateModuleParamDispatch(e, bound, requirement, args, t); + } + return JassIm.ImTypeVarDispatch(e, requirement, args, + t.getTypeVar(((WurstTypeTypeParam) receiver).getDef())); + } + + /** + * Dispatches a requirement called on a module instantiation's type parameter, which stands for + * the argument the using class supplied rather than for a variable of its own. + *

+ * When that argument is itself a type parameter the dispatch is on the using class's variable, + * exactly as if the call had been written there. Otherwise the instance is already determined: + * a module used with a concrete argument leaves no variable for generic elimination to + * substitute, so the implementation is chosen here. + */ + private static ImExpr translateModuleParamDispatch(FunctionCall e, WurstTypeBoundTypeParam bound, + ImTypeClassFunc requirement, ImExprs args, ImTranslator t) { + WurstType argument = bound.getBaseType().normalize(); + if (argument instanceof WurstTypeTypeParam tp) { + return JassIm.ImTypeVarDispatch(e, requirement, args, t.getTypeVar(tp.getDef())); + } + Either impl = bound.imTypeClassBinding(t).get(requirement); + if (impl == null) { + throw new CompileError(e.attrSource(), + "No type class instance supplies " + e.getFuncName() + " for " + argument + "."); + } + ImFunction target = impl.isRight() ? impl.get() : impl.getLeft().getImplementation(); + return ImFunctionCall(e, target, ImTypeArguments(), args, false, CallType.NORMAL); } private static ImExpr translateFunctionCall(FunctionCall e, ImTranslator t, ImFunction f, boolean returnReveiver, boolean nullSafe) { diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/types/TypeClassConstraints.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/types/TypeClassConstraints.java index acca9f546..6a045050b 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/types/TypeClassConstraints.java +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/types/TypeClassConstraints.java @@ -1,11 +1,14 @@ package de.peeeq.wurstscript.types; import de.peeeq.wurstscript.ast.*; +import de.peeeq.wurstscript.ast.Element; +import de.peeeq.wurstscript.attributes.names.FuncLink; import org.eclipse.jdt.annotation.Nullable; import java.util.ArrayList; import java.util.Collections; import java.util.List; +import java.util.stream.Stream; /** * Reads the type class bounds written on a new-style type parameter. @@ -121,6 +124,81 @@ public static boolean hasBounds(TypeParamDef tp) { return boundExpr.lookupType(simple.getTypeName(), false) instanceof InterfaceDef i ? i : null; } + /** + * Surfaces the methods required by a type parameter's bounds as members of a receiver which + * stands for that parameter, so that {@code T.f(args)} resolves. + *

+ * Bounds are ordered and an earlier one wins, but only over the same signature: two bounds may + * require the very same operation, and offering both would make every call ambiguous. + * Differently shaped overloads are not in competition, so later bounds still contribute them and + * overload resolution picks between them as usual. + * + * @param standsFor what the interface's own type parameter is bound to, which is what the + * requirement's parameter and return types substitute to. + * @param receiver the type the call is written on. + */ + public static void addRequirementMethods(TypeParamDef def, WurstType standsFor, WurstType receiver, + Element node, String name, List result) { + List supplied = new ArrayList<>(); + for (InterfaceDef bound : boundInterfaces(def)) { + for (FuncDef method : bound.getMethods()) { + if (!method.getName().equals(name)) { + continue; + } + FuncLink candidate = requirementLink(bound, method, standsFor, receiver, node); + if (!alreadySupplied(supplied, candidate, node)) { + supplied.add(candidate); + } + } + } + result.addAll(supplied); + } + + /** Every requirement of the bounds, for callers which want the whole set rather than one name. */ + public static Stream requirementMethods(TypeParamDef def, WurstType standsFor, + WurstType receiver, Element node) { + return boundInterfaces(def).stream() + .flatMap(bound -> bound.getMethods().stream() + .map(method -> requirementLink(bound, method, standsFor, receiver, node))); + } + + /** + * Exposes one interface method as a requirement: the interface's own type parameter is + * substituted by what the receiver stands for, so the call reads {@code T.f(args)} with the + * arguments exactly as declared. + */ + private static FuncLink requirementLink(InterfaceDef bound, FuncDef method, WurstType standsFor, + WurstType receiver, Element node) { + TypeParamDef ifaceParam = bound.getTypeParameters().get(0); + VariableBinding binding = VariableBinding.emptyMapping() + .set(ifaceParam, new WurstTypeBoundTypeParam(ifaceParam, standsFor, node)); + return FuncLink.create(method, bound) + .withTypeArgBinding(node, binding) + .withReceiverType(receiver); + } + + /** True when an earlier bound already supplied a requirement of the same shape. */ + private static boolean alreadySupplied(List supplied, FuncLink candidate, Element node) { + for (FuncLink existing : supplied) { + List a = existing.getParameterTypes(); + List b = candidate.getParameterTypes(); + if (a.size() != b.size()) { + continue; + } + boolean same = true; + for (int i = 0; i < a.size(); i++) { + if (!a.get(i).equalsType(b.get(i), node)) { + same = false; + break; + } + } + if (same) { + return true; + } + } + return false; + } + /** * Looks up a method required by any bound of the given type parameter. * Earlier bounds win, matching the left-to-right order the bounds were written in. diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/types/WurstTypeBoundTypeParam.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/types/WurstTypeBoundTypeParam.java index 97e6a4ca7..b0858f80d 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/types/WurstTypeBoundTypeParam.java +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/types/WurstTypeBoundTypeParam.java @@ -27,14 +27,24 @@ public class WurstTypeBoundTypeParam extends WurstType { private final @Nullable Map typeConstraintFunctions; private boolean indexInitialized = false; private final Element context; + /** + * True when this stands for the type parameter itself rather than a value of it, as in the + * receiver of {@code T.toIndex(x)}. Only this form exposes the methods required by the bounds. + */ + private final boolean staticRef; public WurstTypeBoundTypeParam(TypeParamDef def, WurstType baseType, Element context) { + this(def, baseType, context, false); + } + + private WurstTypeBoundTypeParam(TypeParamDef def, WurstType baseType, Element context, boolean staticRef) { if (baseType instanceof WurstTypeIntLiteral) { baseType = WurstTypeInt.instance(); } this.typeParamDef = def; this.baseType = baseType; this.context = context; + this.staticRef = staticRef; if (def.getTypeParamConstraints() instanceof NoTypeParamConstraints) { this.typeConstraintFunctions = null; } else { @@ -42,6 +52,11 @@ public WurstTypeBoundTypeParam(TypeParamDef def, WurstType baseType, Element con } } + /** The same binding, seen as the type parameter itself rather than as a value of it. */ + public WurstTypeBoundTypeParam asStaticRef() { + return staticRef ? this : new WurstTypeBoundTypeParam(typeParamDef, baseType, context, true); + } + @Override VariableBinding matchAgainstSupertypeIntern(WurstType other, @Nullable Element location, VariableBinding mapping, VariablePosition variablePosition) { return baseType.matchAgainstSupertypeIntern(other, location, mapping, NONE); @@ -92,17 +107,27 @@ public boolean allowsDynamicDispatch() { @Override public void addMemberMethods(Element node, String name, List result) { + if (staticRef) { + // The requirements are the parameter's own, so a module cannot reach a bound its + // parameter did not declare; their types come from the argument it is bound to, which + // is what the copied body was rewritten to speak in. + TypeClassConstraints.addRequirementMethods(typeParamDef, baseType, this, node, name, result); + return; + } baseType.addMemberMethods(node, name, result); } @Override public Stream getMemberMethods(Element node) { + if (staticRef) { + return TypeClassConstraints.requirementMethods(typeParamDef, baseType, this, node); + } return baseType.getMemberMethods(node); } @Override public boolean isStaticRef() { - return baseType.isStaticRef(); + return staticRef || baseType.isStaticRef(); } @Override @@ -113,7 +138,10 @@ public boolean isCastableToInt() { @Override public WurstType normalize() { - return baseType.normalize(); + // A static reference denotes the parameter, not a value of what it is bound to. Normalising + // it away would leave the argument type, which knows nothing of the bounds the requirements + // are read from. + return staticRef ? this : baseType.normalize(); } public FuncDef getFromIndex() { @@ -178,7 +206,7 @@ private WurstTypeBoundTypeParam withBaseType(WurstType t) { if (t == baseType) { return this; } - return new WurstTypeBoundTypeParam(typeParamDef, t, context); + return new WurstTypeBoundTypeParam(typeParamDef, t, context, staticRef); } public TypeParamDef getTypeParamDef() { diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/types/WurstTypeTypeParam.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/types/WurstTypeTypeParam.java index 2e6cfc385..090068e2e 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/types/WurstTypeTypeParam.java +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/types/WurstTypeTypeParam.java @@ -1,8 +1,6 @@ package de.peeeq.wurstscript.types; import de.peeeq.wurstscript.ast.Element; -import de.peeeq.wurstscript.ast.FuncDef; -import de.peeeq.wurstscript.ast.InterfaceDef; import de.peeeq.wurstscript.ast.TypeExprList; import de.peeeq.wurstscript.ast.TypeParamDef; import de.peeeq.wurstscript.attributes.names.FuncLink; @@ -13,7 +11,6 @@ import io.vavr.control.Option; import org.eclipse.jdt.annotation.Nullable; -import java.util.ArrayList; import java.util.List; import java.util.stream.Stream; @@ -99,45 +96,8 @@ public void addMemberMethods(Element node, String name, List result) { if (!staticRef) { return; } - // Bounds are ordered and an earlier one wins, but only over the same signature: two bounds - // may require the very same operation, and offering both would make every call ambiguous. - // Differently shaped overloads are not in competition, so later bounds still contribute - // them and overload resolution picks between them as usual. - List supplied = new ArrayList<>(); - for (InterfaceDef bound : TypeClassConstraints.boundInterfaces(def)) { - for (FuncDef method : bound.getMethods()) { - if (!method.getName().equals(name)) { - continue; - } - FuncLink candidate = requirementLink(node, bound, method); - if (!alreadySupplied(supplied, candidate, node)) { - supplied.add(candidate); - } - } - } - result.addAll(supplied); - } - - /** True when an earlier bound already supplied a requirement of the same shape. */ - private static boolean alreadySupplied(List supplied, FuncLink candidate, Element node) { - for (FuncLink existing : supplied) { - List a = existing.getParameterTypes(); - List b = candidate.getParameterTypes(); - if (a.size() != b.size()) { - continue; - } - boolean same = true; - for (int i = 0; i < a.size(); i++) { - if (!a.get(i).equalsType(b.get(i), node)) { - same = false; - break; - } - } - if (same) { - return true; - } - } - return false; + // The parameter stands for itself, so a requirement keeps the shape it was declared with. + TypeClassConstraints.addRequirementMethods(def, new WurstTypeTypeParam(def), this, node, name, result); } @Override @@ -145,23 +105,7 @@ public Stream getMemberMethods(Element node) { if (!staticRef) { return Stream.empty(); } - return TypeClassConstraints.boundInterfaces(def).stream() - .flatMap(bound -> bound.getMethods().stream() - .map(method -> requirementLink(node, bound, method))); - } - - /** - * Exposes one interface method as a requirement of this type parameter: the interface's own - * type parameter is substituted by this one, and the receiver becomes the type parameter, so - * the call reads {@code T.f(args)} with the arguments exactly as declared. - */ - private FuncLink requirementLink(Element node, InterfaceDef bound, FuncDef method) { - TypeParamDef ifaceParam = bound.getTypeParameters().get(0); - VariableBinding binding = VariableBinding.emptyMapping() - .set(ifaceParam, new WurstTypeBoundTypeParam(ifaceParam, new WurstTypeTypeParam(def), node)); - return FuncLink.create(method, bound) - .withTypeArgBinding(node, binding) - .withReceiverType(this); + return TypeClassConstraints.requirementMethods(def, new WurstTypeTypeParam(def), this, node); } @Override diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/validation/WurstValidator.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/validation/WurstValidator.java index 39c0d0e1f..85c9fdf47 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/validation/WurstValidator.java +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/validation/WurstValidator.java @@ -2466,7 +2466,22 @@ public VariableBinding case_ExprNewObject(ExprNewObject e) { @Override public VariableBinding case_ModuleUse(ModuleUse moduleUse) { - return null; + // A module's type parameters may carry bounds, and the use is the only place where + // both the parameter and the argument chosen for it are in view. Pair them by + // position, stopping at the shorter list so a wrong arity is reported once, by the + // expander, rather than again here as an unsatisfied bound. + ModuleDef def = moduleUse.attrModuleDef(); + if (def == null) { + return null; + } + VariableBinding mapping = VariableBinding.emptyMapping(); + int paired = Math.min(def.getTypeParameters().size(), moduleUse.getTypeArgs().size()); + for (int i = 0; i < paired; i++) { + TypeParamDef tp = def.getTypeParameters().get(i); + mapping = mapping.set(tp, new WurstTypeBoundTypeParam(tp, + moduleUse.getTypeArgs().get(i).attrTyp(), moduleUse)); + } + return mapping; } @Override @@ -2643,15 +2658,6 @@ private void checkBoundsSatisfied(Element location, TypeParamDef tp, WurstType t /** Every bound written on a type parameter must be usable as a type class. */ private void checkTypeParamBounds(TypeParamDef tp) { - if (TypeClassConstraints.hasBounds(tp) && tp.attrNearestStructureDef() instanceof ModuleDef) { - // Using a module copies its body into the class, replacing the module's type parameters - // in type positions. A requirement is called on the parameter itself, which is an - // expression, so it survives the copy and no longer resolves. Reject that here rather - // than let it fail later as an unknown name. - tp.addError("Type class bounds are not supported on a module type parameter." - + "\nMove the bounded generic into a class, or use the module without a bound."); - return; - } for (TypeExpr boundExpr : TypeClassConstraints.boundExprs(tp)) { String reason = TypeClassConstraints.invalidBoundReason(boundExpr); if (reason != null) { diff --git a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/TypeClassTests.java b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/TypeClassTests.java index df0b45af4..510382732 100644 --- a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/TypeClassTests.java +++ b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/TypeClassTests.java @@ -455,7 +455,8 @@ public void aBoundedMethodParameterInAGenericClassIsRejectedForLua() { * A module may carry a bounded type parameter, and a class using it supplies the argument. Using * a module copies its body into the class, substituting the module's type parameters — and a * requirement is called on the parameter itself, {@code T.show(x)}, which is a name rather than a - * type, so the substitution never reaches it. + * type, so the substitution never reaches it. The instantiation declares the parameter so that + * name still resolves. */ private static final String[] BOUND_ON_MODULE = { "package test", @@ -480,23 +481,77 @@ public void aBoundedMethodParameterInAGenericClassIsRejectedForLua() { }; /** - * Rejected, and this pins that it is rejected clearly rather than mistranslated. - *

* A module body resolves names in the module's own scope by design — {@code nextScope} sends a * {@code ModuleInstanciation} to {@code attrModuleOrigin()} rather than to the class using it, so * a module cannot capture the names of whoever uses it. The type replacement during expansion * therefore reaches every {@code T} used as a type, and cannot reach the one in {@code T.show(x)} - * which is a name: renaming it to the using class's parameter produces a name that scope + * which is a name: renaming it to the using class's parameter would produce a name that scope * deliberately cannot see. *

- * Making it work means the instantiation declaring the parameter itself, so the body keeps saying - * {@code T} and {@code T} resolves — type parameters on {@code ModuleInstanciation}, which is a - * grammar change. Backlog item 7. + * So the instantiation declares the parameter itself and records the argument. The body keeps + * saying {@code T}, that name resolves, and the requirement it reaches is the one {@code T} + * declared while its types are the argument's. Here the argument is the using class's own + * parameter, so the dispatch is on the class's type variable. + */ + @Test + public void boundOnModuleTypeParameter() { + testAssertOkLines(true, BOUND_ON_MODULE); + } + + @Test + public void boundOnModuleTypeParameterLua() { + test().testLua(true).executeProg().lines(BOUND_ON_MODULE); + } + + /** + * The bound is checked where the argument is chosen. Nothing else sees both: the instantiation + * records the argument already resolved, and by the time the copied body dispatches on it the + * module use is no longer there to blame. + */ + @Test + public void unsatisfiedBoundOnModuleUse() { + testAssertErrorsLines(false, "Type string does not satisfy the bound T: Show", + "package test", + "native testSuccess()", + "interface Show", + " function show(T x) returns int", + "implements Show", + " function show(int x) returns int", + " return x * 2", + "module Shower", + " T held", + " function shown() returns int", + " return T.show(held)", + "class Holder", + " use Shower", + "init", + " testSuccess()" + ); + } + + /** + * A using class which is itself generic can only supply the bound by declaring it, because no + * instance is chosen yet at that point. */ @Test - public void boundOnModuleTypeParameterIsRejected() { - testAssertErrorsLines(false, "Type class bounds are not supported on a module type parameter", - BOUND_ON_MODULE); + public void unsatisfiedBoundOnModuleUseFromClassParameter() { + testAssertErrorsLines(false, "Type parameter K does not satisfy the bound T: Show", + "package test", + "native testSuccess()", + "interface Show", + " function show(T x) returns int", + "implements Show", + " function show(int x) returns int", + " return x * 2", + "module Shower", + " T held", + " function shown() returns int", + " return T.show(held)", + "class Holder", + " use Shower", + "init", + " testSuccess()" + ); } /** Each type argument picks its own instance, so one generic serves several types. */ @@ -1174,28 +1229,39 @@ public void boundForwardedThroughTwoLevels() { ); } - /** A bound on a module type parameter is rejected: using a module copies its body out of scope. */ + /** + * A module used with a concrete argument dispatches to that argument's instance directly: the + * using class is not generic, so there is no type variable left for generic elimination to + * substitute and the instance is already determined when the body is translated. + */ @Test public void boundOnGenericModule() { - testAssertErrorsLines(false, "not supported on a module type parameter", - "package test", - "native testSuccess()", - "interface Show", - " function show(T x) returns string", - "implements Show", - " function show(int x) returns string", - " return \"i\"", - "module M", - " function render(T x) returns string", - " return T.show(x)", - "class C", - " use M", - "init", - " if new C().render(1) == \"i\"", - " testSuccess()" - ); + testAssertOkLines(true, BOUND_ON_GENERIC_MODULE); } + @Test + public void boundOnGenericModuleLua() { + test().testLua(true).executeProg().lines(BOUND_ON_GENERIC_MODULE); + } + + private static final String[] BOUND_ON_GENERIC_MODULE = { + "package test", + "native testSuccess()", + "interface Show", + " function show(T x) returns string", + "implements Show", + " function show(int x) returns string", + " return \"i\"", + "module M", + " function render(T x) returns string", + " return T.show(x)", + "class C", + " use M", + "init", + " if new C().render(1) == \"i\"", + " testSuccess()", + }; + /** A method with its own type parameters must not disturb the class binding taken from the receiver. */ @Test public void classBoundWithIndependentMethodTypeParam() { From 4cf59b64a6c5b0e662525b27402db56539648b4b Mon Sep 17 00:00:00 2001 From: Frotty Date: Mon, 17 Aug 2026 19:46:04 +0200 Subject: [PATCH 07/12] Record what the module bound change means for someone writing Wurst The backlog note for it was a plan; replace it with what happened, including the part the plan had wrong. Excluding the declared parameters from inference meant not making the instantiation a generic element at all, rather than teaching inference to skip them. --- BACKLOG.md | 35 +++++++------------ CHANGELOG.md | 17 +++++++++ .../resources/agent-docs/WURST_LANGUAGE.md | 14 ++++++++ 3 files changed, 43 insertions(+), 23 deletions(-) diff --git a/BACKLOG.md b/BACKLOG.md index 371b6d7e7..b227b9f46 100644 --- a/BACKLOG.md +++ b/BACKLOG.md @@ -64,29 +64,6 @@ itself, and one gap in what the suite can see. 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 `de.peeeq.wurstscript/src/main/resources/agent-docs/WURST_LANGUAGE.md`, and it already documents @@ -247,6 +224,18 @@ itself, and one gap in what the suite can see. ## Done +- 7. A module's type parameter can carry a bound. The instantiation declares the module's parameters + and records the arguments, so the receiver in `T.show(x)` has a name to resolve and something to + say what it stands for. Excluding them from inference turned out to mean not making a + `ModuleInstanciation` an `AstElementWithTypeParameters` at all: as one, every method of a generic + module's instantiation asked its caller to infer a parameter its signature never mentions, which is + what `genericModuleInGenericClassGet` was reporting. The parameters are registered as type names in + `TypeNameLinks` instead. The receiver denotes the argument bound to the parameter, offering the + requirements the parameter declared with the argument's types, and dispatch follows the argument — + the using class's type variable when it is one, the instance directly when it is concrete, since a + module used with a concrete argument leaves no variable to substitute. The bound is checked at the + use, the only place which sees the parameter and the argument together. + - 21. A standard library program executes on Lua (#1242). Three packages could not initialise, each on one native the shim did not define — `StringHash` for Colors, `Location` for Vectors, `TimerStart` for GameTimer — and `StringCase` made a fourth once the program itself ran. The diff --git a/CHANGELOG.md b/CHANGELOG.md index ededc734f..92d2ba6a8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -41,6 +41,23 @@ Lua reaches such a class through the interface it implements, so no call names the instantiation and the construction is what the specialisation is taken from. +- A module's type parameter may now carry a type class bound, and the class using the module supplies the + argument: + + module Shower + T held + function shown() returns string + return T.show(held) + + class Holder + use Shower + + Using a module copies its body into the class and replaces the module's type parameters wherever they + are used as types. The receiver in `T.show(held)` is a name rather than a type, so the replacement never + reached it and the bound was rejected. The instantiation now declares the parameters and records the + arguments chosen for them, so that name resolves and says what it stands for. The argument must satisfy + the bound, which is reported at the `use`. This works on both targets. + - Added new pseudo-natives for debugging memory leaks: // returns the maximum type id, can be usd to diff --git a/de.peeeq.wurstscript/src/main/resources/agent-docs/WURST_LANGUAGE.md b/de.peeeq.wurstscript/src/main/resources/agent-docs/WURST_LANGUAGE.md index 84375eeec..16554e0bd 100644 --- a/de.peeeq.wurstscript/src/main/resources/agent-docs/WURST_LANGUAGE.md +++ b/de.peeeq.wurstscript/src/main/resources/agent-docs/WURST_LANGUAGE.md @@ -204,6 +204,20 @@ function outer(R x) returns string // alone would not compile return inner(x) ``` +A module's type parameter may carry a bound, and the class using the module supplies the argument: + +```wurst +module Shower + T held + function shown() returns string + return T.show(held) + +class Holder + use Shower // K must declare the bound it is asked to supply + construct(K k) + held = k +``` + A class with a bounded type parameter can be subclassed, and the subclass may reach the superclass through `super`: ```wurst From b5bcf81b1fba00aa43d0fc7545ed6639092f6d5d Mon Sep 17 00:00:00 2001 From: Frotty Date: Mon, 17 Aug 2026 20:13:35 +0200 Subject: [PATCH 08/12] Thread the concrete type to the paths that need it on Lua A generic object stays erased on this target and only the paths needing a concrete type are specialised. Three places were still asking the object for what the path should have carried. A method call carries the class's type arguments followed by the method's own, and the test for whether the class's were still missing asked whether the call had any at all. A method declaring parameters of its own therefore looked as though it already had both, and its specialisation was matched against a list one longer than the call supplied - which is why a bounded type parameter on a method of a generic class was rejected outright here while the same program runs on Jass. A specialised method was left on the specialised class. That is where the object comes from when its construction was redirected there, and otherwise the object is allocated from the class the method was declared on, so the slot a virtual call named resolved to nothing and failed at runtime rather than at compile time. Move it to whichever class the program actually allocates; the specialised name carries the instantiation, so two specialisations stay distinct on the erased class. Read from what is allocated rather than from the shape of the class, because one class is reached both ways. A specialisation left with no methods drops out entirely, so the second class shape is gone wherever an ordinary generic object is involved. A super call names its target, so it has no receiver to read type arguments from, and this target never lifts the class's type variables onto the function - it reached the erased original, whose dispatch had already been neutralised as dead code. The receiver is still the first argument, and the class it is used as says which instantiation the subclass extends, which is the answer the lift gives on the other target. Dispatch from inside a constructor is still rejected here. All three of these read the instantiation off a receiver, and a constructor call has none: the only thing stating it is the type of what the result is assigned to. --- BACKLOG.md | 54 +++++-- CHANGELOG.md | 8 + .../imtranslation/EliminateGenerics.java | 152 +++++++++++++++++- .../resources/agent-docs/WURST_LANGUAGE.md | 4 +- .../wurstscript/tests/FastHashMapTests.java | 41 +++-- .../wurstscript/tests/TypeClassTests.java | 34 ++-- 6 files changed, 243 insertions(+), 50 deletions(-) diff --git a/BACKLOG.md b/BACKLOG.md index b227b9f46..97a285297 100644 --- a/BACKLOG.md +++ b/BACKLOG.md @@ -31,6 +31,12 @@ itself, and one gap in what the suite can see. a repro for both targets, `TypeClassTests.dispatchInsideConstructor` and `dispatchInsideConstructorIsRejectedForLua`, the second pinning the current diagnostic. + Still open after item 23's first half landed. The two mechanisms that closed items 13 and 25 both + read the instantiation off a receiver — the first argument of a call which names its target, or the + receiver of a method call. A constructor has neither: `new_Box(21)` takes no receiver, and the only + thing stating `Box` is the type of the variable it is assigned to. So this still needs the + type-driven collector below, which is a second mechanism rather than a widening of the one there is. + Not the same gap as item 5, and the fix from it does not reach: a constructor belongs to the 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 @@ -62,7 +68,22 @@ itself, and one gap in what the suite can see. 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. + **Half done.** Items 13 and 25 are closed, and the two things that closed them are the model + working: a specialised method is moved to the class its objects are actually allocated from, and a + call which names its target reads the instantiation off its receiver argument rather than off the + object's class. A specialisation nothing allocates is now left with no methods and drops out + entirely, so the second class shape is gone wherever an ordinary generic object is involved - + `FastHashMapTests.assertSpecialisedClassesAllocateTheirFields` states both accepted outcomes rather + than the one that used to hold. + + What is left is item 6, and it is the part this decision said would be the work: reading the + instantiation from a *type* rather than from a receiver. The classes which still allocate a + specialised copy are the ones whose construction was itself specialised, which is the same + question. Deciding it from what the program allocates, as the pass above does, is a fact rather + than a guess - but it is decided after the fact, and the model would rather no specialised class + were ever allocated. + + Not blocking the container, which works on both targets today. 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 @@ -150,22 +171,6 @@ itself, and one gap in what the suite can see. far produced nothing to work from, which is why it cost a re-run and no diagnosis. The next one will say what differed. -25. **A bounded type parameter on a method of a generic class is rejected on Lua.** - `class Holder` with `function convert(Q other)` fails there with "Generics should - match class method type variables", while the same program compiles and runs on the other target. - On master as well, so it is not a regression — found while trying to give item 10's fix Lua - coverage, which is what it blocks: the only shape reaching that lookup with two parameters at once - is a class parameter beside a method parameter, and Lua will not compile it. - - Pinned by `TypeClassTests.aBoundedMethodParameterInAGenericClassIsRejectedForLua`. A version with - the second parameter on a free function does compile on Lua and passes with or without item 10's - change, so it covers nothing; that is why the rejection is pinned instead. - - Where to start: the message comes from the arity check between a call's generics and the callee's - type variables. A method of a generic class has the class's variables lifted onto it on the Jass - 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. **Done. A dispatch slot's segment is recorded where it is assigned, not recovered from a name.** The segment used to be found by cutting a method's name at its last underscore, which is the right answer only when the rest contains no underscore and the method is not a specialised copy. Four bugs @@ -224,6 +229,21 @@ itself, and one gap in what the suite can see. ## Done +- 25. A bounded type parameter on a method of a generic class compiles and runs on Lua. A method call + there carries the class's type arguments followed by the method's own, and the check for whether the + class's were still missing asked whether the call had *any* — so a method declaring parameters of its + own was read as already having both and its specialisation was matched against a list one longer than + what the call supplied. `aBoundedMethodParameterInAGenericClassLua` runs the program now instead of + pinning the rejection. + +- 13 (Lua half). A subclass of a bounded generic class works on Lua. Two things were wrong and each hid + the other. The specialised method was left on the specialised class while the object is allocated + from the erased one, so the slot a virtual call named resolved to nothing; it is moved to whichever + class the program actually allocates. And `super.m()` names its target, so it reached the erased + original whose dispatch had been neutralised as dead — `nil + extra` at runtime. It now takes the + instantiation from the class its first argument is used as, which is the same answer the Jass path + gets from the lift it does not do here. + - 7. A module's type parameter can carry a bound. The instantiation declares the module's parameters and records the arguments, so the receiver in `T.show(x)` has a name to resolve and something to say what it stands for. Excluding them from inference turned out to mean not making a diff --git a/CHANGELOG.md b/CHANGELOG.md index 92d2ba6a8..75a54e31c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -58,6 +58,14 @@ arguments chosen for them, so that name resolves and says what it stands for. The argument must satisfy the bound, which is reported at the `use`. This works on both targets. +- On the Lua target, a bounded generic class can now be subclassed and a method of one may declare + bounded type parameters of its own. A generic object stays erased there and only the paths needing a + concrete type are specialised, so the concrete type has to reach those paths rather than the object: a + specialised method is bound to the class its objects are allocated from, and a call which names its + target — `super.m()` is one — takes the instantiation from the class its receiver is used as. A + specialisation nothing allocates is no longer emitted at all. A requirement dispatched from inside a + constructor is still rejected on Lua, where a constructor call has no receiver to read. + - Added new pseudo-natives for debugging memory leaks: // returns the maximum type id, can be usd to diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/EliminateGenerics.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/EliminateGenerics.java index 1e0bbdc0e..3a4dfc06e 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/EliminateGenerics.java +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/EliminateGenerics.java @@ -131,9 +131,58 @@ public void transformGenericNewOnly() { } eliminateRemainingGenericNewCalls(); assertNoReachableGenericNewMarkers(); + bindSpecialisedMethodsToTheAllocatedClass(); settleRemainingDispatches(); } + /** + * Moves a specialisation's methods to the class its objects are actually allocated from. + *

+ * Specialising a method leaves the copy on the specialised class. That is where the object comes + * from when a construction on this path was redirected there, and a virtual call finds the slot + * through the object as usual. Otherwise the object stays erased — which is this target's normal + * representation — and is allocated from the class the method was declared on, so the slot the + * call names resolves to nothing and the call fails at runtime rather than at compile time. + *

+ * Decided from what the program allocates rather than from the shape of the class, because one + * class can be reached both ways: a container whose constructor was specialised is allocated from + * the copy, while an ordinary generic object beside it is not. A specialised name carries its + * instantiation, so two specialisations of one method stay distinct on the erased class. + */ + private void bindSpecialisedMethodsToTheAllocatedClass() { + Map erasedOf = new IdentityHashMap<>(); + for (Table.Cell cell : specializedClasses.cellSet()) { + erasedOf.put(cell.getValue(), cell.getRowKey()); + } + Set allocated = allocatedClasses(); + // Walked in program order rather than over the specialisation table, whose iteration is + // hash-ordered: what ends up on a class, and in which order, decides its emitted slot names. + for (ImClass specialized : new ArrayList<>(prog.getClasses())) { + ImClass erased = erasedOf.get(specialized); + if (erased == null || erased == specialized + || allocated.contains(specialized) || !allocated.contains(erased)) { + continue; + } + for (ImMethod method : specialized.getMethods().removeAll()) { + method.setMethodClass(JassIm.ImClassType(erased, JassIm.ImTypeArguments())); + erased.getMethods().add(method); + } + } + } + + /** Every class the program allocates an instance of. */ + private Set allocatedClasses() { + Set result = Collections.newSetFromMap(new IdentityHashMap<>()); + prog.accept(new Element.DefaultVisitor() { + @Override + public void visit(ImAlloc alloc) { + super.visit(alloc); + result.add(alloc.getClazz().getClassDef()); + } + }); + return result; + } + /** * Neutralises the dispatches left in functions that were specialized. *

@@ -170,6 +219,7 @@ private static ImExpr defaultValueFor(ImType type) { private void collectGenericNewRoots() { + methodsByImplementation = null; prog.accept(new Element.DefaultVisitor() { @Override public void visit(ImFunction function) { @@ -249,9 +299,70 @@ && functionNeedsSpecialization(call.getFunc(), Collections.newSetFromMap(new Ide if (!typeArgumentsContainTypeVariable(call.getTypeArguments())) { genericsUses.add(new GenericImFunctionCall(call)); } + return; + } + if (call.getTypeArguments().isEmpty()) { + collectSuperCallToGenericMethod(call); + } + } + + /** + * Collects a call which names the implementation of a generic class's method outright, which is + * what {@code super.m()} becomes. + *

+ * Such a call has no receiver to read type arguments from, and this target never lifts the class's + * type variables onto the function, so there is nothing on the call to specialise against and the + * erased original is reached instead — where the dispatch it contains is dead. The receiver is + * still the first argument, and the class it is used as says which instantiation the caller + * extends, so the type the body needs is threaded to the call rather than taken from the object. + */ + private void collectSuperCallToGenericMethod(ImFunctionCall call) { + if (call.getArguments().isEmpty()) { + return; + } + ImMethod method = methodImplementedBy(call.getFunc()); + if (method == null || !call.getFunc().getTypeVariables().isEmpty()) { + return; + } + ImClass owningClass = method.getMethodClass().getClassDef(); + if (owningClass.getTypeVariables().isEmpty() + || !(call.getArguments().get(0).attrTyp() instanceof ImClassType receiverType)) { + return; + } + ImClassType classType = adaptToSuperclass(receiverType, owningClass); + if (classType == null + || classType.getTypeArguments().size() != owningClass.getTypeVariables().size() + || typeArgumentsContainTypeVariable(classType.getTypeArguments())) { + return; + } + genericsUses.add(new GenericSuperCall(call, method, new GenericTypes(classType.getTypeArguments()))); + } + + /** + * The method a function is the implementation of, or null when it is not one. + *

+ * Rebuilt per collection pass rather than kept, because specialising a method adds another. This + * target leaves methods on their classes, so there is no owner map of the kind moving them out + * builds. + */ + private @Nullable ImMethod methodImplementedBy(ImFunction function) { + if (methodsByImplementation == null) { + methodsByImplementation = new IdentityHashMap<>(); + prog.accept(new Element.DefaultVisitor() { + @Override + public void visit(ImMethod method) { + super.visit(method); + if (method.getImplementation() != null) { + methodsByImplementation.putIfAbsent(method.getImplementation(), method); + } + } + }); } + return methodsByImplementation.get(function); } + private @Nullable Map methodsByImplementation; + /** * A construction states an instantiation that no call site has to mention. A closure is the case * that needs it: its class is built from the enclosing type variables and reached through its @@ -362,7 +473,7 @@ private void collectGenericNewUse(ImMethodCall call) { Collections.newSetFromMap(new IdentityHashMap<>()))) { return; } - if (call.getTypeArguments().isEmpty()) { + if (isMissingClassTypeArguments(call, method)) { addMemberTypeArguments(call, method.attrClass()); } if (typeArgumentsContainTypeVariable(call.getTypeArguments())) { @@ -377,6 +488,22 @@ private void collectGenericNewUse(ImMethodCall call) { } } + /** + * Whether a call is short of the type arguments belonging to the receiver's class. + *

+ * A specialisation is matched against the class's type variables followed by the method's own, so + * a method declaring parameters of its own leaves the call supplying the shorter list rather than + * an empty one. Reading a non-empty list as "already has them" is what rejected a bounded type + * parameter on a method of a generic class here, while the same program compiles on Jass, where + * lifting the class's variables onto the method gives the call both at once. + */ + private static boolean isMissingClassTypeArguments(ImMethodCall call, ImMethod method) { + ImFunction implementation = method.getImplementation(); + int own = implementation == null ? 0 : implementation.getTypeVariables().size(); + return call.getTypeArguments().size() == own + && !method.getMethodClass().getClassDef().getTypeVariables().isEmpty(); + } + /** * Replaces a member call's still-generic type arguments with those of the constructor call that * produced its receiver, as in {@code new Box().render(x)}. @@ -2093,6 +2220,29 @@ public void eliminate() { } } + /** + * A {@code super} call reaching a generic class's method, rewritten to the copy specialised for + * the instantiation the caller extends. The instantiation came from the receiver's class rather + * than from the call, so there are no type arguments on the call to clear. + */ + class GenericSuperCall implements GenericUse { + private final ImFunctionCall call; + private final ImMethod method; + private final GenericTypes generics; + + GenericSuperCall(ImFunctionCall call, ImMethod method, GenericTypes generics) { + this.call = call; + this.method = method; + this.generics = generics; + } + + @Override + public void eliminate() { + call.setFunc(specializeMethodImplementation(method, generics)); + specializedCallSites.add(call); + } + } + class GenericNewCall implements GenericUse { private final ImFunctionCall call; diff --git a/de.peeeq.wurstscript/src/main/resources/agent-docs/WURST_LANGUAGE.md b/de.peeeq.wurstscript/src/main/resources/agent-docs/WURST_LANGUAGE.md index 16554e0bd..4dbb1b683 100644 --- a/de.peeeq.wurstscript/src/main/resources/agent-docs/WURST_LANGUAGE.md +++ b/de.peeeq.wurstscript/src/main/resources/agent-docs/WURST_LANGUAGE.md @@ -229,7 +229,9 @@ class SubBox extends Box return super.size(extra) + 100 ``` -On Lua this compiles and runs but the override does not reach the superclass implementation: the object is allocated from the erased class while the method belongs to the specialised one. Use it on Jass only for now. +This works on both targets. A method of such a class may also declare bounded type parameters of its own, on both targets. + +One gap is left on Lua: a requirement dispatched from inside the constructor of a bounded generic class is rejected there, because a constructor call has no receiver to take the instantiation from. Dispatch from a method, a closure, or a `super` call is supported. ## Strings diff --git a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/FastHashMapTests.java b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/FastHashMapTests.java index 074d47b85..1a574b66e 100644 --- a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/FastHashMapTests.java +++ b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/FastHashMapTests.java @@ -2,6 +2,7 @@ import com.google.common.base.Charsets; import com.google.common.io.Files; +import org.eclipse.jdt.annotation.Nullable; import org.testng.annotations.Test; import java.io.File; @@ -421,15 +422,30 @@ public void fastHashMapAgainstTheStandardLibraryLua() throws IOException { } /** - * A specialised class allocates the same fields as the class it was specialised from. Nothing - * refers to the copies it holds - an access made before specialisation still names the original's - * variable - so a pass which drops unread fields drops all of them, and an instance allocated - * from the specialised class comes out with no fields at all while the emitted code goes on - * reading them by name. + * The erased class and any specialised copy of it agree on the fields they allocate. + *

+ * Nothing refers to the copies a specialised class holds - an access made before specialisation + * still names the original's variable - so a pass which drops unread fields drops all of them, + * and an instance allocated from the specialised class comes out with no fields at all while the + * emitted code goes on reading them by name. + *

+ * A specialisation whose methods are bound to the erased class the objects come from has nothing + * left to allocate and is not emitted, which is the shape this target aims for and leaves no field + * set to disagree. One emitted without being allocated is neither: dead weight, and the two class + * shapes coexisting is what produced the bug above. Both accepted states are named, so this cannot + * pass by finding nothing. */ private static void assertSpecialisedClassesAllocateTheirFields(String compiled) { String erasedFields = allocatedFields(compiled, "FastHashMap"); - String specialisedFields = allocatedFields(compiled, "FastHashMap_specialized\\w*"); + String specialisedFields = allocatedFieldsOrNull(compiled, "FastHashMap_specialized\\w*"); + if (specialisedFields == null) { + if (compiled.contains("FastHashMap_specialized")) { + throw new AssertionError("a specialised class is emitted but never allocated;" + + " its methods should be bound to the class the objects come from, leaving nothing" + + " of it behind, in:\n" + compiled); + } + return; + } if (!erasedFields.equals(specialisedFields)) { throw new AssertionError("the specialised class should allocate the same fields as the erased one." + "\n erased: " + erasedFields @@ -438,12 +454,17 @@ private static void assertSpecialisedClassesAllocateTheirFields(String compiled) } private static String allocatedFields(String compiled, String classPattern) { - Matcher m = Pattern.compile("function " + classPattern + ":create\\d*\\(\\)\\s*\\R" - + "\\s*local new_inst = \\(\\{([^}]*)\\}\\)").matcher(compiled); - if (!m.find()) { + String fields = allocatedFieldsOrNull(compiled, classPattern); + if (fields == null) { throw new AssertionError("expected an allocation for " + classPattern + " in:\n" + compiled); } - return m.group(1).trim(); + return fields; + } + + private static @Nullable String allocatedFieldsOrNull(String compiled, String classPattern) { + Matcher m = Pattern.compile("function " + classPattern + ":create\\d*\\(\\)\\s*\\R" + + "\\s*local new_inst = \\(\\{([^}]*)\\}\\)").matcher(compiled); + return m.find() ? m.group(1).trim() : null; } /** diff --git a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/TypeClassTests.java b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/TypeClassTests.java index 510382732..6ee8ade16 100644 --- a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/TypeClassTests.java +++ b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/TypeClassTests.java @@ -342,16 +342,13 @@ public void subclassOfBoundedGeneric() { } /** - * The same program on Lua, where it still does not work, so the difference between the targets is - * stated rather than left to be discovered. {@code transformGenericNewOnly} runs neither - * {@code simplifyClasses} nor {@code addMemberTypeArguments}, so the class's type variables are - * never lifted onto its functions and there is nothing for a super call to carry. The object is - * allocated from the erased {@code Box} table while the specialised one holds the method, so it - * compiles and runs and never reaches {@code testSuccess}. Tracked as backlog item 13, whose - * remaining half is the erasure decision in item 23. + * The same program on Lua, where the object stays erased. Both halves of that had to be met: the + * specialised method is bound to the class the object is allocated from, so a virtual call finds + * it, and the super call — which names its target and so has no receiver to read type arguments + * from — is rewritten to the copy specialised for the instantiation the subclass extends. */ - @Test(expectedExceptions = Error.class, expectedExceptionsMessageRegExp = ".*Succeed function not called.*") - public void subclassOfBoundedGenericIsStillBrokenOnLua() { + @Test + public void subclassOfBoundedGenericLua() { test().testLua(true).executeProg().lines(SUBCLASS_OF_BOUNDED_GENERIC); } @@ -434,21 +431,16 @@ public void aBoundedMethodParameterMayShareTheClassParameterName() { } /** - * The same program on Lua, where it does not compile at all — and not because of the collision. - * A bounded type parameter on a method of a generic class is rejected on that target with - * "Generics should match class method type variables", on master as well as here, so the shape - * which exercises this lookup cannot be run there to check the dispatch. + * The same program on Lua, which used to be rejected outright with "Generics should match class + * method type variables" — a method call there carries the class's type arguments followed by the + * method's own, and a method declaring parameters of its own was read as already having both. *

- * A version Lua does compile, with the second parameter on a free function rather than a method, - * passes without the change as well as with it: the two parameters never reach one lookup that - * way, so it would be coverage in name only. This pins the rejection instead, and the gap behind - * it is backlog item 25. Should that be fixed, this test fails and gains a dispatch assertion. + * Running it is the point: both parameters reach one lookup only in this shape, so a version with + * the second parameter on a free function would be coverage in name only. */ @Test - public void aBoundedMethodParameterInAGenericClassIsRejectedForLua() { - test().testLua(true).executeProg() - .expectError("Generics should match class method type variables") - .lines(SAME_NAMED_BOUNDED_PARAMETERS); + public void aBoundedMethodParameterInAGenericClassLua() { + test().testLua(true).executeProg().lines(SAME_NAMED_BOUNDED_PARAMETERS); } /** From b78e302ae10288dad6c5591b4ce8810964acaff4 Mon Sep 17 00:00:00 2001 From: Frotty Date: Mon, 17 Aug 2026 21:15:49 +0200 Subject: [PATCH 09/12] Give both class shapes the slot when both are allocated The pass moved a specialisation's methods to the erased class only when the specialised one was not allocated, which read the two as alternatives. They are not: a call is rewritten to the specialised method whichever class its receiver came from, so when both shapes are allocated, whichever lost the method is left without the slot the call names. The erased class now gets a binding of its own instead, and the specialised class keeps what it has. Being allocated is what settles where a slot is needed. Only the closure case reaches this today, where the erased class already carries a binding under the same name and the emitted tables come out unchanged, so this is the rule stated rather than a behaviour change I can show failing. The new test covers the other half of the same question: three receivers of one class - built by a specialised generic function, constructed directly, and a subclass instance - all reaching one specialised slot made virtual by the override. A specialised function copy allocates the erased class too, so the move has to serve every way of arriving at the slot rather than only the construction written in place. --- .../imtranslation/EliminateGenerics.java | 21 +++++++++- .../wurstscript/tests/TypeClassTests.java | 39 +++++++++++++++++++ 2 files changed, 58 insertions(+), 2 deletions(-) diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/EliminateGenerics.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/EliminateGenerics.java index 3a4dfc06e..abdefc6a7 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/EliminateGenerics.java +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/EliminateGenerics.java @@ -148,6 +148,11 @@ public void transformGenericNewOnly() { * class can be reached both ways: a container whose constructor was specialised is allocated from * the copy, while an ordinary generic object beside it is not. A specialised name carries its * instantiation, so two specialisations of one method stay distinct on the erased class. + *

+ * When both are allocated neither can give up the slot — a call was rewritten to the specialised + * method whichever class its receiver came from — so the erased class gets a binding of its own + * rather than the method moving. Being allocated is what settles this and not which shape looks + * primary; treating the two as alternatives leaves whichever lost without the slot. */ private void bindSpecialisedMethodsToTheAllocatedClass() { Map erasedOf = new IdentityHashMap<>(); @@ -159,8 +164,20 @@ private void bindSpecialisedMethodsToTheAllocatedClass() { // hash-ordered: what ends up on a class, and in which order, decides its emitted slot names. for (ImClass specialized : new ArrayList<>(prog.getClasses())) { ImClass erased = erasedOf.get(specialized); - if (erased == null || erased == specialized - || allocated.contains(specialized) || !allocated.contains(erased)) { + if (erased == null || erased == specialized || !allocated.contains(erased)) { + continue; + } + if (allocated.contains(specialized)) { + // Both shapes are allocated, so neither can give up the slot: a call was rewritten to + // the specialised method whichever class its receiver came from. The specialised class + // keeps its methods and the erased one gets a binding of its own to the same + // implementation. + for (ImMethod method : new ArrayList<>(specialized.getMethods())) { + ImMethod onErased = method.copyWithRefs(); + onErased.setMethodClass(JassIm.ImClassType(erased, JassIm.ImTypeArguments())); + onErased.setImplementation(method.getImplementation()); + erased.getMethods().add(onErased); + } continue; } for (ImMethod method : specialized.getMethods().removeAll()) { diff --git a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/TypeClassTests.java b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/TypeClassTests.java index 6ee8ade16..7ba0a4bdc 100644 --- a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/TypeClassTests.java +++ b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/TypeClassTests.java @@ -352,6 +352,45 @@ public void subclassOfBoundedGenericLua() { test().testLua(true).executeProg().lines(SUBCLASS_OF_BOUNDED_GENERIC); } + /** + * Three receivers of one class reaching the same specialised slot, which the subclass makes + * virtual: one built by a specialised generic function, one constructed directly, one a subclass + * instance. The first two stay erased — a specialised function's copy allocates the erased class + * as well — so this says that moving the slot to the class objects come from serves every way of + * arriving at it, not only the construction written in place. + */ + @Test + public void oneGenericReachedThroughEveryConstructionLua() { + test().testLua(true).executeProg().lines( + "package test", + "native testSuccess()", + "interface Show", + " function show(T x) returns int", + "implements Show", + " function show(int x) returns int", + " return x", + "class Box", + " K key", + " construct(K k)", + " key = k", + " function size(int extra) returns int", + " return K.show(key) + extra", + "class SubBox extends Box", + " construct(int k)", + " super(k)", + " override function size(int extra) returns int", + " return super.size(extra) + 100", + "function make(K k) returns Box", + " return new Box(k)", + "init", + " Box made = make(5)", + " Box direct = new Box(7)", + " Box sub = new SubBox(5)", + " if made.size(1) == 6 and direct.size(1) == 8 and sub.size(1) == 106", + " testSuccess()" + ); + } + /** * A method may have type parameters of its own on top of the class's. The call already carries an * argument for its own, so what it is short of is the class's prefix rather than everything, and From ad4509a61a6fb97f4a3fb4382c23fab826569347 Mon Sep 17 00:00:00 2001 From: Frotty Date: Mon, 17 Aug 2026 21:31:08 +0200 Subject: [PATCH 10/12] Stop promising the Lua shape the arity fix only stopped rejecting The language doc said a method of a bounded generic class may declare bounded type parameters of its own on both targets. AGENTS.md says not to promise exactly that: a method combining its own type parameters with its owning generic class's is outside the Lua contract, and such a loader belongs on a free generic function or on a method parameterised only by its owning class. What the fix did was narrower than what was written. It removed an arity check which counted the class's type arguments against a call that had supplied only the method's, so one program stopped being rejected and now runs. One running program is one call site, not a guarantee about dispatch and loader paths that shape can reach. The doc names it as unsupported instead, beside the constructor gap. The changelog says the rejection is gone without reading it as support, and the test says the same so the next reader does not widen it again. --- BACKLOG.md | 17 +++++++++++------ CHANGELOG.md | 18 +++++++++++------- .../resources/agent-docs/WURST_LANGUAGE.md | 4 ++-- .../wurstscript/tests/TypeClassTests.java | 4 ++++ 4 files changed, 28 insertions(+), 15 deletions(-) diff --git a/BACKLOG.md b/BACKLOG.md index 97a285297..2dc027654 100644 --- a/BACKLOG.md +++ b/BACKLOG.md @@ -229,12 +229,17 @@ itself, and one gap in what the suite can see. ## Done -- 25. A bounded type parameter on a method of a generic class compiles and runs on Lua. A method call - there carries the class's type arguments followed by the method's own, and the check for whether the - class's were still missing asked whether the call had *any* — so a method declaring parameters of its - own was read as already having both and its specialisation was matched against a list one longer than - what the call supplied. `aBoundedMethodParameterInAGenericClassLua` runs the program now instead of - pinning the rejection. +- 25. A bounded type parameter on a method of a generic class no longer trips the arity check on Lua. A + method call there carries the class's type arguments followed by the method's own, and the check for + whether the class's were still missing asked whether the call had *any* — so a method declaring + parameters of its own was read as already having both and its specialisation was matched against a + list one longer than what the call supplied. `aBoundedMethodParameterInAGenericClassLua` runs the + program now instead of pinning the rejection. + + Removing that rejection is not the same as supporting the shape, and `AGENTS.md` says not to promise + it: a method combining its own type parameters with its owning generic class's stays outside the Lua + contract, one running program being one call site rather than a guarantee. The docs say so rather than + reading the fix as general support. - 13 (Lua half). A subclass of a bounded generic class works on Lua. Two things were wrong and each hid the other. The specialised method was left on the specialised class while the object is allocated diff --git a/CHANGELOG.md b/CHANGELOG.md index 75a54e31c..f9f1551e0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -58,13 +58,17 @@ arguments chosen for them, so that name resolves and says what it stands for. The argument must satisfy the bound, which is reported at the `use`. This works on both targets. -- On the Lua target, a bounded generic class can now be subclassed and a method of one may declare - bounded type parameters of its own. A generic object stays erased there and only the paths needing a - concrete type are specialised, so the concrete type has to reach those paths rather than the object: a - specialised method is bound to the class its objects are allocated from, and a call which names its - target — `super.m()` is one — takes the instantiation from the class its receiver is used as. A - specialisation nothing allocates is no longer emitted at all. A requirement dispatched from inside a - constructor is still rejected on Lua, where a constructor call has no receiver to read. +- On the Lua target, a bounded generic class can now be subclassed. A generic object stays erased there + and only the paths needing a concrete type are specialised, so the concrete type has to reach those + paths rather than the object: a specialised method is bound to the class its objects are allocated + from, and a call which names its target — `super.m()` is one — takes the instantiation from the class + its receiver is used as. A specialisation nothing allocates is no longer emitted at all. + + Two shapes remain unsupported on Lua. A requirement dispatched from inside a constructor is still + rejected, a constructor call having no receiver to read the instantiation from. And a method + combining its own type parameters with those of the generic class owning it is still not a supported + shape, though it is no longer rejected outright: the arity check it tripped over counted the class's + type arguments against a call that had only supplied the method's. - Added new pseudo-natives for debugging memory leaks: diff --git a/de.peeeq.wurstscript/src/main/resources/agent-docs/WURST_LANGUAGE.md b/de.peeeq.wurstscript/src/main/resources/agent-docs/WURST_LANGUAGE.md index 4dbb1b683..5dda397ce 100644 --- a/de.peeeq.wurstscript/src/main/resources/agent-docs/WURST_LANGUAGE.md +++ b/de.peeeq.wurstscript/src/main/resources/agent-docs/WURST_LANGUAGE.md @@ -229,9 +229,9 @@ class SubBox extends Box return super.size(extra) + 100 ``` -This works on both targets. A method of such a class may also declare bounded type parameters of its own, on both targets. +This works on both targets. -One gap is left on Lua: a requirement dispatched from inside the constructor of a bounded generic class is rejected there, because a constructor call has no receiver to take the instantiation from. Dispatch from a method, a closure, or a `super` call is supported. +Two things are not guaranteed on Lua. A requirement dispatched from inside the constructor of a bounded generic class is rejected there, because a constructor call has no receiver to take the instantiation from; dispatch from a method, a closure, or a `super` call is supported. And a method which combines its own type parameters with those of the generic class owning it is not a supported shape on that target — write it as a free generic function, or parameterise the method only by its owning class. ## Strings diff --git a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/TypeClassTests.java b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/TypeClassTests.java index 7ba0a4bdc..86a874cc1 100644 --- a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/TypeClassTests.java +++ b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/TypeClassTests.java @@ -476,6 +476,10 @@ public void aBoundedMethodParameterMayShareTheClassParameterName() { *

* Running it is the point: both parameters reach one lookup only in this shape, so a version with * the second parameter on a free function would be coverage in name only. + *

+ * One program running is not the shape being supported. A method combining its own type parameters + * with its owning generic class's stays outside the Lua contract - see {@code AGENTS.md} - so this + * says the arity check no longer rejects it, and nothing wider. */ @Test public void aBoundedMethodParameterInAGenericClassLua() { From feb71bec9a725054e7ad5ea3e423e540cdca849c Mon Sep 17 00:00:00 2001 From: Frotty Date: Mon, 17 Aug 2026 21:47:35 +0200 Subject: [PATCH 11/12] Specialise a generic class's own functions against its type variables A constructor belongs to its class rather than to a generic function of its own, so it declares no type variables: it uses the class's, which this target does not lift onto it. Specialising it was therefore read as nothing to do, and the call site had its type argument stripped and marked done, leaving the dispatch inside the constructor with no concrete type and the backend reporting a bound it could not resolve. The argument was on the call the whole time. Three notes on this had reasoned the other way - that a constructor call carries nothing and the instantiation is only on the type of what the result is assigned to, so closing it needed a collector reading types rather than receivers. It does not. Matching such a function against its class's type variables is the same rule already used for a method implementation, and it reaches the constructor body and the field initialiser it calls as well. Method implementations and a class's own functions now share one specialiser, since the only thing that differed was where the type variables were found. The call which names a function of a generic class and takes the instantiation from its receiver is the same shape too, so it is no longer specific to super calls. Also tightens the assertion added with the previous commit: it looked for a class by bare name, and a specialised function is named after the one it was copied from, so it read those as a class emitted without being allocated. --- BACKLOG.md | 61 +++------ CHANGELOG.md | 22 +-- .../imtranslation/EliminateGenerics.java | 129 +++++++++++------- .../resources/agent-docs/WURST_LANGUAGE.md | 4 +- .../wurstscript/tests/FastHashMapTests.java | 5 +- .../wurstscript/tests/TypeClassTests.java | 16 +-- 6 files changed, 122 insertions(+), 115 deletions(-) diff --git a/BACKLOG.md b/BACKLOG.md index 2dc027654..a97772baa 100644 --- a/BACKLOG.md +++ b/BACKLOG.md @@ -27,34 +27,6 @@ itself, and one gap in what the suite can see. the two targets have disagreed before, and every disagreement found so far was found by running the same program on both. -6. **Lua dispatch inside the constructor** of a bounded generic class. Works on Jass; there is now - a repro for both targets, `TypeClassTests.dispatchInsideConstructor` and - `dispatchInsideConstructorIsRejectedForLua`, the second pinning the current diagnostic. - - Still open after item 23's first half landed. The two mechanisms that closed items 13 and 25 both - read the instantiation off a receiver — the first argument of a call which names its target, or the - receiver of a method call. A constructor has neither: `new_Box(21)` takes no receiver, and the only - thing stating `Box` is the type of the variable it is assigned to. So this still needs the - type-driven collector below, which is a second mechanism rather than a widening of the one there is. - - Not the same gap as item 5, and the fix from it does not reach: a constructor belongs to the - 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`, and `new_Box` still generic; the calls *inside* it - (`construct_Box`, `Box_init`) do carry the class's type variable, but nothing gives the - outermost one a concrete argument. `collectGenericNewUse` requires non-empty type arguments, so - it never starts. - - What Jass does, from `TypeClassTests_dispatchInsideConstructor_no_opts.jim`: it specialises the - constructor function itself, `b_8 = new_Box⟪integer⟫(21)`. It gets there from *types*, not from - the call — `collectGenericUsages` collects a `GenericVar` for the local declared - `Box` and a `GenericReturnTypeFunc` for `new_Box`, whose return type is generic. - The Lua collector has neither; it only ever looks at calls. So attaching type arguments to - constructor calls, which an earlier note here proposed, is not what the Jass path relies on and - would be a second mechanism rather than the same one. - - Collecting from types on the Lua path runs straight into item 23, so settle that first. - 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. @@ -68,22 +40,22 @@ itself, and one gap in what the suite can see. 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. - **Half done.** Items 13 and 25 are closed, and the two things that closed them are the model - working: a specialised method is moved to the class its objects are actually allocated from, and a - call which names its target reads the instantiation off its receiver argument rather than off the - object's class. A specialisation nothing allocates is now left with no methods and drops out - entirely, so the second class shape is gone wherever an ordinary generic object is involved - + **Done.** Items 6, 13 and 25 are closed and the model is what closed them: a specialised method is + moved to the class its objects are actually allocated from, and every remaining site reads the + instantiation off something the call already has rather than off the object. A specialisation + nothing allocates is left with no methods and drops out entirely, so the second class shape is gone + wherever an ordinary generic object is involved - `FastHashMapTests.assertSpecialisedClassesAllocateTheirFields` states both accepted outcomes rather than the one that used to hold. - What is left is item 6, and it is the part this decision said would be the work: reading the - instantiation from a *type* rather than from a receiver. The classes which still allocate a - specialised copy are the ones whose construction was itself specialised, which is the same - question. Deciding it from what the program allocates, as the pass above does, is a fact rather - than a guess - but it is decided after the fact, and the model would rather no specialised class - were ever allocated. - - Not blocking the container, which works on both targets today. + The type-driven collector this entry expected to need was not needed. Item 6 looked like it wanted + one, since a constructor has no receiver and the note below reasoned the instantiation was only on + the type of what the result is assigned to. It is not: `new_Box(21)` carries the type argument + already. What was missing is that a function of a generic class declares no type variables of its + own — it uses the class's, which this target does not lift — so specialising it was read as nothing + to do, the argument was stripped and the dispatch inside was left abstract. Matching such a function + against its class's variables is the whole fix, and it is the same rule as everywhere else here + rather than a second mechanism. Written down because three earlier notes argued for the harder one. 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 @@ -229,6 +201,13 @@ itself, and one gap in what the suite can see. ## Done +- 6. A requirement dispatched from inside the constructor of a bounded generic class works on Lua. The + call running a constructor carries its type argument already; what was missing is that a constructor + declares no type variables of its own, using its class's, so specialising it was treated as nothing + to do — the argument was stripped and the dispatch left with no concrete type. Functions of a generic + class are now matched against the class's type variables, which also covers the constructor body and + the field initialiser it calls. + - 25. A bounded type parameter on a method of a generic class no longer trips the arity check on Lua. A method call there carries the class's type arguments followed by the method's own, and the check for whether the class's were still missing asked whether the call had *any* — so a method declaring diff --git a/CHANGELOG.md b/CHANGELOG.md index f9f1551e0..df00ae9d7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -58,17 +58,17 @@ arguments chosen for them, so that name resolves and says what it stands for. The argument must satisfy the bound, which is reported at the `use`. This works on both targets. -- On the Lua target, a bounded generic class can now be subclassed. A generic object stays erased there - and only the paths needing a concrete type are specialised, so the concrete type has to reach those - paths rather than the object: a specialised method is bound to the class its objects are allocated - from, and a call which names its target — `super.m()` is one — takes the instantiation from the class - its receiver is used as. A specialisation nothing allocates is no longer emitted at all. - - Two shapes remain unsupported on Lua. A requirement dispatched from inside a constructor is still - rejected, a constructor call having no receiver to read the instantiation from. And a method - combining its own type parameters with those of the generic class owning it is still not a supported - shape, though it is no longer rejected outright: the arity check it tripped over counted the class's - type arguments against a call that had only supplied the method's. +- On the Lua target, a bounded generic class can now be subclassed, and a requirement can be dispatched + from inside a constructor. A generic object stays erased there and only the paths needing a concrete + type are specialised, so the concrete type has to reach those paths rather than the object: a + specialised method is bound to the class its objects are allocated from, a call which names its target + — `super.m()` is one — takes the instantiation from the class its receiver is used as, and a function + of a generic class is matched against that class's type variables rather than being read as having + none of its own. A specialisation nothing allocates is no longer emitted at all. + + One shape remains unsupported on Lua: a method combining its own type parameters with those of the + generic class owning it, though it is no longer rejected outright — the arity check it tripped over + counted the class's type arguments against a call that had only supplied the method's. - Added new pseudo-natives for debugging memory leaks: diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/EliminateGenerics.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/EliminateGenerics.java index abdefc6a7..17db6a7ca 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/EliminateGenerics.java +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/EliminateGenerics.java @@ -236,7 +236,7 @@ private static ImExpr defaultValueFor(ImType type) { private void collectGenericNewRoots() { - methodsByImplementation = null; + classByFunction = null; prog.accept(new Element.DefaultVisitor() { @Override public void visit(ImFunction function) { @@ -319,30 +319,28 @@ && functionNeedsSpecialization(call.getFunc(), Collections.newSetFromMap(new Ide return; } if (call.getTypeArguments().isEmpty()) { - collectSuperCallToGenericMethod(call); + collectCallThroughGenericReceiver(call); } } /** - * Collects a call which names the implementation of a generic class's method outright, which is - * what {@code super.m()} becomes. + * Collects a call which names a function of a generic class outright, taking the instantiation + * from the receiver it was handed. *

- * Such a call has no receiver to read type arguments from, and this target never lifts the class's - * type variables onto the function, so there is nothing on the call to specialise against and the - * erased original is reached instead — where the dispatch it contains is dead. The receiver is - * still the first argument, and the class it is used as says which instantiation the caller - * extends, so the type the body needs is threaded to the call rather than taken from the object. + * {@code super.m()} is the case that shows why: the call names its target, so there is no receiver + * to read type arguments from, and this target never lifts the class's type variables onto the + * function, so nothing on the call says which instantiation to specialise for and the erased + * original is reached instead — where the dispatch it contains is dead. The receiver is still the + * first argument, and the class it is used as gives the same answer the lift gives elsewhere. A + * constructor's own body is reached the same way, its {@code this} being that first argument. */ - private void collectSuperCallToGenericMethod(ImFunctionCall call) { - if (call.getArguments().isEmpty()) { + private void collectCallThroughGenericReceiver(ImFunctionCall call) { + ImClass owningClass = classOwning(call.getFunc()); + if (owningClass == null || owningClass.getTypeVariables().isEmpty() + || !call.getFunc().getTypeVariables().isEmpty()) { return; } - ImMethod method = methodImplementedBy(call.getFunc()); - if (method == null || !call.getFunc().getTypeVariables().isEmpty()) { - return; - } - ImClass owningClass = method.getMethodClass().getClassDef(); - if (owningClass.getTypeVariables().isEmpty() + if (call.getArguments().isEmpty() || !(call.getArguments().get(0).attrTyp() instanceof ImClassType receiverType)) { return; } @@ -352,33 +350,36 @@ private void collectSuperCallToGenericMethod(ImFunctionCall call) { || typeArgumentsContainTypeVariable(classType.getTypeArguments())) { return; } - genericsUses.add(new GenericSuperCall(call, method, new GenericTypes(classType.getTypeArguments()))); + genericsUses.add(new GenericClassFunctionCall(call, owningClass, + new GenericTypes(classType.getTypeArguments()))); } /** - * The method a function is the implementation of, or null when it is not one. + * The class a function belongs to, whether as a method's implementation or as a function of its + * own, and null when it belongs to none. *

- * Rebuilt per collection pass rather than kept, because specialising a method adds another. This - * target leaves methods on their classes, so there is no owner map of the kind moving them out - * builds. + * Rebuilt per collection pass rather than kept, because specialising adds more. This target leaves + * both on their classes, so there is no owner map of the kind moving them out builds. */ - private @Nullable ImMethod methodImplementedBy(ImFunction function) { - if (methodsByImplementation == null) { - methodsByImplementation = new IdentityHashMap<>(); - prog.accept(new Element.DefaultVisitor() { - @Override - public void visit(ImMethod method) { - super.visit(method); + private @Nullable ImClass classOwning(ImFunction function) { + if (classByFunction == null) { + classByFunction = new IdentityHashMap<>(); + for (ImClass imClass : prog.getClasses()) { + for (ImFunction f : imClass.getFunctions()) { + classByFunction.putIfAbsent(f, imClass); + } + for (ImMethod method : imClass.getMethods()) { if (method.getImplementation() != null) { - methodsByImplementation.putIfAbsent(method.getImplementation(), method); + classByFunction.putIfAbsent(method.getImplementation(), + method.getMethodClass().getClassDef()); } } - }); + } } - return methodsByImplementation.get(function); + return classByFunction.get(function); } - private @Nullable Map methodsByImplementation; + private @Nullable Map classByFunction; /** * A construction states an instantiation that no call site has to mention. A closure is the case @@ -1268,6 +1269,17 @@ private ImFunction specializeFunction(ImFunction f, GenericTypes generics) { boolean needsGlobals = needsGlobalSpecialization(f); if (!isGeneric && !needsGlobals) { + // A function of a generic class declares no type variables of its own: it uses the + // class's, which this target does not lift onto it. The call already says which + // instantiation it is for, so match against the class's variables rather than treating + // the function as nothing to specialise, strip the arguments and leave the dispatch + // inside it with no concrete type. A constructor is the case that needs this - the call + // running it is the only place its instantiation is stated, there being no receiver yet. + ImClass owner = genericNewOnly ? classOwning(f) : null; + if (owner != null && !owner.getTypeVariables().isEmpty() + && owner.getTypeVariables().size() == generics.getTypeArguments().size()) { + return specializeClassFunction(f, owner, f, generics); + } return f; } if (generics.containsTypeVariable()) { @@ -1351,27 +1363,39 @@ private ImMethod specializeMethod(ImMethod m, GenericTypes generics) { } private ImFunction specializeMethodImplementation(ImMethod method, GenericTypes generics) { - ImFunction implementation = method.getImplementation(); - ImFunction specialized = specializedFunctions.get(implementation, generics); + return specializeClassFunction(method.getImplementation(), + method.getMethodClass().getClassDef(), method, generics); + } + + /** + * Specialises a function belonging to a generic class, whether it implements a method or is a + * function of the class in its own right - a constructor and the body it runs are the latter. + *

+ * This target does not lift a class's type variables onto its functions, so such a function uses + * them where they are declared and the arguments to match are the class's followed by any the + * function declares itself. + */ + private ImFunction specializeClassFunction(ImFunction function, ImClass owningClass, + Element blameFor, GenericTypes generics) { + ImFunction specialized = specializedFunctions.get(function, generics); if (specialized != null) { return specialized; } - List typeVariables = new ArrayList<>( - method.getMethodClass().getClassDef().getTypeVariables()); - typeVariables.addAll(implementation.getTypeVariables()); + List typeVariables = new ArrayList<>(owningClass.getTypeVariables()); + typeVariables.addAll(function.getTypeVariables()); if (typeVariables.size() != generics.getTypeArguments().size()) { - throw new CompileError(method, "Generics should match class method type variables."); + throw new CompileError(blameFor, "Generics should match class method type variables."); } - ImFunction newImplementation = implementation.copyWithRefs(); - specializedFunctions.put(implementation, generics, newImplementation); + ImFunction newImplementation = function.copyWithRefs(); + specializedFunctions.put(function, generics, newImplementation); specializedFunctionGenerics.put(newImplementation, generics); prog.getFunctions().add(newImplementation); - translator.recordSpecialisation(newImplementation, implementation, generics.getTypeArguments()); - recordCopiedTypeVars(implementation.getTypeVariables(), newImplementation.getTypeVariables()); + translator.recordSpecialisation(newImplementation, function, generics.getTypeArguments()); + recordCopiedTypeVars(function.getTypeVariables(), newImplementation.getTypeVariables()); newImplementation.getTypeVariables().removeAll(); - newImplementation.setName(implementation.getName() + "_specialized"); + newImplementation.setName(function.getName() + "_specialized"); rewriteGenerics(newImplementation, generics, typeVariables); collectGenericNewUses(newImplementation); return newImplementation; @@ -2238,24 +2262,25 @@ public void eliminate() { } /** - * A {@code super} call reaching a generic class's method, rewritten to the copy specialised for - * the instantiation the caller extends. The instantiation came from the receiver's class rather - * than from the call, so there are no type arguments on the call to clear. + * A call naming a function of a generic class, rewritten to the copy specialised for the + * instantiation the call is for. That instantiation came from the receiver it was handed or from + * the type the result is stored in, rather than from the call, so there are no type arguments on + * the call to clear. */ - class GenericSuperCall implements GenericUse { + class GenericClassFunctionCall implements GenericUse { private final ImFunctionCall call; - private final ImMethod method; + private final ImClass owningClass; private final GenericTypes generics; - GenericSuperCall(ImFunctionCall call, ImMethod method, GenericTypes generics) { + GenericClassFunctionCall(ImFunctionCall call, ImClass owningClass, GenericTypes generics) { this.call = call; - this.method = method; + this.owningClass = owningClass; this.generics = generics; } @Override public void eliminate() { - call.setFunc(specializeMethodImplementation(method, generics)); + call.setFunc(specializeClassFunction(call.getFunc(), owningClass, call, generics)); specializedCallSites.add(call); } } diff --git a/de.peeeq.wurstscript/src/main/resources/agent-docs/WURST_LANGUAGE.md b/de.peeeq.wurstscript/src/main/resources/agent-docs/WURST_LANGUAGE.md index 5dda397ce..b7133e175 100644 --- a/de.peeeq.wurstscript/src/main/resources/agent-docs/WURST_LANGUAGE.md +++ b/de.peeeq.wurstscript/src/main/resources/agent-docs/WURST_LANGUAGE.md @@ -231,7 +231,9 @@ class SubBox extends Box This works on both targets. -Two things are not guaranteed on Lua. A requirement dispatched from inside the constructor of a bounded generic class is rejected there, because a constructor call has no receiver to take the instantiation from; dispatch from a method, a closure, or a `super` call is supported. And a method which combines its own type parameters with those of the generic class owning it is not a supported shape on that target — write it as a free generic function, or parameterise the method only by its owning class. +A requirement may be dispatched from a method, a constructor, a closure, or a `super` call, on both targets. + +One shape is not guaranteed on Lua: a method which combines its own type parameters with those of the generic class owning it. Write it as a free generic function, or parameterise the method only by its owning class. ## Strings diff --git a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/FastHashMapTests.java b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/FastHashMapTests.java index 1a574b66e..1c0b82cf9 100644 --- a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/FastHashMapTests.java +++ b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/FastHashMapTests.java @@ -439,7 +439,10 @@ private static void assertSpecialisedClassesAllocateTheirFields(String compiled) String erasedFields = allocatedFields(compiled, "FastHashMap"); String specialisedFields = allocatedFieldsOrNull(compiled, "FastHashMap_specialized\\w*"); if (specialisedFields == null) { - if (compiled.contains("FastHashMap_specialized")) { + // The class table, not any name containing it: a specialised function is named after the + // one it was copied from, so matching the bare name would read those as a class. + if (Pattern.compile("(?m)^\\s*FastHashMap_specialized\\w*\\s*=\\s*\\(\\{\\s*\\}\\)") + .matcher(compiled).find()) { throw new AssertionError("a specialised class is emitted but never allocated;" + " its methods should be bound to the class the objects come from, leaving nothing" + " of it behind, in:\n" + compiled); diff --git a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/TypeClassTests.java b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/TypeClassTests.java index 86a874cc1..0d781a427 100644 --- a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/TypeClassTests.java +++ b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/TypeClassTests.java @@ -172,17 +172,15 @@ public void dispatchInsideConstructor() { } /** - * Still rejected for Lua, and this pins that it is rejected clearly rather than mistranslated. - * A constructor belongs to the class, not to a generic function of its own, so the call that - * runs it carries no type arguments — {@code new_Box(21)} in the intermediate language, with - * the instantiation only on the type of what it is assigned to. Nothing on the Lua path reads - * it from there, so the dispatch inside the constructor is never given a concrete type. - * Should that be made to work, this test fails and becomes the success case above. + * The same on Lua. A constructor belongs to its class rather than to a generic function of its + * own, so it declares no type variables and specialising it was treated as nothing to do — the + * call's type argument was stripped and the dispatch inside was left with no concrete type. The + * argument was on the call all along; what was missing is that a function of a generic class is + * matched against the class's type variables, since this target never lifts them onto it. */ @Test - public void dispatchInsideConstructorIsRejectedForLua() { - test().testLua(true).executeProg().expectError("could not be resolved for the Lua target") - .lines(DISPATCH_IN_CONSTRUCTOR); + public void dispatchInsideConstructorLua() { + test().testLua(true).executeProg().lines(DISPATCH_IN_CONSTRUCTOR); } /** From 34cf0aeff5309754aedd6fa924e43471215b6cc4 Mon Sep 17 00:00:00 2001 From: Frotty Date: Mon, 17 Aug 2026 22:05:21 +0200 Subject: [PATCH 12/12] Gate receiver-based specialisation, and stop copying a method to the erased class Two corrections to the previous two commits. The call which reads its instantiation off a receiver was specialising whatever it found, without asking whether the target reaches an operation needing a concrete type. Every call into a generic superclass would then get a copy per instantiation, of functions with no dispatch and no construction in them, on a target which otherwise keeps generics erased. The same check the other collectors use applies here; being able to read an instantiation says nothing about whether anything wants it. The other correction goes the other way. Giving the erased class its own copy of a specialised method, when both shapes are allocated, was meant to stop whichever shape lost the method from missing the slot. A copy is a dispatch group of its own, so it is named separately and the binding lands under a name no call site asks for - and joining it to the method it came from, so the two share a name, merges groups which are distinct on purpose and breaks the three closure tests outright. The one shape which reaches this is a closure, where each class already binds its own implementation under the same slot names and the erased allocation is dead. Reverted to leaving the methods where they are, with the reasoning recorded so the next attempt starts from what happened rather than from how it looks. --- .../imtranslation/EliminateGenerics.java | 29 ++++++++++--------- 1 file changed, 15 insertions(+), 14 deletions(-) diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/EliminateGenerics.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/EliminateGenerics.java index 17db6a7ca..d080df02b 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/EliminateGenerics.java +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/EliminateGenerics.java @@ -149,10 +149,12 @@ public void transformGenericNewOnly() { * the copy, while an ordinary generic object beside it is not. A specialised name carries its * instantiation, so two specialisations of one method stay distinct on the erased class. *

- * When both are allocated neither can give up the slot — a call was rewritten to the specialised - * method whichever class its receiver came from — so the erased class gets a binding of its own - * rather than the method moving. Being allocated is what settles this and not which shape looks - * primary; treating the two as alternatives leaves whichever lost without the slot. + * When both are allocated the methods stay where they are. Giving the erased class a copy of its + * own looks safer and is not: a copy is a dispatch group of its own, so it is named separately and + * the binding lands under a name no call site asks for, and joining it to the method it came from + * to share the name merges two groups which are deliberately distinct. The one shape reaching this + * is a closure, where each class binds its own implementation under the same slot names already + * and the erased allocation is dead. Left alone rather than fixed blind. */ private void bindSpecialisedMethodsToTheAllocatedClass() { Map erasedOf = new IdentityHashMap<>(); @@ -168,16 +170,6 @@ private void bindSpecialisedMethodsToTheAllocatedClass() { continue; } if (allocated.contains(specialized)) { - // Both shapes are allocated, so neither can give up the slot: a call was rewritten to - // the specialised method whichever class its receiver came from. The specialised class - // keeps its methods and the erased one gets a binding of its own to the same - // implementation. - for (ImMethod method : new ArrayList<>(specialized.getMethods())) { - ImMethod onErased = method.copyWithRefs(); - onErased.setMethodClass(JassIm.ImClassType(erased, JassIm.ImTypeArguments())); - onErased.setImplementation(method.getImplementation()); - erased.getMethods().add(onErased); - } continue; } for (ImMethod method : specialized.getMethods().removeAll()) { @@ -333,6 +325,11 @@ && functionNeedsSpecialization(call.getFunc(), Collections.newSetFromMap(new Ide * original is reached instead — where the dispatch it contains is dead. The receiver is still the * first argument, and the class it is used as gives the same answer the lift gives elsewhere. A * constructor's own body is reached the same way, its {@code this} being that first argument. + *

+ * Only for a target which reaches one of the operations needing a concrete type. Being able to + * read an instantiation off a receiver says nothing about whether anything wants it, and this + * target keeps generics erased: specialising every call into a generic superclass would make a + * copy per instantiation of functions with no dispatch and no construction in them. */ private void collectCallThroughGenericReceiver(ImFunctionCall call) { ImClass owningClass = classOwning(call.getFunc()); @@ -350,6 +347,10 @@ private void collectCallThroughGenericReceiver(ImFunctionCall call) { || typeArgumentsContainTypeVariable(classType.getTypeArguments())) { return; } + if (!functionNeedsSpecialization(call.getFunc(), + Collections.newSetFromMap(new IdentityHashMap<>()))) { + return; + } genericsUses.add(new GenericClassFunctionCall(call, owningClass, new GenericTypes(classType.getTypeArguments()))); }