Skip to content

Conversation

@ahoppen
Copy link
Member

@ahoppen ahoppen commented May 23, 2024

In SourceKit-LSP, we can get into the following situation:

  1. We open A.swift
  2. We issue a request for A.swift, the request takes a while to execute
  3. The dependencies of A.swift are updated, which causes us to reopen the document in sourcekitd, so that the AST is rebuilt
  4. This shouldn’t cause the request from (2) to be cancelled. We should continue executing it and only re-open the document after the request from (2) has finished

While at it, also make sure that we send syntacticOnly when re-opening a document.

rdar://127475366

ahoppen added 3 commits May 23, 2024 13:41
In SourceKit-LSP, we can get into the following situation:
1. We open A.swift
2. We issue a request for A.swift, the request takes a while to execute
3. The dependencies of A.swift are updated, which causes us to reopen the document in sourcekitd, so that the AST is rebuilt
4. This shouldn’t cause the request from (2) to be cancelled. We should continue executing it and only re-open the document after the request from (2) has finished

rdar://127475366
@ahoppen ahoppen requested review from bnbarham and hamishknight May 23, 2024 21:01
@ahoppen ahoppen requested a review from benlangmuir as a code owner May 23, 2024 21:01
@ahoppen
Copy link
Member Author

ahoppen commented May 23, 2024

@swift-ci Please test

keys.enableDiagnostics: 0,
keys.syntacticOnly: 1,
keys.compilerArgs: compileCommand?.compilerArgs as [SKDRequestValue]?,
keys.cancelBuilds: 0,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be set for the close rather than the open, right?

@ahoppen ahoppen merged commit 3300c77 into swiftlang:main May 24, 2024
@ahoppen ahoppen deleted the dont-cancel-sourcekitd-request-on-close branch May 24, 2024 15:28
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