WIP archetype builder rework #4542
Closed
Conversation
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
slavapestov
force-pushed
the
wip-archetype-builder
branch
10 times, most recently
from
ead393c to
680ccb7
Compare
This makes things more explicit and easier to follow in my opinion. Also, clean up finalizeGenericParamList() to always take the outer generic signature, instead of pulling it out of context.
These will be more useful once substitutions in protocol conformances are moved to use interface types. At first, these are only going to be used by the SIL optimizer.
Build the witness thunk signature in a more principled manner, and clean up how the GenericEnvironment is constructed.
…ic signatures Now that we have the right operations, we can calculate the ApplyInst's substitutions from the generic signature of the witness thunk, rather than by concatenating substitution lists and hoping for the best.
We used RequirementSources to identify redundant requirements when building the canonical mangling signature. There were a few problems with how this worked: - We used the Inferred requirement source for both requirements that were inferred from function parameter and result types (for example, T : Hashable in `func foo<T, U>(dict: [T : U])` and some completely unrelated things, like same-type constraints imposed on nested types by existing same-type constraints on the outer types. - We used the Protocol requirement source for associated type requirements on protocols as well as conformance requirements on inherited protocols. - Introducing a new Redundant requirement did not mark existing Explicit requirements as Redundant. None of these were an issue because of how canonical mangling signature construction works: - We already started with a canonical signature, so there were no Inferred requirements of the first kind (which we *do* want to keep here) - We dropped both Protocol and Redundant requirements, so it didn't matter that the distinction here was not sufficiently fine-grained - We never introduced Explicit requirements that would be later superceded by a Redundant requirement, because we always started with an existing canonical signature where such Explicit requirements were already dropped. However, I want to use the same algorithm for building the original signature as the canonical mangling signature, so this patch introduces these changes: - The Inferred source is now only used for inferred requirements of the first kind; the second kind are now Redundant - The Protocol source is now only used for associated type requirements; inherited protocol conformances are now Redundant - updateRequirementSource() now does the right thing with introduced Redundant requirements For now, this doesn't change much, but an upcoming patch refactors ArchetypeBuilder::getGenericSignature() to also use enumerateRequirements(), except dropping Redundant requirements only, and not Protocol.
Instead of walking over PotentialArchetypes representatives directly and using a separate list to record same-type constraints, just use enumerateRequirements() and check the RequirementSource to drop redundant requirements. This means getGenericSignature() and getCanonicalManglingSignature() can share the same logic for collecting requirements; the only differences are the following: - both drop requirements from Redundant sources, but mangling signatures also drop requirements from Protocol sources - mangling signatures also canonicalize the types appearing in the final requirement So now ArchetypeBuilder::collectRequirements() becomes much simpler by relying on enumerateRequirements(), and the separate SameTypeRequirements list is gone.
slavapestov
force-pushed
the
wip-archetype-builder
branch
from
680ccb7 to
b37e2fe
Compare
kateinoigakukun
added a commit
that referenced
this pull request
Aug 31, 2022
[pull] swiftwasm from main
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.
Replace this paragraph with a description of your changes and rationale. Provide links to external references/discussions if appropriate.
Resolves SR-NNNN.