The idea is pretty simple: When `MemberImportVisibility` is enabled, we know that imports can only affect the current source file. So, we can just try and remove every single `import` declaration in the file, check if a new error occurred and if not, we can safely remove it.
This allows us to easily get rid of some `@_inheritActorContext`. The others seem to be a little more tricky and I haven’t spent too much time at trying to figure out how to remove the attribute from those.
According to https://developer.apple.com/documentation/foundation/processinfo/activeprocessorcount
> Whereas the processorCount property reports the number of advertised processing cores, the activeProcessorCount property reflects the actual number of active processing cores on the system. There are a number of different factors that may cause a core to not be active, including boot arguments, thermal throttling, or a manufacturing defect.
For short-lived workloads like `concurrentMap` we want to parallelize across the number of cores that are currently active, so use `activeProcessorCount` instead. The only case where we want to continue using `processorCount` is the computation of concurrent tasks for `TaskScheduler` because the value is stored for the lifetime of the SourceKit-LSP process and we don’t want to limit parallelism if SourceKit-LSP was launched during a time of thermal throttling.
I stumbled across this while working on #2302
`containingXCToolchain` loops infinitely when given eg. `C:\foo\..\bar`.
The underlying cause is that `deletingLastPathComponent` does not remove
the `..` on Windows. On macOS it's potentially worse - it *adds* `..`.
We don't see the infinite loop on macOS/Linux though because
`AbsolutePath` already removes them (which is not the case on Windows).
Resolves#2174.
Not sure exactly what paths are triggering this on Windows, as all the
logs in #2174 are within `C:\`. But the stacktrace seems to strongly
imply that we have an infinite loop here, so let's see if switching to
the native check fixes this now that
https://github.com/swiftlang/swift-foundation/issues/976 is in.
Resolves#2174
This should be a last stop-gap measure in case sourcekitd or clangd get stuck, don’t respond to any requests anymore and don’t honor cancellation either. In that case we can restore SourceKit-LSP behavior by killing them and using the crash recovery logic to restore functionality.
rdar://149492159
We were launching the task that watches for the priority change with `high` priority. If the base priority was `medium`, this should immediately report a priority change to `high`. It appears the only reason why this doesn’t happen right now is due to rdar://147868544.
When SourceKit-LSP is shut down, we should make sure that we don’t leave behind child processes, which will become orphans after SourceKit-LSP has terminated. What’s worse, when SourceKit-LSP has exited, these processes might not have any process to read their stdout/stderr, which can lead to them running indefinitely.
This change does not cover the termination of subprocess trees. For example, if we launch `swift build` and need to kill it because it doesn’t honor SIGINT, its child processes will still live on. Similarly, if we kill a BSP server, its child processes might live on. Fixing this is a drastically bigger endeavor, likely requiring changes to Foundation and/or TSC. I filed https://github.com/swiftlang/sourcekit-lsp/issues/2080 for it.
We would hit quadratic behavior in `AsyncQueue` when the build system floods us with `build/logMessage` or `build/task(Start|Progress|Finish)` notifications because we record a dependency on all of the pending log message handling tasks.
We can extend the improvement made in https://github.com/swiftlang/sourcekit-lsp/pull/1840 to fix this quadratic problem: If the current task depends on a task with metadata that depends on itself (ie. all tasks of metadata that needs to be executed in-order), we only need to depend on the last task with that metadata.
This covers many message types and we can now only get into quadratic behavior if we get flooded with two different kinds of messages: One that does not have a self-dependency and one that depends on the message without a self-dependency. In terms of LSP messages, this could be a document read followed by a document update, but I think this is a lot more unlikely than getting spammed with one type of message.
If a source file is part of multiple targets, we should index it in the context of all of those targets because the different targets may produce different USRs since they might use different build settings to interpret the file.
I feel like the implementations are actually simpler if we split them. This will also allow us to add more advanced logic to the JSON compilation database build system in the future, such as inferring the toolchain from the compile command.
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