[AST] Start using conformance access paths in SubstitutionMap #8521
Conversation
DougGregor
force-pushed
the
substitutionmap-conformance-access-path
branch
from
48112e8
to
f524135
Compare
lib/AST/SubstitutionMap.cpp
Outdated
| SubstitutionMap::lookupConformance(CanType type, ProtocolDecl *proto) const { | ||
| // If we have an archetype, map out of the context so we can compute a | ||
| // conformance access path. | ||
| GenericEnvironment *genericEnv = nullptr;; |
| protocol P1 {} | ||
| protocol P2 { associatedtype T } | ||
| protocol P3 { associatedtype U } | ||
| protocol P4: P3 where U: P2, U.T: P1 {} |
There was a problem hiding this comment.
Hm, isn't this isomorphic to the NestedConforms test above? @slavapestov's targeted fix #8428 that made that work includes a "rip this out once conformance access paths are plumbed through" comment, can that be executed now?
There was a problem hiding this comment.
Oh, you're right. Well then! I'll see if I can rip out the hack in #8428 now; it might require better handling of concrete types in GenericSignature.
DougGregor
changed the title
|
@swift-ci please smoke test and merge |
DougGregor
force-pushed
the
substitutionmap-conformance-access-path
branch
2 times, most recently
from
011a016
to
ac94585
Compare
|
@swift-ci please smoke test and merge |
DougGregor
force-pushed
the
substitutionmap-conformance-access-path
branch
from
ac94585
to
adbed0a
Compare
|
@swift-ci please smoke test and merge |
1 similar comment
|
@swift-ci please smoke test and merge |
Substitution maps are effectively tied to a particular generic signature or environment; keep track of that signature/environment so that we can (eventually) use it to find conformances.
…kup. When we are checking the conformance of a nominal type to a particular protocol, we check each of the requirements in the requirement signature, substituting in our Self type and (by extension) type witnesses. However, when we're trying to fulfill requirements for associated types (which might be mapped to concrete types), perform the conformance lookup in the module of the conformance itself---not the SubstitutionMap, which can't possibly have them at this point. Fixes rdar://problem/31041997.
When looking up a particular conformance in a SubstitutionMap, use the associated generic signature to determine how to find that conformance (via a conformance access path). Follow that conformance access path to retrieve the desired conformance. FIXME: There is still one failure, here Constraints/generic_overload.swift which appears to be because we either have the wrong generic signature for the override case or we are looking in the wrong map.
…rmances. When looking for a conformance for an archetype, map it out of context to compute the conformance access path, then do the actual lookups based on mapping the starting type back into the context. Eliminate the parent map and "walk the conformances" functionality.
When asking a substitution map for a conformance, it's okay if the conformance isn't there---just detect this case and return None. Also, collapse a redundant testcase Huon noted.
We're ready to start experimenting with and rolling out this feature in earnest.
|
@swift-ci please smoke test Linux |
1 similar comment
|
@swift-ci please smoke test Linux |
This would cause spurious idempotency failures with generic signature canonicalization.
DougGregor
force-pushed
the
substitutionmap-conformance-access-path
branch
from
adbed0a
to
ba10723
Compare
|
@swift-ci please smoke test and merge |
1 similar comment
|
@swift-ci please smoke test and merge |
| [&](Type type, ProtocolDecl *proto) -> Optional<ProtocolConformanceRef> { | ||
| // We're working relative to a generic environment, map into that | ||
| // context before looking into the conformance map. | ||
| if (genericEnv) |
There was a problem hiding this comment.
It seems silly to map the type out of context only to map it back in again. What do you think of always using interface types as keys in the conformance map, even for generic environment-derived substitutions?
There was a problem hiding this comment.
I think it's a great idea. What I'd like to do is turn the conformance map into a flat array, arranged according to the protocol requirements in the GenericSignature, the way we do with NormalProtocolConformance and its "signature conformances".
| }); | ||
| } | ||
| auto canonType = genericSig->getCanonicalTypeInContext(type, mod); | ||
| auto accessPath = |
There was a problem hiding this comment.
It seems expensive to go through the canonical GSB to do getCanonicalTypeInContext(), and then feed the result back into the GSB for getConformanceAccessPath(). Can getConformanceAccessPath() instead do the right thing with non-canonical types?
There was a problem hiding this comment.
Actually, getConformanceAccessPath() can work on non-canonical types, because it implicitly canonicalizes when looking up the potential archetype. I think the canonicalization you refer to is unnecessary.
| return recordOrReturn(conformance.getInherited(proto)); | ||
| // If we've hit an abstract conformance, everything from here on out is | ||
| // abstract. | ||
| // FIXME: This may not always be true, but it holds for now. |
There was a problem hiding this comment.
This is of course the infamous SR-3500
|
Looks cool! I'm mostly concerned with making lookupConformance() cheaper. Anything we can do to eliminate redundant canonicalization steps or to memoize more stuff would be good. |
…formance. Fixes a regression introduced when we switched SubstitutionMap::lookupConformance() to conformance access paths.
|
@swift-ci please smoke test and merge |
1 similar comment
|
@swift-ci please smoke test and merge |
|
That last commit should address the corelibs Foundation failure. |
|
@DougGregor Have you tried to perform a build with these changes and enabling the SpecializeGenericSubstitutions in Generics.cpp? It may result in exposing more edge cases which are not addressed by this PR yet. |
Reimplement
SubstitutionMap::lookupConformance()in terms of theConformanceAccessPath, eliminating the ugly "parent map" hack and ensuring that all stages of substitutions are depending on the same core abstractions (GenericSignature,ConformanceAccessPath.While here, remove the error blocking the use of associated type where clauses SE-0142, because we should be able to start experimenting with them in earnest now regardless of language mode.
Fixes rdar://problem/31041997 and rdar://problem/31309863.