-
Notifications
You must be signed in to change notification settings - Fork 10.5k
[CodeCompletion] Migrate key path completion to be solver based #38049
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
@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.
I have left one comment inline, otherwise solver changes look good to me.
lib/Sema/CSSimplify.cpp
Outdated
// won't get any more information out of it in subsequent passes - its | ||
// result is always a type variable with no constraints on the preceeding | ||
// components. | ||
if (flags.contains(TMF_GenerateConstraints)) { |
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.
flags
doesn't have any relation to constraint generation phase, you'd need to check getPhase() == ConstraintSystemPhase::ConstraintGeneration
for that. TMF_GenerateConstraints
is used in cases were it's okay to add new constraints while solving current one e.g. simplification logic sometimes has to generate new identical constraint if current one can't be simplified, matchTypes
has to add new constraints when it's matching two bound generic types in order to match generic generic arguments etc.
f09c915
to
de87359
Compare
lib/Sema/TypeCheckCodeCompletion.cpp
Outdated
// We are completing on the root and need to extract the key path's root | ||
// type. | ||
if (KeyPath->getRootType()) { | ||
BaseType = S.getType(KeyPath->getRootType()); |
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'd say you want to use getResolvedType
here since root could be a generic 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.
Thanks. Changed it.
This commit essentially consistes of the following steps: - Add a new code completion key path component that represents the code completion token inside a key path. Previously, the key path would have an invalid component at the end if it contained a code completion token. - When type checking the key path, model the code completion token’s result type by a new type variable that is unrelated to the previous components (because the code completion token might resolve to anything). - Since the code completion token is now properly modelled in the constraint system, we can use the solver based code completion implementation and inspect any solution determined by the constraint solver. The base type for code completion is now the result type of the key path component that preceeds the code completion component. This resolves bugs where code completion was not working correctly if the key path’s type had a generic base or result type. It’s also nice to have moved another completion type over to the solver-based implementation. Resolves rdar://78779234 [SR-14685] and rdar://78779335 [SR-14703]
de87359
to
d64b8ec
Compare
@swift-ci Please smoke test |
This PR essentially consists of the following steps:
This resolves bugs where code completion was not working correctly if the key path’s type had a generic base or result type. It’s also nice to have moved another completion type over to the solver-based implementation.
Resolves rdar://78779234 [SR-14685] and rdar://78779335 [SR-14703]