-
Notifications
You must be signed in to change notification settings - Fork 10.4k
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
Make Self available to member functions (SE-0068?) #22863
Conversation
07e7c58
to
c27f79e
Compare
You'll have to get this working in type context as well, eg |
Eh? I don’t understand. The error is as before:
The patch leaves what works already alone and adds some new contexts where Self is valid. |
If works inside the function body for a struct but I don’t understand why there would be a use case for Self as a return type for a struct type. The following works:
Is it important that this works for a struct which cannot inherit?
I don’t see this mentioned in the proposal. |
OK, I can see where it is mentioned in the review decision. Seems pointless to extend Self to return types of member functions of value types to me. We're getting into diminishing returns. |
@johnno1962 It's not pointless. With generics it allows you to avoid re-stating a complex type. |
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 know you were calling this a "cheeky" implementation at first but this is starting to look real. I think my main remaining concern is how the code is located inside diagnostic paths. If we find a better place for it to live it would be great.
This is great! Does this manage to make |
|
Thanks for trying. This is great progress nonetheless! |
Wrapping up for the day. I can get TypeExpr working but I'm not seeing overloaded methods being called when they are messaged on Self so I've given up on using a TypeExpr for now.
With Joe's
Which implies the type has this property:
Fixable or can we only improve the error message? |
The problem with DynamicSelfType should be fixable. I'll take a look at it. |
Hi @slavapestov, managed to get a TypeExpr working including overrides with the following code: Type SelfType = DC->getInnermostTypeContext()->getSelfInterfaceType();
if (SelfType->is<ClassType>() || SelfType->is<BoundGenericClassType>())
SelfType = DynamicSelfType::get(SelfType, Context);
SelfType = DC->mapTypeIntoContext(SelfType);
return new (Context) TypeExpr(TypeLoc(new (Context)
FixedTypeRepr(SelfType, Loc), SelfType)); Is |
143b7f9
to
82989fb
Compare
82989fb
to
c703a22
Compare
@johnno1962 Rather than testing the type of the DeclContext you can more directly check if its a class or class extension by calling |
Thanks, I've updated the tests in the last commit. This has to be fairly close now. |
Co-Authored-By: johnno1962 <github@johnholdsworth.com>
Over to you @slavapestov. Thanks for your help! Beware, there is a merge commit d3e2550 on CHANGELOG.md which may complicate cherry picking to 5.1 |
I think this is good enough to merge. We can iterate on the details later (the local function case is probably going to be rare anyway). |
@swift-ci Please test |
@swift-ci Please test source compatibility |
Build failed |
Build failed |
Thank you for your contribution! |
Before I forget -- can you submit a PR to the swift-evolution repo marking this proposal as implemented, and file a JIRA to implement the remaining case with local functions? |
@johnno1962 I've updated the proposal status to Implemented (Swift 5.1) |
There is some code mentioned on the forum that doesn't compile:
|
Can I leave it to you to arrange the cherry-pick to swift-5.1-branch? |
Local functions should work though I think dc->getParent()->isLocalContext() doesn't work. |
@johnno1962 There's no need to cherry-pick. The final branching for |
Just in time 🙂 |
Another case that would be nice to eventually support is the use of class C {
func foo(_ fn: (Self) -> Void) {}
} Filed SR-10121 to track that :) |
@johnno1962, are you missing a I agree that if |
I tried with a final as you can see but it didn’t fix it. There will be corner cases to iron out. |
Another snippet of code that is not supported:
Trying to override copied gives:
|
Hi Apple,
A cheeky implementation of SE-0068 that intercepts the error message
use of unresolved identifier 'Self'; did you mean 'self'?
and makesSelf
a synonym fortype(of: self)
when it has no other meaning. Works surprisingly well and only required a change to an error message in one test to have them pass.Resolves SR-1340.