-
Notifications
You must be signed in to change notification settings - Fork 10.5k
[Completion] Type-check parent closures for local functions #77537
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
Avoid type-checking local decls and macros, the type-checking here should instead be handled by TypeCheckASTNodeAtLocRequest.
Really this applies to any capture, not just `self`. Also refactor to make it clear that parent closures and functions are really the only cases that matter here.
Local functions can capture variables from parent closures, so we need to make sure we type-check parent closures when doing completion in a local function. Ideally we ought to be able to be more selective about the elements of the parent closure that we type-check, but that's a more complex change I'm leaving as future work for now.
@swift-ci please test |
@swift-ci please SourceKit stress 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.
Looks reasonable
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.
LGTM
if (!ctx.CompletionCallback) { | ||
// Type-check any local decls encountered. | ||
for (auto *D : LocalDeclsToTypeCheck) | ||
TypeChecker::typeCheckDecl(D); |
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.
Maybe we should skip adding them to the local decls and macro lists?
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.
Sure, I don't mind
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.
[Sema] NFC: Address review feedback on #77537
Local functions can capture variables from parent closures, so we need to make sure we type-check parent closures when doing completion in a local function. Ideally we ought to be able to be more selective about the elements of the parent closure that we type-check, but that's a more complex change I'm leaving as future work for now.
Resolves #77305