Skip to content

Give a super call the type arguments of the class it reaches - #1237

Merged
Frotty merged 2 commits into
masterfrom
fix/subclass-of-bounded-generic
Aug 16, 2026
Merged

Give a super call the type arguments of the class it reaches#1237
Frotty merged 2 commits into
masterfrom
fix/subclass-of-bounded-generic

Conversation

@Frotty

Copy link
Copy Markdown
Member

A bounded generic class could not be subclassed. class SubBox extends Box<int> with an override calling super.size(extra) failed on Jass with Typevar dispatch not eliminated, even though the specialised copy of the superclass was built correctly next to it.

Cause

moveFunctionsOutOfClass lifts a class's type variables onto each function it moves out, so Box.size becomes a function with its own type variable K. addMemberTypeArguments gives those type arguments back to calls that go through a receiver — ImMethodCall and ImMemberAccess — by adapting the receiver's type to the owning class.

A call that names its target outright has no receiver node to read. super.size(extra) and super(k) are both of that kind, so they were left asking for a function with type variables while supplying none. Nothing specialised them, and the super call kept pointing at the original while its dispatch went unresolved to the backend.

The error named 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 — which is why this read as a missed dispatch rather than a missed call.

Fix

addReceiverTypeArguments handles the remaining case: 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. It only touches calls that supply no type arguments to a callee that has type variables — a state that is broken however it arose. functionOwners records which class each function was moved out of, which is what the first attempt at this was missing.

Both super calls are covered, so the super constructor call reaches the specialised constructor too.

Note on the previous analysis

BACKLOG.md claimed annotating the call could not work, on the grounds that the callee had no type variables of its own and that the call had to be redirected to the specialised copy instead. That was wrong — the lift gives it type variables — and the entry is corrected here.

Tests

TypeClassTests.subclassOfBoundedGenericIsRejected becomes subclassOfBoundedGeneric and now executes the program. Full suite green.

The Lua half is still open and stays in the backlog: transformGenericNewOnly runs neither simplifyClasses nor addMemberTypeArguments, so the type variables are never lifted there, and the remaining gap is the erasure-model question tracked as item 5.

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, so it was left asking for a function with type variables while
supplying none, and nothing specialised it. Both `super.m()` and `super()`
are of that kind, which is why a bounded generic class could not be
subclassed: the specialised copy of the superclass was built and the super
call still pointed at the original, taking its unresolved dispatch to the
backend with 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.

@chatgpt-codex-connectorchatgpt-codex-connectorBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit:7da79edcf9

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +816 to +817
if (!call.getTypeArguments().isEmpty() || call.getFunc().getTypeVariables().isEmpty()) {
return;

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 Preserve existing method type arguments on super calls

When the superclass method has its own type parameter, such as Box<K>.choose<Q>(Q) called as super.choose<real>(2.), the call already contains Q's argument, so this early return prevents prepending the receiver-derived K. The callee now has lifted type variables [K, Q] but specialization receives only <real>, causing rewriteGenerics to throw RuntimeException: Rewrite generics with wrong sizes; distinguish existing method arguments from the missing owner-class prefix rather than skipping every nonempty call.

Useful? React with 👍 / 👎.

Comment on lines +340 to +341
public void subclassOfBoundedGeneric() {
testAssertOkLines(true, SUBCLASS_OF_BOUNDED_GENERIC);

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 👍 / 👎.

A method may have type parameters of its own on top of the class's, and the
lift puts the class's on the front of them. Such a call already supplies an
argument for its own parameter, so what it is short of is that prefix rather
than everything; skipping every call which had any arguments left the callee
asking for two and given one, which fails while rewriting.
Also pins the Lua behaviour of the subclass case rather than leaving the
difference between the targets to be found: it compiles and runs there and
never reaches testSuccess, so the test says so and will fail once that half
works.
@Frotty

Copy link
Copy Markdown
MemberAuthor

Both addressed.

Preserve existing method type arguments on super calls — correct, and it was a crash rather than a missed case. The lift prepends the class's type variables to the function's own, so a call is short of that prefix, not of everything; my guard treated any non-empty argument list as already handled. It now compares counts and prepends only when what is missing is exactly the owning class's arity, with the adapted class type's arity checked to match before anything is added.

superCallToAGenericMethodOfABoundedGenericClass covers it: Box<K: Show> with choose<Q>, called as super.choose<Marker>(new Marker(), extra). K is int and Q is a class, so the test also fails if the prefix ever goes on in the wrong order rather than only if it is missing.

Keep bounded-subclass behavior aligned on Lua — fair, the backlog entry alone was not enough. subclassOfBoundedGenericIsStillBrokenOnLua now runs the same program on Lua and pins that it compiles, runs, and never reaches testSuccess. It cannot use expectError the way dispatchInsideConstructorIsRejectedForLua does, because nothing errors — the object is allocated from the erased Box table while the specialised one holds the method — so it asserts the failure explicitly and will break as soon as that half starts working.

The Lua half stays open for the reason in item 13: transformGenericNewOnly runs neither simplifyClasses nor addMemberTypeArguments, so the type variables are never lifted there and a super call has nothing to carry. Closing it is the erasure-model question tracked as item 5, not something to fold into this change.

Full suite green.

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Swish!

Reviewed commit:cc379c0b63

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@Frotty
Frotty merged commit 0cef8ad into masterAug 16, 2026
4 checks passed
@Frotty
Frotty deleted the fix/subclass-of-bounded-generic branch August 16, 2026 11:08
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@Frotty