Store the conforming type within an abstract ProtocolConformanceRef #80225
Conversation
DougGregor
requested review from
xymus,
AnthonyLatsis,
hborla,
slavapestov,
xedin and
rjmccall
as code owners
|
@swift-ci please smoke test |
| @@ -158,6 +178,9 @@ class ProtocolConformanceRef { | |||
| return ProtocolConformanceRef(UnionType::getFromOpaqueValue(value)); | |||
| } | |||
|
|
|||
| /// Retrieve the conforming type. | |||
| Type getConformingType() const; | |||
There was a problem hiding this comment.
ProtocolConformance just calls this getType(), lets use that name
| @@ -158,6 +178,9 @@ class ProtocolConformanceRef { | |||
| return ProtocolConformanceRef(UnionType::getFromOpaqueValue(value)); | |||
| } | |||
|
|
|||
| /// Retrieve the conforming type. | |||
| Type getConformingType() const; | |||
|
|
|||
| /// Return the protocol requirement. | |||
| ProtocolDecl *getRequirement() const; | |||
There was a problem hiding this comment.
And if you rename this to getProtocol(), again for consistency with ProtocolConformance, I will forever be grateful
| @@ -4533,7 +4533,7 @@ void ASTMangler::appendAnyProtocolConformance( | |||
| return; | |||
|
|
|||
| auto path = genericSig->getConformancePath(conformingType, | |||
| conformance.getAbstract()); | |||
There was a problem hiding this comment.
Let's add a FIXME here to remind ourselves that the conformingType parameter to this function is no longer necessary
| @@ -293,25 +301,40 @@ ProtocolConformanceRef::getAssociatedConformance(Type conformingType, | |||
| conformingType->is<UnresolvedType>() || | |||
| conformingType->is<PlaceholderType>()); | |||
|
|
|||
| return ProtocolConformanceRef(protocol); | |||
| return ProtocolConformanceRef::forAbstract(conformingType, protocol); | |||
There was a problem hiding this comment.
Oh I forgot about this spot!
There was a problem hiding this comment.
The above conformingType assert should eventually be in forAbstract() itself -- but maybe not yet
There was a problem hiding this comment.
Yeah, I want to get the refactoring in place first, then crank up the assertions until we can start simplifying.
| @@ -5379,7 +5379,11 @@ static void ensureRequirementsAreSatisfied(ASTContext &ctx, | |||
|
|
|||
| auto outerIsolation = conformance->getIsolation(); | |||
| ProtocolConformanceRef(assocConf).forEachIsolatedConformance( | |||
| [&](ProtocolConformance *isolatedConformance) { | |||
| [&](ProtocolConformanceRef isolatedConformanceRef) { | |||
| if (!isolatedConformanceRef.isConcrete()) | |||
There was a problem hiding this comment.
Can this be a pack conformance?
There was a problem hiding this comment.
No, we look through them for this callback. I'll be back in this code shortly.
|
|
||
| // Figure out which arena this should go in. | ||
| RecursiveTypeProperties properties; | ||
| if (conformingType) |
There was a problem hiding this comment.
Where are the remaining callers that pass a null Type?
There was a problem hiding this comment.
I don't know of any, but I want the assertions in place before I take this out.
|
@swift-ci Please test source compatibility |
| @@ -2117,6 +2117,12 @@ namespace decls_block { | |||
| BCFixed<2> // the builtin conformance kind | |||
| >; | |||
|
|
|||
| using AbstractConformanceLayout = BCRecordLayout< | |||
There was a problem hiding this comment.
You forgot to bump the module format
There was a problem hiding this comment.
My local tree was out of date and I bumped it in exactly the same way you did in #80107 :)
|
We got no signal from the macOS run due to other issues at the time. A commit with the (new) module format bump + addressing Slava's review comments is coming shortly. |
DougGregor
requested review from
eeckstein,
jckarter and
tshortli
as code owners
DougGregor
force-pushed
the
conforming-type-in-abstract-conformance
branch
from
3a8875a to
ed1e683
Compare
|
@swift-ci please smoke test |
|
@swift-ci please test source compatibility |
An "abstract" ProtocolConformanceRef is a conformance of a type parameter or archetype to a given protocol. Previously, we would only store the protocol requirement itself---but not track the actual conforming type, requiring clients of ProtocolConformanceRef to keep track of this information separately. Record the conforming type as part of an abstract ProtocolConformanceRef, so that clients will be able to recover it later. This is handled by a uniqued AbstractConformance structure, so that ProtocolConformanceRef itself stays one pointer. There remain a small number of places where we create an abstract ProtocolConformanceRef with a null type. We'll want to chip away at those and establish some stronger invariants on the abstract conformance in the future.
|
Source compatibility failures were UPASS, ignoring. |
DougGregor
force-pushed
the
conforming-type-in-abstract-conformance
branch
from
ed1e683 to
731f584
Compare
|
@swift-ci please smoke test |
An "abstract" ProtocolConformanceRef is a conformance of a type parameter or archetype to a given protocol. Previously, we would only store the protocol requirement itself---but not track the actual conforming type, requiring clients of ProtocolConformanceRef to keep track of this information separately.
Record the conforming type as part of an abstract ProtocolConformanceRef, so that clients will be able to recover it later. This is handled by a uniqued AbstractConformance structure, so that ProtocolConformanceRef itself stays one pointer.
There remain a small number of places where we create an abstract ProtocolConformanceRef with a null type. We'll want to chip away at those and establish some stronger invariants on the abstract conformance in the future.