-
Notifications
You must be signed in to change notification settings - Fork 10.6k
[embedded] Fix associated type conformances for specialized witness tables #85644
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
base: main
Are you sure you want to change the base?
[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`
|
@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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You could use either Conformance.getTypeWitness() or entry.getAssociatedTypeWitness().Witness here, but they should be equal.
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.
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).Conformer@t3.swift: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).Conformer@t3.swift:6:8" (struct_type decl="Swift.(file).Int"))
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.
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.
|
Failed Tests (1): |
No description provided.