VS Code spells file paths with a lowercase drive letter, while the rest of Windows APIs use an uppercase drive letter. Normalize the drive letter spelling to be uppercase.
Fixes#1855
rdar://141001203
As the user types, we filter the code completion results. Cancelling the completion request on every keystroke means that we will never build the initial list of completion results for this code completion session if building that list takes longer than the user's typing cadence (eg. for global completions) and we will thus not show any completions.
Adding an item to `AsyncQueue` was linear in the number of pending queue items, thus adding n items to an `AsyncQueue` before any can execute is in O(n^2). This decision was made intentionally because the primary use case for `AsyncQueue` was to track pending LSP requests, of which we don’t expect to have too many pending requests at any given time.
While we can't fix the quadratic performance issue in general, we can resolve the quadratic issue of `AsyncQueue<Serial>` by making a new task only depend on the last item in the queue, which then transitively depends on all the previous items. `AsyncQueue<Serial>` are the queues that are most likely to contain many items.
Fixes#1725
rdar://137886469
We made quite a few fixes recently to make sure that path handling works correctly using `URL` on Windows. Use `URL` in most places to have a single type that represents file paths instead of sometimes using `AbsolutePath`.
While doing so, also remove usages of `TSCBasic.FileSystem` an `InMemoryFileSystem`. The pattern of using `InMemoryFileSystem` for tests was never consistently used and it was a little confusing that some types took a `FileSystem` parameter while other always assumed to work on the local file system.
When `TestSourceKitLSPClient.nextNotification` timed out, it would cancel `AsyncSequence.Iterator.next()` and thus also cancel the underlying `AsyncStream` (which I only now found out). This means that calling `nextNotification` again would never return any notification. Hence, if we didn’t receive a `PublishDiagnosticsNotification` within the first timeout, we would never get one.
`URL.path` returns forward slashes in the path on Windows (https://github.com/swiftlang/swift-foundation/issues/973) where we expect backslashes. Work around that by defining our own `filePath` property that is backed by `withUnsafeFileSystemRepresentation`, which produces backslashes.
rdar://137963660
When we receive build settings after hitting the timeout, we call `fileBuildSettingsChanged` on the delegate, which should cause the document to get re-opened in sourcekitd and diagnostics to get refreshed.
rdar://136332685
Fixes#1693
Otherwise, the body task might finish before we yield the `TimeoutError` and thus `withTimeout` would not actually throw a `TimeoutError`.
rdar://137171114
I saw a nondeterministic test failure of `AsyncUtilsTests.testWithTimeoutEscalatesPriority` and I believe this was the cause.
Also refactor a few test helper functions on the way.
Previously, when we hit the timeout in `withTimeout`, we just cancelled `body` but relied on cooperative cancellation before the caller could continue. Since the caller is obviously no longer interested in the operation, we can return with `CancellationError` immediately.
Fixes#883
rdar://116705684
Adding an item to `AsyncQueue<Serial>` is linear in the number of pending queue items, thus adding n items to an `AsyncQueue` before any can execute is in O(n^2). This decision was made intentionally because the primary use case for `AsyncQueue` was to track pending LSP requests, of which we don’t expect to have too many pending requests at any given time.
`SourceKitIndexDelegate` was also using `AsyncQueue` to track the number of pending units to be processed and eg. after indexing SourceKit-LSP, I have seen this grow up to ~20,000. With the quadratic behavior, this explodes time-wise.
Turns out that we don’t actually need to use a queue here at all, an atomic is sufficient and much faster.
Independently, we should consider mitigating the quadratic behavior of `AsyncQueue<Serial>` or `AsyncQueue` in general.
When we receive build settings after hitting the timeout, we call `fileBuildSettingsChanged` on the delegate, which should cause the document to get re-opened in sourcekitd and diagnostics to get refreshed.
rdar://136332685
`Workspace` is responsible for creating the `BuildSystemManager` and responds to most of the delegate calls. It should thus also be the delegate of `BuildSystemManager`.
We currently load the entire package before generating a `SwiftPMBuildSystem`. That means that the initialize request to SourceKit-LSP is blocked until the package has been loaded, preventing us from offering any sort of functionality, including syntactic functionality like formatting.
Decouple build system creation and build graph generation (aka. package loading for SwiftPM). We can operate with fallback build settings until the build graph has been loaded and reopen the document once the proper build settings are available.
rdar://126644596
This works around https://github.com/apple/swift-corelibs-foundation/issues/5041, which caused a null byte to be incorrectly added after the user name's home directory, causing, among others, directory iteration to recurse indefinitely, always adding another level of the user's name.
Change a l public declarations to the `package` access level, accept for:
- The `LanguageServerProtocol` module
- The `BuildServerProtocol` module
- `InProcessClient.InProcessSourceKitLSPClient`
- `LanguageServerProtocolJSONRPC` (I would like to create a more ergonomic API for this like `InProcessSourceKitLSPClient` in the future, but for now, we’ll leave it public)
Unfortunately, our pattern of marking functions as `@_spi(Testing) public` no longer works with the `package` access level because declarations at the `package` access level cannot be marked as SPI. I have decided to just mark these functions as `package`. Alternatives would be:
- Add an underscore to these functions, like we did for functions exposed for testing before the introduction of `SPI`
- Use `@testable` import in the test targets and mark the methods as `internal`
Resolves#1315
rdar://128295618
This patch fixes up:
- 1 File 'Foo.swift' is part of module 'module'; ignoring import
warning
- 70 "Instance method 'foo' cannot be used in an '@inlinable'
function because 'module' was not imported by this file" warnings
- 2 "Result of call 'foo' is unused" warnings
The purpose of the different modules wasn’t clearly defined, which lead to inconsistent responsibilities between the different modules. Define each module’s purpose and move a few files between modules to satisfy these definitions.
There are a few more larger changes that will need to be made for a fully consistent module structure. These are FIXMEs in the new Modules.md document and I’ll address them in follow-up PRs.
Unfortunately, `setpriority` only allows reduction of a process’s priority and doesn’t support priority elevation (unless you are a super user). I still think that it’s valuable to set the process’s priority based on the task priority when it is launched because many indexing processes never get their priority escalated and should thus run in the background.
On Windows, we can elevate the process’s priority.
rdar://127474245
