Skip to content

[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

Merged
merged 1 commit into from
Jun 30, 2021

Conversation

ahoppen
Copy link
Member

@ahoppen ahoppen commented Jun 23, 2021

This PR essentially consists 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]

@ahoppen ahoppen requested review from xedin and rintaro June 23, 2021 11:26
@ahoppen
Copy link
Member Author

ahoppen commented Jun 23, 2021

@swift-ci Please smoke test

Copy link
Contributor

@xedin xedin left a 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.

// 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)) {
Copy link
Contributor

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.

@ahoppen ahoppen force-pushed the pr/keypath-completion branch from f09c915 to de87359 Compare June 25, 2021 21:13
// We are completing on the root and need to extract the key path's root
// type.
if (KeyPath->getRootType()) {
BaseType = S.getType(KeyPath->getRootType());
Copy link
Contributor

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.

Copy link
Member Author

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]
@ahoppen ahoppen force-pushed the pr/keypath-completion branch from de87359 to d64b8ec Compare June 25, 2021 21:19
@ahoppen
Copy link
Member Author

ahoppen commented Jun 25, 2021

@swift-ci Please smoke test

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants