Uh oh!
There was an error while loading. Please reload this page.
Verify the junk dispatch slot and record why it stays - #1247
Open
Frotty wants to merge 1 commit into
Open
Conversation
Confirmed dead: in the Lua for FastHashMap<int, int> the slot is assigned once, bound to the alphabetically first method, and never called - the only method calls in that script are the two constructors. Confirmed harder to remove than the note implied. addDirectAliases was changed to drop a composed name which more than one method of the class produces, on the grounds that a name meaning one of these arbitrarily is worse than a name meaning nothing, and the slot was still emitted: collectDispatchSlotNames composes it independently from the cross product of hierarchy class names and the group's semantic names, and being called per group it cannot see the sibling collision without class-level knowledge it is not given. So a fix needs a shared notion of ambiguity across both composers, which is the coordinated change the note weighed against the benefit, and the benefit is still one unused table key per specialised class. Reverted, and the item now carries what was verified rather than what was assumed.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Backlog only — no behaviour change. Item 15 said a specialised class carries one dead dispatch slot and that fixing it risks more than it saves. Both halves are now verified rather than assumed, and the second turned out to be more true than the note knew.
It is dead
In the Lua for
FastHashMap<int, int>:Assigned once, bound to the alphabetically first method, never called — the only method calls in that script are
:create(and:create1(. The name is the owner's plus the segment after the last underscore of the method's, and for a specialised method that segment is the type argument, so every method of the class composes the same name.Suppressing it in one composer is not enough
I changed
addDirectAliasesto drop a composed name which more than one method of the same class produces, on the grounds that a name meaning "one of these, arbitrarily" is worse than a name meaning nothing. All Lua dispatch, FastHashMap and type class tests passed — and the slot was still emitted.LuaTranslator.collectDispatchSlotNamescomposes it independently, from the cross product of the class names in the hierarchy and the semantic names of the method group. It is called per group of same-named methods, so it cannot see that a sibling group composes the same name without class-level knowledge it is not given.So a real fix needs a shared notion of ambiguity across both composers. That is the coordinated change the note weighed against the benefit, and the benefit is still one unused table key per specialised class. Reverted.
What a fix would need
Recorded in the item, together with the earlier attempt: using the declared name instead collapses overloads, which two existing tests exist to prevent. Both sources of a semantic name are wrong in opposite directions — the mangled trailing segment collides across the siblings of one specialisation, the declared name across overloads — so the name has to separate both, in both composers. The declared name together with the dispatch signature key would.