Record what a specialised node was copied from, in one place - #1248
Merged
Conversation
Specialising a generic entity makes a new node rather than recording a relation, so a copy cannot say what it stands for. Three passes recovered that by other means and each was wrong in a way which reached a user: fields dropped as dead because an access still named the original's variable, a value dispatched through the wrong instance because type variables were matched by name, and a slot named after a type argument because the name was composed from a copy's mangled name. Two of those were fixed by hand-rolled side tables on the translator, one for fields and one for type variables. This is the same relation once: a copy, what it was copied from, and the type arguments it was made for. The two tables are gone and their five callers - in EliminateGenerics, ImOptimizer, RemoveGarbage and LuaTranslator - read the one relation instead. Behaviour is unchanged on purpose. Classes, functions and method implementations now record their origin too, and nothing reads those yet: the naming and pruning passes which still derive structure from mangled names are the next step, and they need the relation to exist first. SpecialisationOriginTest covers the relation directly, since three passes now depend on it: a copy leads back to its original, a copy of a copy to the root, two nodes sharing a name are not the same node, the type arguments are kept, and a cycle is reported rather than followed forever.
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 free
to 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.
Stage one of the root-level change: make instantiation a relation the compiler holds, rather than something it re-derives.
Behaviour is unchanged on purpose. This introduces the relation and moves existing users onto it. The passes which still derive structure from mangled names are the next change, and they need this to exist first.
The problem it addresses
Specialising a generic entity makes a new node rather than recording a relation, so a copy has no way to say what it stands for. Three passes recovered that by other means, and each was wrong in a way that reached a user:
Two of those were fixed with hand-rolled side tables on the translator — one for fields, one for type variables. They are the same relation written twice.
What this does
One relation: a copy, what it was copied from, and the type arguments it was made for.
Both side tables are gone. Their five callers — in
EliminateGenerics,ImOptimizer,RemoveGarbageandLuaTranslator— read the one relation. Classes, functions and method implementations record their origin as well now, with the type arguments; nothing reads those yet, which is deliberate.Keeping the type arguments is the part that matters for what comes next: a name can be composed from
(original, arguments)instead of parsed back out of a string.Tests
SpecialisationOriginTestcovers the relation directly, since three passes depend on it: a copy leads back to its original, a copy of a copy to the root, two nodes sharing a name are not the same node, the arguments are kept, and a cycle is reported rather than followed forever — a cycle cannot arise from specialising, so one means a mistake elsewhere.Narrow suites green:
TypeClassTests,FastHashMapTests,GenericsTests,LuaTranslationTests,GenericsWithTypeclassesTests,SpecialisationOriginTest.What comes next, and what I have parked
Stage two is the payoff: naming and pruning consume the relation, retiring
semanticNameFromMethodName, the two independent slot composers, andProgramState.identifyGenericStaticGlobals's longest-underscore-prefix search for a global's owning class.Module bounds is parked on
feat/module-instanciation-type-params, unpushed. The grammar change works as far as resolution and dispatch, but it regresses two existing tests: generic modules already resolve their type parameters by matching the receiver type, and declaring the parameters on the instantiation collides with that. Reconciling the two is a design decision rather than a patch, and the branch carries the error chain that got there.