Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 22 additions & 15 deletions BACKLOG.md
Original file line numberDiff line numberDiff line change
Expand Up@@ -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<T:>
function show(T x) returns int
Expand All@@ -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
Expand All@@ -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
Expand Down
Original file line numberDiff line numberDiff line change
Expand Up@@ -39,6 +39,8 @@ public class EliminateGenerics {
*/
private final Set<Element> specializedCallSites = Collections.newSetFromMap(new IdentityHashMap<>());
private final Table<ImFunction, GenericTypes, ImFunction> specializedFunctions = HashBasedTable.create();
/** The class each function was moved out of, for calls which name their target without a receiver. */
private final Map<ImFunction, ImClass> functionOwners = new IdentityHashMap<>();
private final Table<ImMethod, GenericTypes, ImMethod> specializedMethods = HashBasedTable.create();
private final Table<ImClass, GenericTypes, ImClass> specializedClasses = HashBasedTable.create();
private final Multimap<ImClass, BiConsumer<GenericTypes, ImClass>> onSpecializedClassTriggers = HashMultimap.create();
Expand DownExpand Up@@ -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.
* <p>
* 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<ImTypeArgument> 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)) {
Expand DownExpand Up@@ -862,6 +906,7 @@ private void moveFunctionsOutOfClass(ImClass c) {
List<ImFunction> functions = c.getFunctions().removeAll();
for (ImFunction f : functions) {
prog.getFunctions().add(f);
functionOwners.put(f, c);

List<ImTypeVar> newTypeVars = new ArrayList<>();
for (ImTypeVar imTypeVar : c.getTypeVariables()) {
Expand Down
Original file line numberDiff line numberDiff line change
Expand Up@@ -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.
* <p>
* 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);
Comment on lines +340 to +341

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Keep bounded-subclass behavior aligned on Lua

This changes the bounded-generic subclass case from an expected rejection to supported behavior, but the regression runs only the Jass path: on Lua, transformGenericNewOnly bypasses the new lifting logic and the same program still dispatches through the erased Box table, so it compiles but never reaches testSuccess. Implement the behavior for Lua as well, or retain an explicit Lua regression documenting a genuinely backend-specific difference rather than silently exposing divergent language semantics.

AGENTS.md reference: AGENTS.md:L217-L221

Useful? React with 👍 / 👎.

}

/**
* 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<T:>",
" function show(T x) returns int",
"implements Show<int>",
" function show(int x) returns int",
" return x",
"class Box<K: Show>",
" K key",
" construct(K k)",
" key = k",
" function choose<Q>(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<int>",
" construct(int k)",
" super(k)",
" override function size(int extra) returns int",
" return super.choose<Marker>(new Marker(), extra) + 100",
"init",
" Box<int> 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. */
Expand Down
Loading