(Almost) Remove AllArchetypes list #4451
Merged
Conversation
This file contains 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
This is the opposite of GenericSignature::getSubstitutionMap(), transforming an interface type substitution mapping into a Substitution array. Previously, Substitution arrays were built with hand-rolled logic, usually relying on a GenericParamList's AllArchetypes list. We want to stop using the AllArchetypes list, instead using the requirements array from a GenericSignature. Nothing calls this method yet; existing code will be refactored to call it in the next few patches.
…enericSignature::getSubstitutions(), NFC This is the first, and most trivial, usage of the new GenericSignature::getSubstitutions() method. Note that getForwardingSubstitutions() now takes a GenericSignature, which is slightly awkward. However, this is in line with our goal of 'hollowing out' GenericParamList by removing knowledge of the finalized generic requirements. Also, there is now a new getForwardingSubstitutionMap() function, which returns an interface type substitution mapping. This is used in the new getForwardingSubstitutions() implementation, and all also be used elsewhere later. Finally, in the SILFunction we now cache the forwarding substitutions, instead of re-computing them every time. I doubt this makes a big difference in performance, but it's a simple enhancement and every little bit helps.
It seems we passed this flag in from the very start, when this code was added in 2013. It no longer appears to make a difference though, since I think other bugs that were causing us to miss archetypes here are now fixed.
…:getSubstitutions(), NFC
This removes a pile of getAll{Nested,}Archetypes() usages and
greatly simplifies the logic here.
…cSignature::getSubstitutions(), NFC Note that there was some non-obvious dead code here: - We already drop conformance requirements on dependent types that are the subject of same-type constraints, so we no longer have to skip dependent types that are mapped to concrete types explicitly. In fact, if this were not the case, other code that iterates over the requirements of a GenericSignature would be wrong. The original hack was added in 2014, I guess we fixed the ArchetypeBuilder since then. - We never end up here where the original type to substitute is a recursive archetype. Recursive archetypes are not really a thing that is implemented properly, and even in the compiler_crashers collection this wasn't triggered. - With the above two changes, the mapTypeIntoContext() call is not necessary at all, which is nice because this is somewhat inefficient; mapTypeIntoContext() walks all outer generic parameter lists to find the archetype for the given dependent type.
…cSignature, NFC This function takes a substitution array and produces a contextual type substitution map, so it is the contextual type equivalent of GenericSignature::getSubstitutionMap(), which produces an interface type substitution map. The new version takes a GenericSignature, just like the new getForwardingSubstitutions(), so that it can walk the requirements of the signature rather than walking the AllArchetypes list. Also, this new version now produces a mapping from archetypes to conformances in addition to the type mapping, which will allow it to be used in a few places that had hand-coded logic.
…itutionMap(), NFC Again, we now pass in a GenericSignature so that we can walk its requirements, rather than relying on the AllArchetypes list embedded inside the GenericParamList.
…enericParamList::getSubstitutionMap(), NFC
…in place of GenericParamList::getAllNestedArchetypes(), NFC Mostly just plumbing the GenericSignature through to the caller.
…es, NFC I don't like this code, it doesn't make much sense to me. Ideally, this entire file should be replaced with the new code we have in include/swift/RemoteAST for mapping mangled type strings to Types and Decls.
…ry Decl's AllArchetypes I don't see any tests failing with this code removed; I guess either the duplicate archetype issue no longer occurs, or does not matter since we use interface types almost everywhere when talking about Decls from other modules.
… void instead, NFC
|
@swift-ci Please test |
|
@swift-ci Please smoke test os x |
|
Build failed |
|
@swift-ci Please clean test Linux |
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.
This PR nukes most usages of AllArchetypes, except some stuff in serialization which breaks some obscure .sib tests if I remove them.
I wanted to get the rest checked in before I untangle serialization, to avoid bitrot.