Repository navigation
Interface bridge: give generic classes a method of their own too - #1423
Conversation
A class which implements an interface method with a method it inherits from a class outside the interface (C extends Base implements Omega) gets a method of its own since #1421, because the dispatch over the interface's implementations never visits Base. Generic classes were left out, so a call through the interface reached the wrong implementation for them, on every target and already in the interpreter: C<T:> extends Base, C extends Base<int>, C<T:> extends Base<T> implements Omega<T>, and the same with an override below C or a default from another interface. The bridge shared Base's function. A method is specialised with the functions of its own class, and Base's function belongs to Base and needs Base's type arguments as C sees them, which need not be C's (C<T:> extends Base<string>). So where C or Base is generic the bridge gets a function of C which calls Base's with this, as super.m() does; the specialisation then finds Base's type arguments from the type of this. Its parameters are Base's function's with Base's type variables replaced by those arguments. Bridges between plain classes keep sharing the function. With -inline the forwarding call is inlined away (Lua output test).
|
@codex review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4fc2cf3413
ℹ️ 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".
The function a generic class gets for an inherited implementation now: - is vararg where the inherited function is, so a call with several values reaches it whole (the interpreter rejected the arity, Lua dropped values); - maps a type parameter no supertype binds, one of an enclosing generic class which a static class inside it captures, to that parameter as the class's own functions see it, instead of failing to compile; - is made once per class and implementation, and added to its class after every unit is translated, by its sort key: two of them can share a name (the overloads of a method), and the order they were made in followed the order of the compilation units.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b93d74cfe9
ℹ️ 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".
1 participant
Problem
Since #1421, a class that implements an interface method with a method inherited from a class outside the interface (
C extends Base implements Omega) gets a method of its own. Without it, the dispatch over the interface's implementations never visitsBase. #1421 left generic classes out, so calls through the interface still reached the wrong implementation for them. This happened on every target, including the interpreter. Affected shapes:C<T:> extends Base implements Omega, also with an override belowC(generic or plain) and with a default from another interfaceC extends Base<int> implements OmegaC<T:> extends Base<T> implements Omega<T>, withBasereading a field of its type parameterFixed<T:> extends Base<string>andSecond<A:, B:> extends Base<B> implements Swap<B>Change
The bridge used to share
Base's function. A method is specialised with the functions of its own class, butBase's function belongs toBase. It must be specialised forBase's type arguments asCsees them, which need not beC's own (C<T:> extends Base<string>).So when
CorBaseis generic,InterfaceTranslatornow gives the bridge a function ofCthat callsBase's function withthis. This is the same shape assuper.m(), soEliminateGenericsreadsBase's type arguments from the type ofthis, as it does for a super call. The new function takesBase's function's parameters, withBase's type variables replaced by those arguments. Bridges between non-generic classes still share the function, so their output is unchanged.Fixing the bridge where it is created avoids patching
EliminateGenerics. That pass assumes a method's implementation belongs to the method's class in five places: specialising the method, removing generic leftovers, deciding which calls need dispatch specialisation, the Lua implementation specialisation, and call type arguments. Simply lifting the restriction compiled but miscompiled: the generic original of the bridge survived and Jass failed with "Could not resolve type id for class C", orC<int>dropped out of the interface dispatch.Cost: the bridge function adds one forwarding call. With
-inlinethe call is inlined away; a Lua output test checks that the bridge holdsBase's body directly. Without-inline, a dispatch to such a class pays one extra call.Checks
InterfaceTests(Jass, interpreter and Lua). The six behaviour tests all fail on master; the seventh checks the optimised Lua output shape.InterfaceTests,GenericsWithTypeclassesTests,GenericsTests,ClassesTests,ClosureTests,ModuleTests,LuaBackendAuditTests,LuaTranslationTests,LuaKeyedMapTests,DeterministicChecks,FieldIterationTests,NewFeatureTests,BugTests,TreeShakerTests,OptimizerTests,TypeClassTests,InterpreterTests,RealWorldExamples,StdLibOwnTests,CodeListSupportTests,LuaCodeListTests,FastHashMapTests,KeyedTableTests.-lua -inline -localOptimizations, produce Lua byte-identical to master. Neither uses the generic shape.Not covered
These shapes cannot reach the bridge, so nothing handles them:
<T>and new<T:>generics across the class and its superclass. The validator rejects it.