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
[Sema/SILGen/IRGen/StdLib] Implement metatype keypaths #73242
base: main
Are you sure you want to change the base?
Conversation
a1b2bb5
to
0ec01dc
Compare
// CHECK: true | ||
print(keyPath3FromLibB == keyPath3FromLibC) | ||
// CHECK: true | ||
print(keyPath3FromLibB != keyPath4FromLibC) |
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.
@amritpan I think we need a lot more tests. Could you please add tests that access static properties and subscripts to test/SILGen/keypaths.swift
, we also need chained access tests in Sema and in SILGen and interpreter tests for all of the above that make sure that changes behave as expected at runtime.
_ = \X.a // expected-error{{static member 'a' cannot be used on instance of type 'X'}} | ||
_ = \X.Type.a | ||
_ = \X.b // expected-error{{static member 'b' cannot be used on instance of type 'X'}} | ||
_ = \X.Type.b |
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.
We need a few tests here that flip the situation around and try to access instance member (property or subscript) on a metatype base.
lib/SIL/Verifier/SILVerifier.cpp
Outdated
@@ -7192,6 +7192,8 @@ void SILProperty::verify(const SILModule &M) const { | |||
auto sig = dc->getGenericSignatureOfContext(); | |||
auto baseTy = dc->getInnermostTypeContext()->getSelfInterfaceType() | |||
->getReducedType(sig); | |||
if (decl->isStatic()) | |||
baseTy = CanMetatypeType::get(baseTy); |
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.
Since retrieval of the base type is not as straightforward anymore I think it should either become a property of SILProperty
or we should add a method on SILProperty i.e. CanType SILProperty::getBaseType() const
to compute it internally.
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.
I added CanType SILProperty::getBaseType() const
to SILVerifier.cpp above ::verify
and don't think that is the place for it but am not sure where else it should go. Perhaps at the end of SILType.cpp?
a3ba657
to
bc8d04b
Compare
bc8d04b
to
71bc3a6
Compare
This is the implementation for SE-NNNN which extends keypath usage to include references to static properties,
eg. metatype keypaths: