[CodeCompletion] Some refactorings surrounding solver-based completion #41885
Conversation
ahoppen
changed the title
ahoppen
force-pushed
the
pr/refactor-solver-completion
branch
from
3011ff6 to
fe46180
Compare
|
@swift-ci Please smoke test |
|
@swift-ci Please SourceKit stress test |
… completion callbacks
…all solver-based completion kinds
ahoppen
force-pushed
the
pr/refactor-solver-completion
branch
from
fe46180 to
03d819f
Compare
|
@swift-ci Please smoke test |
| // func foo(fn: () async -> Void) | ||
| // foo { <HERE> } | ||
| // In this case, the closure is wrapped with a 'FunctionConversionExpr' | ||
| // which has 'async' function type. |
There was a problem hiding this comment.
This assume a type-checked version of the AST, is that always the case?
There was a problem hiding this comment.
I believe so. The call side inside CompletionLookup is intended for the legacy code completion, which works on a type-checked AST and https://github.com/apple/swift/pull/41885/files/03d819f442f05b58f6a2b1816b2e280292db5329#diff-3088e637c4f06d0fc6cb38370068cd6fa6fe8dcaa10ec35c9086602c59eaba17R149 works on the already type-checked part of the AST to determine async-ness after failing to determine async-ness from the solution.
There was a problem hiding this comment.
Sounds good, just double-checking! :)
| /// Called for each solution produced while type-checking an expression | ||
| /// that the code completion expression participates in. | ||
| virtual void sawSolution(const constraints::Solution &solution) { | ||
| GotCallback = true; |
There was a problem hiding this comment.
This looks suspicious - there is no way to ensure that each override calls TypeCheckCompletionCallback::sawSolution(solution) internally. I see that they currently do but it only takes one to go unnoticed... I'd like to suggest virtual to be removed from this method, and instead be used on a protected sawSolutionImpl(const Solution &) which is going to be called from this one.
There was a problem hiding this comment.
Sounds like a good idea. I’ll do that.
…mpl from subclasses This eliminates a source of bugs if subclasses of `TypeCheckCompletionCallback` forget to call the superclass’s implementation of `sawSolution` from their overridden method.
|
@swift-ci Please smoke test |
Perform a couple of refactoring for solver-based completion, with the goal of sharing more code between the different completion kinds:
TypeCheckCompletionCallbacksfrom Sema to IDEUnresolvedTypeand some had fewer fallback mechanisms to retrieve the type, have one implementation that does everythingExprCompletionchecked whether the surrounding context is async, I don’t have any failing test cases for the other completion kinds but it definitely makes sense for them to share the same logic to determine whether the surrounding context is async