-
Notifications
You must be signed in to change notification settings - Fork 10.5k
[CS] Consolidate logic forming locators to callees #24826
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
Conversation
This commit adds `ConstraintSystem::getCalleeLocator`, which forms a locator that describes the callee of a given expression. This function is then used to replace various places where this logic is duplicated. This commit also changes the conditions under which a ConstructorMember callee locator is formed. Previously it was formed for a CallExpr with a TypeExpr function expr. However, now such a locator is formed if the function expr is of AnyMetatypeType. This allows it to be more lenient with invalid code, as well as work with DotSelfExpr. Resolves SR-10694.
Currently we check that the fixes share the same anchor, however this doesn't account for the case where, given an ApplyExpr, one fix is anchored on its function expr, and another is anchored on the apply itself (because the former fix might be looking at the callee's requirements, and the latter fix might be looking at an argument of the call). This commit changes the logic such that we check that fixes share the same callee locator, which covers the above case. In addition, now that we have the callee locator, we can use this to find the overload directly.
This helps clean up a bunch of call sites.
cc. @xedin |
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.
Nice!
@swift-ci Please smoke test |
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.
Great, thanks! That's exactly the refactoring I wanted to do when I had some time, I have also talked about this with @sl, we have to many of these checks already in CSDiagnostics.
Thanks for pointing this out @xedin. This looks awesome! Glad to see this ended up getting cleaned up even sooner than expected 🎉 |
This PR adds
ConstraintSystem::getCalleeLocator
, which forms a locator that describes the callee of a given expression. This function is then used to replace various places where this logic is duplicated, and in doing so improves a couple of diagnostics due to catching cases that were previously missed. Most notably, it's now more lenient when forming aConstructorMember
locator for an apply. Previously such a locator was only formed for an apply of aTypeExpr
, however now it's formed for an apply of an expr ofAnyMetatypeType
. This better matches the logic insimplifyApplicableFnConstraint
and allows us to better diagnose things liketype(of: x)(...)
.getCalleeLocator
is then used when checking if we can diagnose an ambiguity with fixes, which allows us to have fixes anchored to both the apply and to its function expr (where the former could be looking at an argument of the apply, and the latter could be looking at a requirement of the callee).Finally, this PR adds a convenience
getDeclOrNull
function toOverloadChoice
, and fixes a simple compiler crasher. Given how straightforward these changes were, I didn't feel it was worth splitting them into a separate PR.Resolves SR-10694 and SR-10696.