diff --git a/BACKLOG.md b/BACKLOG.md index aa81417ac..a97772baa 100644 --- a/BACKLOG.md +++ b/BACKLOG.md @@ -27,76 +27,35 @@ 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. - - 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, 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. + + **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. + + 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 @@ -184,21 +143,29 @@ 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 + 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 @@ -208,23 +175,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 +189,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; @@ -242,6 +201,45 @@ 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 + 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 + 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 + `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..df00ae9d7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -41,6 +41,35 @@ 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. + +- 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: // returns the maximum type id, can be usd to 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/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/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/EliminateGenerics.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/EliminateGenerics.java index 1e0bbdc0e..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 @@ -131,9 +131,67 @@ 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. + *

+ * 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<>(); + 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(erased)) { + continue; + } + if (allocated.contains(specialized)) { + 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 +228,7 @@ private static ImExpr defaultValueFor(ImType type) { private void collectGenericNewRoots() { + classByFunction = null; prog.accept(new Element.DefaultVisitor() { @Override public void visit(ImFunction function) { @@ -249,9 +308,80 @@ && functionNeedsSpecialization(call.getFunc(), Collections.newSetFromMap(new Ide if (!typeArgumentsContainTypeVariable(call.getTypeArguments())) { genericsUses.add(new GenericImFunctionCall(call)); } + return; + } + if (call.getTypeArguments().isEmpty()) { + collectCallThroughGenericReceiver(call); } } + /** + * Collects a call which names a function of a generic class outright, taking the instantiation + * from the receiver it was handed. + *

+ * {@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. + *

+ * 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()); + if (owningClass == null || owningClass.getTypeVariables().isEmpty() + || !call.getFunc().getTypeVariables().isEmpty()) { + return; + } + if (call.getArguments().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; + } + if (!functionNeedsSpecialization(call.getFunc(), + Collections.newSetFromMap(new IdentityHashMap<>()))) { + return; + } + genericsUses.add(new GenericClassFunctionCall(call, owningClass, + new GenericTypes(classType.getTypeArguments()))); + } + + /** + * 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 adds more. This target leaves + * both on their classes, so there is no owner map of the kind moving them out builds. + */ + 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) { + classByFunction.putIfAbsent(method.getImplementation(), + method.getMethodClass().getClassDef()); + } + } + } + } + return classByFunction.get(function); + } + + private @Nullable Map classByFunction; + /** * 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 +492,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 +507,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)}. @@ -1124,6 +1270,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()) { @@ -1207,27 +1364,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; @@ -2093,6 +2262,30 @@ public void eliminate() { } } + /** + * 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 GenericClassFunctionCall implements GenericUse { + private final ImFunctionCall call; + private final ImClass owningClass; + private final GenericTypes generics; + + GenericClassFunctionCall(ImFunctionCall call, ImClass owningClass, GenericTypes generics) { + this.call = call; + this.owningClass = owningClass; + this.generics = generics; + } + + @Override + public void eliminate() { + call.setFunc(specializeClassFunction(call.getFunc(), owningClass, call, generics)); + specializedCallSites.add(call); + } + } + class GenericNewCall implements GenericUse { private final ImFunctionCall call; 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/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/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/main/resources/agent-docs/WURST_LANGUAGE.md b/de.peeeq.wurstscript/src/main/resources/agent-docs/WURST_LANGUAGE.md index 84375eeec..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 @@ -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 @@ -215,7 +229,11 @@ 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 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 074d47b85..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 @@ -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,33 @@ 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) { + // 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); + } + 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 +457,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/LuaTranslationTests.java b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LuaTranslationTests.java index c64ac4144..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 @@ -1408,6 +1408,101 @@ 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 dispatches inside 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 + public void underscoreNamedOverrideDispatchesInAGenericHierarchy() { + 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()); 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..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); } /** @@ -342,19 +340,55 @@ 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); } + /** + * 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 @@ -434,28 +468,28 @@ 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. + *

+ * 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. *

- * 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. + * 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 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); } /** * 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 +514,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 +1262,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() {