AST: Fix derivation of conformances in Substitution::subst() in the p… #4672
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
|
@swift-ci Please smoke test |
…resence of same-type constraints
Applying a substitution list to a Substitution is done in two
steps:
1) First, apply the substitution list to the original replacement
type R to get the substituted replacement type R'.
2) Second, for each abstract conformance of R from the original
substitution, look up a concrete conformance of R' from the
substitution list.
With minimized generic signatures, we would omit conformances of
a nested type T.A if they could be derived from some other
conformance already in the substitution list.
However, the derivation process was incorrect, because it would
only look at the canonical parent type T, and not any other parent
type that had a child A which was same-type equivalent to T.A.
For example, consider the following code:
protocol P1 { associatedtype A : P3 }
protocol P2 { associatedtype A : P4 }
protocol P3 {}
protocol P4 {}
func doSomething<T : P4>(...) {}
func doSomethingElse<T1 : P1, T2 : P2>(...) where T1.A == T2.A {
doSomething(...)
}
If we specialize doSomethingElse() with a pair of concrete types
to replace T1 and T2, we would need to find a concrete conformance
to replace the abstract conformance T2.A : P4 in the call to
doSomething().
Since the conformance of T2.A : P4 is a redundant requirement,
it does not appear in the conformance map; furthermore, T1.A and
T2.A are same-type equivalent, so they map to the same archetype.
We would therefore look at the canonical parent of T2.A, which is
T1, and look up the conformance of T1 : P1 in the substitution list.
However, the only requirement P1 imposes on A is the conformance
A : P3. There's no conformance A : P4 inside T1 : P1, so we would
hit an assertion.
Indeed, the conformance T1.A : P4 must be recovered from the
conformance of T2 : P2, because P2 requires that A : P4.
This patch ensures that the conformance can be found by changing
the ArchetypeConformanceMap from a simple mapping of archetypes
to conformances into a structure that records same-type constraints
as well.
So instead of just looking at the canonical parent of the archetype
T1.A, we consult the structure to check all archetypes that have
T1.A as a child, in this case both T1 and T2.
T2 : P2 contains the conformance we need, allowing the above code
to be specialized correctly.
slavapestov
force-pushed
the
fix-same-type-conformance-derivation
branch
from
22e30f6 to
9cba638
Compare
|
@swift-ci Please smoke test |
1 similar comment
|
@swift-ci Please smoke test |
aaditya-chandrasekhar
pushed a commit
to val-verde/swift
that referenced
this pull request
Sep 30, 2022
[pull] swiftwasm-release/5.7 from release/5.7
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 fixes a second regression from #4617. Here, there was an issue in deriving conformances for nested types that were omitted from the minimized signature if a parent type of the nested type was the subject of a same-type constraint.
There's some further cleanup possible here, to generalize the
ArchetypeConformanceMapto work with interface types, and use it to replace some similar code inGenericSignature.cppthat suffers from the same problem in theory (we just haven't hit it yet since it's only used in witness thunk emission right now).I'll work on consolidating the duplicated code and fixing the
GenericSignaturecase after I conform that the original regression is fixed by this patch.