diff --git a/BACKLOG.md b/BACKLOG.md index 6ece56210..1377c8400 100644 --- a/BACKLOG.md +++ b/BACKLOG.md @@ -72,8 +72,8 @@ because `LOOP.md` refers to items by number. source parameter. Making the node canonical lets all three compare by identity and removes a class of silent wrong dispatch. Mechanical, well covered by the suite. -13. **A bounded generic class cannot be subclassed.** Diagnosed on the Jass side and pinned by - `TypeClassTests.subclassOfBoundedGenericIsRejected`; the Lua half is still open. The program: +13. **A bounded generic class cannot be subclassed on Lua.** Fixed on Jass and pinned by + `TypeClassTests.subclassOfBoundedGeneric`; the Lua half is still open. The program: interface Show function show(T x) returns int @@ -99,18 +99,20 @@ because `LOOP.md` refers to items by number. if b.size(1) == 6 and b.shift(1) == 1001 and s.size(1) == 106 testSuccess() - On Jass the override's `super.size(extra)` becomes a direct call to the superclass - implementation, and once `Box` is specialised that call points at a function which has been - replaced by `Box_size⟪integer⟫`. The `.jim` for the test shows both side by side: the - specialised copy with its dispatch resolved, and `SubBox_size` still calling the original. - - Tried extending `addMemberTypeArguments` to attach the receiver's type arguments to such calls, - the way it already does for method calls and member accesses, and it changes nothing. The - callee has no type variables of its own — they belong to the class — so there is nothing for - type arguments on the call to select, and specialisation of a class function happens by - `specializeClass` copying the whole class instead. The call has to be **redirected** to that - copy, not annotated. The natural place is wherever a class is specialised: every call from a - subclass into a superclass function needs to follow. + On Jass the override's `super.size(extra)` became a direct call to the superclass + implementation, and once `Box` was specialised that call pointed at a function which had been + replaced by `Box_size⟪integer⟫`. Fixed by `addReceiverTypeArguments` in `EliminateGenerics`: + moving a function out of its class lifts the class's type variables onto the function, and a + call through a receiver gets them back from the receiver's type. A call which names its target + outright — `super.size(extra)` and `super(k)` both do — has no receiver to read, so it was left + asking for a function with type variables while supplying none. The receiver is still there as + the first argument, so the class it is used as gives the same type arguments the receiver would + have, and both super calls now reach the copy specialised for the instantiation. + + An earlier note here said annotating the call could not work, on the grounds that the callee had + no type variables of its own. That was wrong: `moveFunctionsOutOfClass` lifts the class's onto + it. The first attempt failed because nothing recorded which class a function had been moved out + of, so there was no way to adapt the receiver to it. Item 6 is the same family but not the same fix: there the constructor call also carries no type arguments, and there the instantiation is not on any argument either, only on the type of what @@ -119,7 +121,12 @@ because `LOOP.md` refers to items by number. Lua compiles and runs but never reaches `testSuccess`: the override makes `size` dispatched, and the emitted call is `b:Box_size_specialized_integer(1)` while `b` was allocated from the *erased* `Box` table, which binds only `shift`. The specialised table has the slot; the instance - never gets that table. That half is the erasure question again, as in item 5. + never gets that table. That half is the erasure question again, as in item 5, and the Jass fix + does not reach it: `transformGenericNewOnly` runs neither `simplifyClasses` nor + `addMemberTypeArguments`, so on Lua the type variables are never lifted in the first place. + Pinned by `TypeClassTests.subclassOfBoundedGenericIsStillBrokenOnLua`, which asserts the failure + rather than leaving the difference between the targets to be discovered. Fixing this half makes + that test fail, which is the point: it then becomes a second success case. 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/wurstscript/translation/imtranslation/EliminateGenerics.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/EliminateGenerics.java index 9d423291b..130d3bba0 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 @@ -39,6 +39,8 @@ public class EliminateGenerics { */ private final Set specializedCallSites = Collections.newSetFromMap(new IdentityHashMap<>()); private final Table specializedFunctions = HashBasedTable.create(); + /** The class each function was moved out of, for calls which name their target without a receiver. */ + private final Map functionOwners = new IdentityHashMap<>(); private final Table specializedMethods = HashBasedTable.create(); private final Table specializedClasses = HashBasedTable.create(); private final Multimap> onSpecializedClassTriggers = HashMultimap.create(); @@ -791,9 +793,51 @@ public void visit(ImMemberAccess ma) { super.visit(ma); addMemberTypeArguments(ma, (ImClass) ma.getVar().getParent().getParent()); } + + @Override + public void visit(ImFunctionCall call) { + super.visit(call); + addReceiverTypeArguments(call); + } }); } + /** + * Gives a call which reaches a class function directly the type arguments of the class. + *

+ * Moving a function out of its class lifts the class's type variables onto the function, and a + * call through a receiver gets them back from the receiver's type. A call which names its target + * outright has no receiver to read - {@code super.m()} and {@code super()} are both of this kind - + * so it is left asking for a function with type variables while supplying none, and nothing + * specialises it. The receiver is still there as the first argument, so the class it is used as + * gives the same type arguments the receiver would have. + */ + private void addReceiverTypeArguments(ImFunctionCall call) { + ImClass owningClass = functionOwners.get(call.getFunc()); + if (owningClass == null || call.getArguments().isEmpty()) { + return; + } + // The class's variables are lifted onto the front of the function's own, so what a call is + // short of is that prefix. A method with type parameters of its own already supplies theirs, + // which is a shorter list rather than an empty one. + int missing = call.getFunc().getTypeVariables().size() - call.getTypeArguments().size(); + if (missing != owningClass.getTypeVariables().size()) { + return; + } + if (!(call.getArguments().get(0).attrTyp() instanceof ImClassType receiverType)) { + return; + } + ImClassType classType = adaptToSuperclass(receiverType, owningClass); + if (classType == null || classType.getTypeArguments().size() != missing) { + return; + } + List typeArgs = new ArrayList<>(); + for (ImTypeArgument typeArgument : classType.getTypeArguments()) { + typeArgs.add(typeArgument.copy()); + } + call.getTypeArguments().addAll(0, typeArgs); + } + private void addMemberTypeArguments(ImMemberOrMethodAccess access, ImClass owningClass) { ImType receiverType = access.getReceiver().attrTyp(); if (!(receiverType instanceof ImClassType rt)) { @@ -862,6 +906,7 @@ private void moveFunctionsOutOfClass(ImClass c) { List functions = c.getFunctions().removeAll(); for (ImFunction f : functions) { prog.getFunctions().add(f); + functionOwners.put(f, c); List newTypeVars = new ArrayList<>(); for (ImTypeVar imTypeVar : c.getTypeVariables()) { 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 2aa4f6f0e..f4266e2a5 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 @@ -331,20 +331,66 @@ public void unwrittenArrayOfATypeParameterReadsAsItsDefaultReversed() { }; /** - * Subclassing a bounded generic class does not work yet, and this pins where it stops. The - * override's {@code super.size(extra)} becomes a direct call to the superclass implementation, - * and that call carries no type arguments — the type variables belong to the class, not to the - * method — so nothing specialises it. Specialising the class replaces the original with - * {@code Box_size⟪integer⟫}, and the super call is left pointing at what was removed. - *

- * The same shape as the constructor case above: class type arguments reach method calls and - * member accesses, but not constructor calls or super calls. Should either be fixed, look at - * both. The message names what was inside the function rather than the reference that kept it - * alive, because Jass emits whatever is called rather than only what the program still holds. + * A subclass of a bounded generic class reaches its superclass through both a super constructor + * call and a super method call, and each is a call which names its target rather than going + * through a receiver. Both carry the class's type arguments, so both reach the copy specialised + * for the instantiation the subclass extends. */ @Test - public void subclassOfBoundedGenericIsRejected() { - testAssertErrorsLines(false, "Typevar dispatch not eliminated", SUBCLASS_OF_BOUNDED_GENERIC); + public void subclassOfBoundedGeneric() { + testAssertOkLines(true, SUBCLASS_OF_BOUNDED_GENERIC); + } + + /** + * 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 question in item 5. + */ + @Test(expectedExceptions = Error.class, expectedExceptionsMessageRegExp = ".*Succeed function not called.*") + public void subclassOfBoundedGenericIsStillBrokenOnLua() { + test().testLua(true).executeProg().lines(SUBCLASS_OF_BOUNDED_GENERIC); + } + + /** + * 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 + * the two lists have to end up in the order the lift put the variables in. + */ + private static final String[] SUPER_CALL_TO_A_GENERIC_METHOD = { + "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 choose(Q q, int extra) returns int", + " return K.show(key) + extra", + " function size(int extra) returns int", + " return extra", + "class Marker", + "class SubBox extends Box", + " construct(int k)", + " super(k)", + " override function size(int extra) returns int", + " return super.choose(new Marker(), extra) + 100", + "init", + " Box s = new SubBox(5)", + " if s.size(1) == 106", + " testSuccess()", + }; + + @Test + public void superCallToAGenericMethodOfABoundedGenericClass() { + testAssertOkLines(true, SUPER_CALL_TO_A_GENERIC_METHOD); } /** Each type argument picks its own instance, so one generic serves several types. */