From fc100d24bf6ca26771389602450cf1ffbfff4dbe Mon Sep 17 00:00:00 2001 From: Alex Hoppen Date: Fri, 24 Jan 2025 13:21:50 -0800 Subject: [PATCH] Cache the mapping from compiler to toolchain While at it, also make `Toolchain.path` non-optional and clean up `ToolchainRegistry.init` slightly. --- .../SwiftPMBuildSystem.swift | 2 +- Sources/Diagnose/ReproducerBundle.swift | 14 +++--- Sources/SourceKitLSP/SourceKitLSPServer.swift | 2 +- .../Swift/SwiftLanguageService.swift | 2 +- Sources/ToolchainRegistry/Toolchain.swift | 2 +- .../ToolchainRegistry/ToolchainRegistry.swift | 47 ++++++++++++------- Tests/DiagnoseTests/DiagnoseTests.swift | 2 +- 7 files changed, 40 insertions(+), 31 deletions(-) diff --git a/Sources/BuildSystemIntegration/SwiftPMBuildSystem.swift b/Sources/BuildSystemIntegration/SwiftPMBuildSystem.swift index 1cddb92f..b8407ab1 100644 --- a/Sources/BuildSystemIntegration/SwiftPMBuildSystem.swift +++ b/Sources/BuildSystemIntegration/SwiftPMBuildSystem.swift @@ -425,7 +425,7 @@ package actor SwiftPMBuildSystem: BuiltInBuildSystem { languageIds: [.c, .cpp, .objective_c, .objective_cpp, .swift], dependencies: self.targetDependencies[targetId, default: []].sorted { $0.uri.stringValue < $1.uri.stringValue }, dataKind: .sourceKit, - data: SourceKitBuildTarget(toolchain: toolchain.path.map(URI.init)).encodeToLSPAny() + data: SourceKitBuildTarget(toolchain: URI(toolchain.path)).encodeToLSPAny() ) } targets.append( diff --git a/Sources/Diagnose/ReproducerBundle.swift b/Sources/Diagnose/ReproducerBundle.swift index 72851aef..3ff842c8 100644 --- a/Sources/Diagnose/ReproducerBundle.swift +++ b/Sources/Diagnose/ReproducerBundle.swift @@ -28,14 +28,12 @@ func makeReproducerBundle(for requestInfo: RequestInfo, toolchain: Toolchain, bu atomically: true, encoding: .utf8 ) - if let toolchainPath = toolchain.path { - try toolchainPath.realpath.filePath - .write( - to: bundlePath.appendingPathComponent("toolchain.txt"), - atomically: true, - encoding: .utf8 - ) - } + try toolchain.path.realpath.filePath + .write( + to: bundlePath.appendingPathComponent("toolchain.txt"), + atomically: true, + encoding: .utf8 + ) if requestInfo.requestTemplate == RequestInfo.fakeRequestTemplateForFrontendIssues { let command = "swift-frontend \\\n" diff --git a/Sources/SourceKitLSP/SourceKitLSPServer.swift b/Sources/SourceKitLSP/SourceKitLSPServer.swift index bf86d03f..2c529db1 100644 --- a/Sources/SourceKitLSP/SourceKitLSPServer.swift +++ b/Sources/SourceKitLSP/SourceKitLSPServer.swift @@ -567,7 +567,7 @@ package actor SourceKitLSPServer { logger.log( """ - Using toolchain at \(toolchain.path?.description ?? "") (\(toolchain.identifier, privacy: .public)) \ + Using toolchain at \(toolchain.path.description) (\(toolchain.identifier, privacy: .public)) \ for \(uri.forLogging) """ ) diff --git a/Sources/SourceKitLSP/Swift/SwiftLanguageService.swift b/Sources/SourceKitLSP/Swift/SwiftLanguageService.swift index c6cdee74..bfe2c849 100644 --- a/Sources/SourceKitLSP/Swift/SwiftLanguageService.swift +++ b/Sources/SourceKitLSP/Swift/SwiftLanguageService.swift @@ -237,7 +237,7 @@ package actor SwiftLanguageService: LanguageService, Sendable { } else if let clientPlugin = toolchain.sourceKitClientPlugin, let servicePlugin = toolchain.sourceKitServicePlugin { pluginPaths = PluginPaths(clientPlugin: clientPlugin, servicePlugin: servicePlugin) } else { - logger.fault("Failed to find SourceKit plugin for toolchain at \(toolchain.path?.path ?? "nil")") + logger.fault("Failed to find SourceKit plugin for toolchain at \(toolchain.path.path)") pluginPaths = nil } self.sourcekitd = try await DynamicallyLoadedSourceKitD.getOrCreate( diff --git a/Sources/ToolchainRegistry/Toolchain.swift b/Sources/ToolchainRegistry/Toolchain.swift index d39ec1c0..29a7f955 100644 --- a/Sources/ToolchainRegistry/Toolchain.swift +++ b/Sources/ToolchainRegistry/Toolchain.swift @@ -77,7 +77,7 @@ public final class Toolchain: Sendable { /// The path to this toolchain, if applicable. /// /// For example, this may be the path to an ".xctoolchain" directory. - package let path: URL? + package let path: URL // MARK: Tool Paths diff --git a/Sources/ToolchainRegistry/ToolchainRegistry.swift b/Sources/ToolchainRegistry/ToolchainRegistry.swift index 1bcc32e3..122fed43 100644 --- a/Sources/ToolchainRegistry/ToolchainRegistry.swift +++ b/Sources/ToolchainRegistry/ToolchainRegistry.swift @@ -11,6 +11,7 @@ //===----------------------------------------------------------------------===// import Dispatch +import SKLogging import SwiftExtensions import TSCExtensions @@ -68,10 +69,14 @@ package final actor ToolchainRegistry { private let toolchainsByIdentifier: [String: [Toolchain]] /// The toolchains indexed by their path. - /// - /// Note: Not all toolchains have a path. private let toolchainsByPath: [URL: Toolchain] + /// Map from compiler paths (`clang`, `swift`, `swiftc`) mapping to the toolchain that contained them. + /// + /// This allows us to find the toolchain that should be used for semantic functionality based on which compiler it is + /// built with in the `compile_commands.json`. + private let toolchainsByCompiler: [URL: Toolchain] + /// The currently selected toolchain identifier on Darwin. package let darwinToolchainOverride: String? @@ -98,29 +103,40 @@ package final actor ToolchainRegistry { var toolchainsAndReasons: [(toolchain: Toolchain, reason: ToolchainRegisterReason)] = [] var toolchainsByIdentifier: [String: [Toolchain]] = [:] var toolchainsByPath: [URL: Toolchain] = [:] + var toolchainsByCompiler: [URL: Toolchain] = [:] for (toolchain, reason) in toolchainsAndReasonsParam { // Non-XcodeDefault toolchain: disallow all duplicates. - if toolchain.identifier != ToolchainRegistry.darwinDefaultToolchainIdentifier { - guard toolchainsByIdentifier[toolchain.identifier] == nil else { - continue - } + if toolchainsByIdentifier[toolchain.identifier] != nil, + toolchain.identifier != ToolchainRegistry.darwinDefaultToolchainIdentifier + { + logger.error("Found two toolchains with the same identifier: \(toolchain.identifier)") + continue } - // Toolchain should always be unique by path if it is present. - if let path = toolchain.path { - guard toolchainsByPath[path] == nil else { - continue - } - toolchainsByPath[path] = toolchain + // Toolchain should always be unique by path. + if toolchainsByPath[toolchain.path] != nil { + logger.fault("Found two toolchains with the same path: \(toolchain.path)") + continue } + toolchainsByPath[toolchain.path] = toolchain toolchainsByIdentifier[toolchain.identifier, default: []].append(toolchain) + + for case .some(let compiler) in [toolchain.clang, toolchain.swift, toolchain.swiftc] { + guard toolchainsByCompiler[compiler] == nil else { + logger.fault("Found two toolchains with the same compiler: \(compiler)") + continue + } + toolchainsByCompiler[compiler] = toolchain + } + toolchainsAndReasons.append((toolchain, reason)) } self.toolchainsAndReasons = toolchainsAndReasons self.toolchainsByIdentifier = toolchainsByIdentifier self.toolchainsByPath = toolchainsByPath + self.toolchainsByCompiler = toolchainsByCompiler if let darwinToolchainOverride, !darwinToolchainOverride.isEmpty, darwinToolchainOverride != "default" { self.darwinToolchainOverride = darwinToolchainOverride @@ -267,12 +283,7 @@ package final actor ToolchainRegistry { /// If we have a toolchain in the toolchain registry that contains the compiler with the given URL, return it. /// Otherwise, return `nil`. package func toolchain(withCompiler compiler: URL) -> Toolchain? { - for toolchain in toolchains { - if [toolchain.clang, toolchain.swift, toolchain.swiftc].contains(compiler) { - return toolchain - } - } - return nil + return toolchainsByCompiler[compiler] } } diff --git a/Tests/DiagnoseTests/DiagnoseTests.swift b/Tests/DiagnoseTests/DiagnoseTests.swift index 31683e5e..4c18636a 100644 --- a/Tests/DiagnoseTests/DiagnoseTests.swift +++ b/Tests/DiagnoseTests/DiagnoseTests.swift @@ -239,7 +239,7 @@ private func assertReduceSourceKitD( let (markers, fileContents) = extractMarkers(markedFileContents) let toolchain = try await unwrap(ToolchainRegistry.forTesting.default) - logger.debug("Using \(toolchain.path?.description ?? "") to reduce source file") + logger.debug("Using \(toolchain.path.description) to reduce source file") let markerOffset = try XCTUnwrap(markers["1️⃣"], "Failed to find position marker 1️⃣ in file contents")