Extend SILCombiner code to handle existential self concrete type propagation using ProtocolConformanceAnalysis #18109
Conversation
|
@atrick Could you review this one now? |
rajbarik
force-pushed
the
raj-refactor
branch
3 times, most recently
from
0151185 to
7abdfbc
Compare
|
@atrick @slavapestov Any possibility of getting this review done quicker? Thanks. |
| /// This routine replaces the old witness method inst with a new one. | ||
| void SILCombiner::replaceWitnessMethodInst( | ||
| FullApplySite Apply, WitnessMethodInst *WMI, SILBuilderContext &BuilderCtx, | ||
| const CanType &ConcreteType, ProtocolConformanceRef &ConformanceRef) { |
There was a problem hiding this comment.
I think ProtocolConformanceRef should be passed by value.
| // Bail, if this existential has a sole conforming type. If there is a sole | ||
| // confirming type, we defer the determination of concrete type until cast | ||
| // instructions are added in SILCombiner. | ||
| if (hasSoleConformingType) |
There was a problem hiding this comment.
It's strange that you would intentionally construct an invalid ConcreteExistentialInfo, presumably to "fill it in" later. I'll have to review the related patches to understand why you need to do this.
…agation using ProtocolConformanceAnalysis
|
@atrick handled your concern, I think. Can you look at it again? Notably, I removed the InitExistential != null check in isValid(). This seems a redundant check given that we are checking everything else to be not null. Also, for using the same isValid() with SoleConformingType, we may not have an InitExistential. Please let me know more concerns soon. I will commit the other PR based on these changes. |
|
LGTM |
|
@swift-ci Please smoke test |
Extend SILCombiner code to handle existential self concrete type propagation using ProtocolConformanceAnalysis.
Prior PR: