Merge pull request #1953 from ahoppen/clean-up-toolchain

Cache the mapping from compiler to toolchain

Author: ahoppen

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(

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"

Sources/SourceKitLSP/SourceKitLSPServer.swift

@@ -567,7 +567,7 @@ package actor SourceKitLSPServer {
 
     logger.log(
       """
-      Using toolchain at \(toolchain.path?.description ?? "<nil>") (\(toolchain.identifier, privacy: .public)) \
+      Using toolchain at \(toolchain.path.description) (\(toolchain.identifier, privacy: .public)) \
       for \(uri.forLogging)
       """
     )

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(

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
 

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 toolchainsByPath[toolchain.path] != nil {
+        logger.fault("Found two toolchains with the same path: \(toolchain.path)")
+        continue
       }
 
-      // Toolchain should always be unique by path if it is present.
-      if let path = toolchain.path {
-        guard toolchainsByPath[path] == nil else {
+      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
         }
-        toolchainsByPath[path] = toolchain
+        toolchainsByCompiler[compiler] = toolchain
       }
 
-      toolchainsByIdentifier[toolchain.identifier, default: []].append(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]
   }
 }
 

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 ?? "<nil>") 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")