[embedded] Fix associated type conformances for specialized witness tables #85644
Conversation
…d with existentials
Don't convert indirect to direct arguments. This is needed to support general existentials in embedded swift.
* `var conformances: ConformanceArray` * `var formalConcreteType: CanonicalType`
aschwaighofer
requested review from
eeckstein,
hborla,
jckarter,
rjmccall,
slavapestov and
xedin
as code owners
|
@swift-ci test |
|
@swift-ci test |
| auto typeWitness = Conformance.getTypeWitness(assocType); | ||
| if (langOpts.hasFeature(Feature::EmbeddedExistentials) && | ||
| SILWT->isSpecialized()) { | ||
| typeWitness = entry.getAssociatedTypeWitness().Witness; |
There was a problem hiding this comment.
This doesn't make sense, why is Conformance.getTypeWitness() returning the wrong thing? Is Conformance is not a SpecializedProtocolConformance? If not, it should be fixed to match up and the conditional removed.
There was a problem hiding this comment.
You could use either Conformance.getTypeWitness() or entry.getAssociatedTypeWitness().Witness here, but they should be equal.
There was a problem hiding this comment.
Conformance.getTypeWitness returns the wrong thing because Conformance is the root conformance not the specialized_conformance.
Example:
protocol PWithAssoc {
associatedtype Assoc
func a() -> Assoc
}
struct Conformer<T> {
var t: T
init(_ t: T) {
self.t = t
}
}
struct Container<T> : PWithAssoc {
var x: Conformer<T>
init(_ t: T) {
self.x = Conformer(t)
}
func a() -> Conformer<T> {
return x
}
}
func test(_ p: PWithAssoc) {
let _ = p.a()
}
test(Container(1))
In this example, we have a specialized SILWitness table:
sil_witness_table shared [specialized] Container<Int>: specialize <Int> (<T> Container<T>: PWithAssoc module t3) {
associated_type Assoc: Conformer<Int>
method #PWithAssoc.a: <Self where Self : PWithAssoc> (Self) -> () -> Self.Assoc : @$e2t39ContainerVyxGAA10PWithAssocA2aEP1a0D0QzyFTWSi_Tg5 // specialized protocol witness for PWithAssoc.a() in conformance Container<A>
}
In the witnesstablebuilder code Conformance is initialized to the root conformance SILWT->getConformance()->getRootConformance():
class WitnessTableBuilderBase {
protected:
IRGenModule &IGM;
SILWitnessTable *SILWT;
const RootProtocolConformance &Conformance;
const ProtocolConformance &ConformanceInContext;
CanType ConcreteType;
std::optional<FulfillmentMap> Fulfillments;
WitnessTableBuilderBase(IRGenModule &IGM, SILWitnessTable *SILWT)
: IGM(IGM), SILWT(SILWT),
Conformance(*SILWT->getConformance()->getRootConformance()),
ConformanceInContext(*mapConformanceIntoContext(SILWT->getConformance()->getRootConformance())),
ConcreteType(Conformance.getDeclContext()
->mapTypeIntoEnvironment(Conformance.getType())
->getCanonicalType()) {}
And that ends up being the normal conformance:
(normal_conformance type="Container<T>" protocol="PWithAssoc" explicit nonisolated
(assoc_type req="Assoc" type="Conformer<T>")
(value req="a()" witness="t3.(file).Container.a()@t3.swift:20:8")
(assoc_conformance type="Self" proto="Copyable"
(builtin_conformance type="Container<T>" protocol="Copyable" synthesized nonisolated))
(assoc_conformance type="Self" proto="Escapable"
(builtin_conformance type="Container<T>" protocol="Escapable" synthesized nonisolated))
(assoc_conformance type="Self.Assoc" proto="Copyable"
(builtin_conformance type="Conformer<T>" protocol="Copyable" synthesized nonisolated))
(assoc_conformance type="Self.Assoc" proto="Escapable"
(builtin_conformance type="Conformer<T>" protocol="Escapable" synthesized nonisolated)))
And then we end up with: (bound_generic_struct_type decl="t3.(file)[email protected]:6:8" (generic_type_param_type depth=0 index=0 name="T" param_kind=type)) Instead of Conformer<Int> for
auto typeWitness = Conformance.getTypeWitness(assocType);
If we used SILWT->getConformance().getTypeWitness(assocType) we would get the the right type (bound_generic_struct_type decl="t3.(file)[email protected]:6:8" (struct_type decl="Swift.(file).Int"))
There was a problem hiding this comment.
Ok, thanks for the investigation. I think the right fix is to not unwrap to get the root conformance there at all. If the witness table is not specialized, it should already be the root conformance. But that might be tricky to untangle so you don't have to do it if you don't want to.
Two more questions. Does this need to be conditional on langOpts.hasFeature(Feature::EmbeddedExistentials) then? Also there's another place where getAssociatedConformance() gets called on the unspecialized root conformance. Is that also wrong?
I'll clean this up some day. ConformanceInContext could probably be removed as well.
There was a problem hiding this comment.
Also there's another place where getAssociatedConformance() gets called on the unspecialized root conformance. Is that also wrong?
Similar solution there (I assume you mean the call below), the embedded code path uses the entry.getAssociatedConformanceWitness() ... It looks like you changed that code path from using SILWT->getConformance()->getAssociatedConformance to what it is now in SIL: Preliminary refactoring of SILWitnessTable::AssociatedConformanceWitness):
ProtocolConformanceRef associatedConformance =
ConformanceInContext.getAssociatedConformance(
requirement.getAssociation(),
requirement.getAssociatedRequirement());
#ifndef NDEBUG
assert(entry.getKind() == SILWitnessTable::AssociatedConformance
&& "sil witness table does not match protocol");
#endif
auto associatedWitness = entry.getAssociatedConformanceWitness();
#ifndef NDEBUG
assert(associatedWitness.Requirement == requirement.getAssociation()
&& "sil witness table does not match protocol");
auto piIndex = PI.getAssociatedConformanceIndex(requirement);
assert((size_t)piIndex.getValue() ==
Table.size() - WitnessTableFirstRequirementOffset &&
"offset doesn't match ProtocolInfo layout");
#endif
if (IGM.Context.LangOpts.hasFeature(Feature::Embedded)) {
// In Embedded Swift associated-conformance entries simply point to the witness table
// of the associated conformance.
ProtocolConformance *assocConf = associatedWitness.Witness.getConcrete();
llvm::Constant *witnessEntry = IGM.getAddrOfWitnessTable(assocConf);
auto &schema = IGM.getOptions().PointerAuth
.ProtocolAssociatedTypeWitnessTableAccessFunctions;
Table.addSignedPointer(witnessEntry, schema, requirement);
return;
}
|
Failed Tests (1): |
3 participants
No description provided.