-
Notifications
You must be signed in to change notification settings - Fork 10.5k
embedded: fix specialization of associated conformance entries in witness tables #79908
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -959,6 +959,13 @@ namespace { | |
} | ||
|
||
void addAssociatedConformance(const AssociatedConformance &req) { | ||
if (req.getAssociation()->getASTContext().LangOpts.hasFeature(Feature::Embedded) && | ||
!req.getAssociatedRequirement()->requiresClass()) { | ||
// If it's not a class protocol, the associated type can never be used to create | ||
// an existential. Therefore this witness entry is never used at runtime | ||
// in embedded swift. | ||
return; | ||
} | ||
Entries.push_back(WitnessTableEntry::forAssociatedConformance(req)); | ||
} | ||
|
||
|
@@ -1748,6 +1755,14 @@ class AccessorConformanceInfo : public ConformanceInfo { | |
(void)entry; | ||
SILEntries = SILEntries.slice(1); | ||
|
||
if (IGM.Context.LangOpts.hasFeature(Feature::Embedded) && | ||
!requirement.getAssociatedRequirement()->requiresClass()) { | ||
// If it's not a class protocol, the associated type can never be used to create | ||
// an existential. Therefore this witness entry is never used at runtime | ||
// in embedded swift. | ||
return; | ||
} | ||
|
||
auto associate = | ||
ConformanceInContext.getAssociatedType( | ||
requirement.getAssociation())->getCanonicalType(); | ||
|
@@ -1775,7 +1790,10 @@ class AccessorConformanceInfo : public ConformanceInfo { | |
if (IGM.Context.LangOpts.hasFeature(Feature::Embedded)) { | ||
// In Embedded Swift associated-conformance entries simply point to the witness table | ||
// of the associated conformance. | ||
llvm::Constant *witnessEntry = IGM.getAddrOfWitnessTable(associatedConformance.getConcrete()); | ||
ProtocolConformanceRef assocConf = | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is it possible for assocConf to differ from associatedConformance above? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, because |
||
SILWT->getConformance()->getAssociatedConformance(requirement.getAssociation(), | ||
requirement.getAssociatedRequirement()); | ||
llvm::Constant *witnessEntry = IGM.getAddrOfWitnessTable(assocConf.getConcrete()); | ||
auto &schema = IGM.getOptions().PointerAuth | ||
.ProtocolAssociatedTypeWitnessTableAccessFunctions; | ||
Table.addSignedPointer(witnessEntry, schema, requirement); | ||
|
@@ -3734,7 +3752,7 @@ llvm::Value *irgen::emitWitnessTableRef(IRGenFunction &IGF, | |
|
||
// In Embedded Swift, only class-bound wtables are allowed. | ||
if (srcType->getASTContext().LangOpts.hasFeature(Feature::Embedded)) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. agreed. I will change it next time I work on this code |
||
assert(proto->requiresClass()); | ||
ASSERT(proto->requiresClass()); | ||
} | ||
|
||
assert(Lowering::TypeConverter::protocolRequiresWitnessTable(proto) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It might be better to push this check up to SILWitnessVisitor, which already includes this check for example:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will not work because the FragileWitnessTableBuilder needs to pull the witness table entry before returning.