IRGen: Fix subtle issue with "circular" conformance access paths #39588
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.
There are three things going on in rdar://83687967:
getConformanceAccessPath()
The ABI break
This part is actually not the root cause of the problem, but it is the reason why the test case used to work in Swift 5.4. Since we already shipped Swift 5.5, I think changing the ABI back to match the old behavior in this particular case would risk further regressions. The new ABI seems to make more sense, and I'm not changing it in this PR. But it could be changed back if we need to.
Here is the code:
Requirement inference on the concrete type
G<Child.A>
in thewhere
clause introduces the explicit conformance requirementChild.A: P1
from the requirementA: P1
in the signature ofG
.In 5.4 and 5.5, we discover the alternate conformance access path
(Child: P3)(Self.B: P2)(Self.A: P1)
forChild.A
. Note that the canonical type ofChild.B.A
isChild.A
.In 5.4, the "derived via concrete" check evaluates to true for this path, because the prefix Child.B is fixed to a concrete type. This excludes this conformance path from consideration.
In 5.5, the code was rewritten in an attempt to preserve existing behavior while fixing additional cases involving concrete types that the "derived via concrete" check didn't handle. This is where the "signature rebuilding after dropping conformances" stage of the GSB came from.
Clearly, I failed at preserving existing behavior in all cases, because in this case, it turns out that the "derived via concrete" check was too conservative. It was supposed to only consider a path "derived via concrete" if the root conformance was also redundant. This would happen if
Child : P3
was made redundant by a requirement likeChild == SomeConcreteType<...>
. In this example though,(Child: P3)
is not redundant, so the original conformance access path is actually valid forChild.A: P1
. This means that the inferred requirementChild.A : P1
is actually redundant, according to the actual semantics of conformance access paths.Indeed, 5.4 had a further inconsistency which highlights why the old behavior was wrong. In protocols, we don't perform requirement inference, since the idea is that the user should explicitly state all conformance requirements on associated types in a protocol, for readability.
You could write these two protocols in 5.4:
In the first example, there's no explicit
Child.A : P1
requirement. In the second, there was. Reformulating the original test case into the protocol requirement signature equivalent would still trigger the same IRGen crash with protocol X1, but not X2 in Swift 5.4:The conformance access path bug
There was a new implementation of conformance access paths in 5.5. It would skip recursing into the children of a conformance access path if its subject type was fixed to a concrete type. This is actually wrong because of exactly this case that we never tested: the witness table for
Child.A: P1
is recovered fromChild.B: P2
, andChild.B
is a concrete type.Concrete types were skipped here because the use of
getCanonicalTypeInContext()
would replace the type with its concrete type, but really what we want is the "canonical anchor", which is the most canonical type parameter of a type parameter (even if it happens to be concrete). After making that change a couple of redundant term/type round-trips become apparent, so simplifying the algorithm by usingTerm
throughout fixes this problem.The IRGen bug
The final problem is revealed when IRGen attempts to recover the witness table from the conformance path
(Child: P3)(Self.B: P2)(Self.A: P1)
. The associated conformance access function for the final step(Self.A : P1)
takes the metadata forChild.B
together with the witness table forChild.B : P2
as input. SinceChild.B
is concrete, IRGen would prefer to construct it directly, as the known concrete typeG<Child.A>
. The metadata forG
is instantiated with the type metadataChild.B
and the conformanceChild.A: P1
. Unfortunately, recoveringChild.A: P1
from the generic environment ofcaller11()
requires recursively evaluating the same conformance access path that is being evaluated now.However, IRGen can also recover
Child.B
from the conformanceChild: P3
, which is immediately available, and the metadata forChild
, by calling the associated type metadata access function. This function also constructsG<Child.A>
, however it does it from the concrete nominal type metadata for theChild: P3
conformance, which must storeChild.A: P1
. By using abstract metadata access here, we avoid the circularity.This last part is a bit of a hack because a more principled implementation would amend the conformance access path representation to encode access paths for each metadata component as well. I'll clean this up later.
Next steps
If we do need to change back to the 5.4 ABI, we can bring back the "derived via concrete" check in the GSB, and try to simulate it in the requirement machine's in-progress minimization algorithm.
The added test case covers several variations of this pattern to ensure the minimized set of conformance requirements does not change again.
The "generating conformances" algorithm that I'm working on to replace the GSB's signature minimization builds a set of simultaneous equations relating conformance access paths algebraically, and uses this to find a minimal set. These equations also encode the information describing how to recover the type metadata for the subject type of a conformance requirement. By serializing this information in type metadata, we will in the future be able to recover witness tables using conformance access paths encoded in the mangling format, avoiding
swift_conformsToProtocol()
calls at runtime on newer deployment targets. This will build on @al45tair's work in #39246.Fixes rdar://83687967.