Merge pull request #1971 from bnbarham/pass-allow-errors-for-ast

Update the compiler arguments used for background AST builds

Author: bnbarham

Contributor Documentation/Implementing a BSP server.md

@@ -29,7 +29,7 @@ If the build system loads the entire build graph during initialization, it may i
 
 ## Supporting background indexing
 
-To support background indexing, the build system must set `data.prepareProvider: true` in the `build/initialize` response and implement the `buildTarget/prepare` method.
+To support background indexing, the build system must set `data.prepareProvider: true` in the `build/initialize` response and implement the `buildTarget/prepare` method. The compiler options used to prepare a target should match those sent for `textDocument/sourceKitOptions` in order to avoid mismatches when loading modules.
 
 ## Optional methods
 

Sources/BuildSystemIntegration/BuildSystemManager.swift

@@ -325,7 +325,7 @@ package actor BuildSystemManager: QueueBasedMessageHandler {
     }
   }
 
-  private var cachedSourceKitOptions = RequestCache<TextDocumentSourceKitOptionsRequest>()
+  private var cachedAdjustedSourceKitOptions = RequestCache<TextDocumentSourceKitOptionsRequest>()
 
   private var cachedBuildTargets = Cache<WorkspaceBuildTargetsRequest, [BuildTargetIdentifier: BuildTargetInfo]>()
 
@@ -529,7 +529,7 @@ package actor BuildSystemManager: QueueBasedMessageHandler {
       } else {
         nil
       }
-    self.cachedSourceKitOptions.clear(isolation: self) { cacheKey in
+    self.cachedAdjustedSourceKitOptions.clear(isolation: self) { cacheKey in
       guard let updatedTargets else {
         // All targets might have changed
         return true
@@ -758,13 +758,22 @@ package actor BuildSystemManager: QueueBasedMessageHandler {
       target: target,
       language: language
     )
-
-    let response = try await cachedSourceKitOptions.get(request, isolation: self) { request in
-      try await buildSystemAdapter.send(request)
+    let response = try await cachedAdjustedSourceKitOptions.get(request, isolation: self) { request in
+      let options = try await buildSystemAdapter.send(request)
+      switch language.semanticKind {
+      case .swift:
+        return options?.adjustArgsForSemanticSwiftFunctionality(fileToIndex: document)
+      case .clang:
+        return options?.adjustingArgsForSemanticClangFunctionality()
+      default:
+        return options
+      }
     }
+
     guard let response else {
       return nil
     }
+
     return FileBuildSettings(
       compilerArguments: response.compilerArguments,
       workingDirectory: response.workingDirectory,
@@ -1239,3 +1248,153 @@ package actor BuildSystemManager: QueueBasedMessageHandler {
 private func isDescendant(_ selfPathComponents: [String], of otherPathComponents: [String]) -> Bool {
   return selfPathComponents.dropLast().starts(with: otherPathComponents)
 }
+
+fileprivate extension TextDocumentSourceKitOptionsResponse {
+  /// Adjust compiler arguments that were created for building to compiler arguments that should be used for indexing
+  /// or background AST builds.
+  ///
+  /// This removes compiler arguments that produce output files and adds arguments to eg. allow errors and index the
+  /// file.
+  func adjustArgsForSemanticSwiftFunctionality(fileToIndex: DocumentURI) -> TextDocumentSourceKitOptionsResponse {
+    let optionsToRemove: [CompilerCommandLineOption] = [
+      .flag("c", [.singleDash]),
+      .flag("disable-cmo", [.singleDash]),
+      .flag("emit-dependencies", [.singleDash]),
+      .flag("emit-module-interface", [.singleDash]),
+      .flag("emit-module", [.singleDash]),
+      .flag("emit-objc-header", [.singleDash]),
+      .flag("incremental", [.singleDash]),
+      .flag("no-color-diagnostics", [.singleDash]),
+      .flag("parseable-output", [.singleDash]),
+      .flag("save-temps", [.singleDash]),
+      .flag("serialize-diagnostics", [.singleDash]),
+      .flag("use-frontend-parseable-output", [.singleDash]),
+      .flag("validate-clang-modules-once", [.singleDash]),
+      .flag("whole-module-optimization", [.singleDash]),
+      .flag("experimental-skip-all-function-bodies", frontendName: "Xfrontend", [.singleDash]),
+      .flag("experimental-skip-non-inlinable-function-bodies", frontendName: "Xfrontend", [.singleDash]),
+      .flag("experimental-skip-non-exportable-decls", frontendName: "Xfrontend", [.singleDash]),
+      .flag("experimental-lazy-typecheck", frontendName: "Xfrontend", [.singleDash]),
+
+      .option("clang-build-session-file", [.singleDash], [.separatedBySpace]),
+      .option("emit-module-interface-path", [.singleDash], [.separatedBySpace]),
+      .option("emit-module-path", [.singleDash], [.separatedBySpace]),
+      .option("emit-objc-header-path", [.singleDash], [.separatedBySpace]),
+      .option("emit-package-module-interface-path", [.singleDash], [.separatedBySpace]),
+      .option("emit-private-module-interface-path", [.singleDash], [.separatedBySpace]),
+      .option("num-threads", [.singleDash], [.separatedBySpace]),
+      // Technically, `-o` and the output file don't need to be separated by a space. Eg. `swiftc -oa file.swift` is
+      // valid and will write to an output file named `a`.
+      // We can't support that because the only way to know that `-output-file-map` is a different flag and not an option
+      // to write to an output file named `utput-file-map` is to know all compiler arguments of `swiftc`, which we don't.
+      .option("o", [.singleDash], [.separatedBySpace]),
+      .option("output-file-map", [.singleDash], [.separatedBySpace, .separatedByEqualSign]),
+    ]
+
+    var result: [String] = []
+    result.reserveCapacity(compilerArguments.count)
+    var iterator = compilerArguments.makeIterator()
+    while let argument = iterator.next() {
+      switch optionsToRemove.firstMatch(for: argument) {
+      case .removeOption:
+        continue
+      case .removeOptionAndNextArgument:
+        _ = iterator.next()
+        continue
+      case .removeOptionAndPreviousArgument(let name):
+        if let previousArg = result.last, previousArg.hasSuffix("-\(name)") {
+          _ = result.popLast()
+        }
+        continue
+      case nil:
+        break
+      }
+      result.append(argument)
+    }
+
+    result += [
+      // Avoid emitting the ABI descriptor, we don't need it
+      "-Xfrontend", "-empty-abi-descriptor",
+    ]
+
+    result += supplementalClangIndexingArgs.flatMap { ["-Xcc", $0] }
+
+    return TextDocumentSourceKitOptionsResponse(compilerArguments: result, workingDirectory: workingDirectory)
+  }
+
+  /// Adjust compiler arguments that were created for building to compiler arguments that should be used for indexing
+  /// or background AST builds.
+  ///
+  /// This removes compiler arguments that produce output files and adds arguments to eg. typecheck only.
+  func adjustingArgsForSemanticClangFunctionality() -> TextDocumentSourceKitOptionsResponse {
+    let optionsToRemove: [CompilerCommandLineOption] = [
+      // Disable writing of a depfile
+      .flag("M", [.singleDash]),
+      .flag("MD", [.singleDash]),
+      .flag("MMD", [.singleDash]),
+      .flag("MG", [.singleDash]),
+      .flag("MM", [.singleDash]),
+      .flag("MV", [.singleDash]),
+      // Don't create phony targets
+      .flag("MP", [.singleDash]),
+      // Don't write out compilation databases
+      .flag("MJ", [.singleDash]),
+      // Don't compile
+      .flag("c", [.singleDash]),
+
+      .flag("fmodules-validate-once-per-build-session", [.singleDash]),
+
+      // Disable writing of a depfile
+      .option("MT", [.singleDash], [.noSpace, .separatedBySpace]),
+      .option("MF", [.singleDash], [.noSpace, .separatedBySpace]),
+      .option("MQ", [.singleDash], [.noSpace, .separatedBySpace]),
+
+      // Don't write serialized diagnostic files
+      .option("serialize-diagnostics", [.singleDash, .doubleDash], [.separatedBySpace]),
+
+      .option("fbuild-session-file", [.singleDash], [.separatedByEqualSign]),
+    ]
+
+    var result: [String] = []
+    result.reserveCapacity(compilerArguments.count)
+    var iterator = compilerArguments.makeIterator()
+    while let argument = iterator.next() {
+      switch optionsToRemove.firstMatch(for: argument) {
+      case .removeOption:
+        continue
+      case .removeOptionAndNextArgument:
+        _ = iterator.next()
+        continue
+      case .removeOptionAndPreviousArgument(let name):
+        if let previousArg = result.last, previousArg.hasSuffix("-\(name)") {
+          _ = result.popLast()
+        }
+        continue
+      case nil:
+        break
+      }
+      result.append(argument)
+    }
+    result += supplementalClangIndexingArgs
+    result.append(
+      "-fsyntax-only"
+    )
+    return TextDocumentSourceKitOptionsResponse(compilerArguments: result, workingDirectory: workingDirectory)
+  }
+}
+
+fileprivate let supplementalClangIndexingArgs: [String] = [
+  // Retain extra information for indexing
+  "-fretain-comments-from-system-headers",
+  // Pick up macro definitions during indexing
+  "-Xclang", "-detailed-preprocessing-record",
+
+  // libclang uses 'raw' module-format. Match it so we can reuse the module cache and PCHs that libclang uses.
+  "-Xclang", "-fmodule-format=raw",
+
+  // Be less strict - we want to continue and typecheck/index as much as possible
+  "-Xclang", "-fallow-pch-with-compiler-errors",
+  "-Xclang", "-fallow-pcm-with-compiler-errors",
+  "-Wno-non-modular-include-in-framework-module",
+  "-Wno-incomplete-umbrella",
+]

Sources/BuildSystemIntegration/CMakeLists.txt

@@ -9,6 +9,7 @@ add_library(BuildSystemIntegration STATIC
   BuiltInBuildSystem.swift
   BuiltInBuildSystemAdapter.swift
   CompilationDatabase.swift
+  CompilerCommandLineOption.swift
   DetermineBuildSystem.swift
   ExternalBuildSystemAdapter.swift
   FallbackBuildSettings.swift

Sources/SemanticIndex/CompilerCommandLineOption.swift renamed to Sources/BuildSystemIntegration/CompilerCommandLineOption.swift

@@ -12,14 +12,18 @@
 
 package struct CompilerCommandLineOption {
   /// Return value of `matches(argument:)`.
-  package enum Match {
+  package enum Match: Equatable {
     /// The `CompilerCommandLineOption` matched the command line argument. The next element in the command line is a
     /// separate argument and should not be removed.
     case removeOption
 
     /// The `CompilerCommandLineOption` matched the command line argument. The next element in the command line is an
     /// argument to this option and should be removed as well.
     case removeOptionAndNextArgument
+
+    /// The `CompilerCommandLineOption` matched the command line argument. The previous element in the command line is
+    /// a prefix to this argument and should be removed if it matches `name`.
+    case removeOptionAndPreviousArgument(name: String)
   }
 
   package enum DashSpelling {
@@ -39,16 +43,28 @@ package struct CompilerCommandLineOption {
   /// The name of the option, without any preceeding `-` or `--`.
   private let name: String
 
+  /// The name of the argument that prefixes this flag, without any preceeding `-` or `--` (eg. `Xfrontend`/`Xclang`).
+  private let frontendName: String?
+
   /// Whether the option can be spelled with one or two dashes.
   private let dashSpellings: [DashSpelling]
 
   /// The ways that arguments can specified after the option. Empty if the option is a flag that doesn't take any
   /// argument.
   private let argumentStyles: [ArgumentStyles]
 
-  package static func flag(_ name: String, _ dashSpellings: [DashSpelling]) -> CompilerCommandLineOption {
+  package static func flag(
+    _ name: String,
+    frontendName: String? = nil,
+    _ dashSpellings: [DashSpelling]
+  ) -> CompilerCommandLineOption {
     precondition(!dashSpellings.isEmpty)
-    return CompilerCommandLineOption(name: name, dashSpellings: dashSpellings, argumentStyles: [])
+    return CompilerCommandLineOption(
+      name: name,
+      frontendName: frontendName,
+      dashSpellings: dashSpellings,
+      argumentStyles: []
+    )
   }
 
   package static func option(
@@ -58,10 +74,29 @@ package struct CompilerCommandLineOption {
   ) -> CompilerCommandLineOption {
     precondition(!dashSpellings.isEmpty)
     precondition(!argumentStyles.isEmpty)
-    return CompilerCommandLineOption(name: name, dashSpellings: dashSpellings, argumentStyles: argumentStyles)
+    return CompilerCommandLineOption(
+      name: name,
+      frontendName: nil,
+      dashSpellings: dashSpellings,
+      argumentStyles: argumentStyles
+    )
   }
 
   package func matches(argument: String) -> Match? {
+    let match = matchesIgnoringFrontend(argument: argument)
+    guard let match, let frontendName else {
+      return match
+    }
+
+    switch match {
+    case .removeOption:
+      return .removeOptionAndPreviousArgument(name: frontendName)
+    default:
+      return match
+    }
+  }
+
+  private func matchesIgnoringFrontend(argument: String) -> Match? {
     let argumentName: Substring
     if argument.hasPrefix("--") {
       if dashSpellings.contains(.doubleDash) {

Sources/BuildSystemIntegration/SwiftPMBuildSystem.swift

@@ -287,7 +287,8 @@ package actor SwiftPMBuildSystem: BuiltInBuildSystem {
       configuration: buildConfiguration,
       toolchain: destinationSwiftPMToolchain,
       triple: destinationSDK.targetTriple,
-      flags: buildFlags
+      flags: buildFlags,
+      prepareForIndexing: options.backgroundPreparationModeOrDefault.toSwiftPMPreparation
     )
 
     packageLoadingQueue.async {
@@ -709,4 +710,17 @@ fileprivate extension URL {
   }
 }
 
+fileprivate extension SourceKitLSPOptions.BackgroundPreparationMode {
+  var toSwiftPMPreparation: BuildParameters.PrepareForIndexingMode {
+    switch self {
+    case .build:
+      return .off
+    case .noLazy:
+      return .noLazy
+    case .enabled:
+      return .on
+    }
+  }
+}
+
 #endif

Sources/LanguageServerProtocolExtensions/CMakeLists.txt

@@ -3,7 +3,7 @@ add_library(LanguageServerProtocolExtensions STATIC
   Connection+Send.swift
   DocumentURI+CustomLogStringConvertible.swift
   DocumentURI+symlinkTarget.swift
-  Language+InferredFromFileExtension.swift
+  Language+Inference.swift
   LocalConnection.swift
   QueueBasedMessageHandler.swift
   RequestAndReply.swift

Sources/LanguageServerProtocolExtensions/Language+InferredFromFileExtension.swift renamed to Sources/LanguageServerProtocolExtensions/Language+Inference.swift

@@ -19,6 +19,22 @@ import LanguageServerProtocol
 #endif
 
 extension Language {
+  package enum SemanticKind {
+    case clang
+    case swift
+  }
+
+  package var semanticKind: SemanticKind? {
+    switch self {
+    case .swift:
+      return .swift
+    case .c, .cpp, .objective_c, .objective_cpp:
+      return .clang
+    default:
+      return nil
+    }
+  }
+
   package init?(inferredFromFileExtension uri: DocumentURI) {
     // URL.pathExtension is only set for file URLs but we want to also infer a file extension for non-file URLs like
     // untitled:file.cpp

Sources/SemanticIndex/CMakeLists.txt

@@ -1,7 +1,6 @@
 
 add_library(SemanticIndex STATIC
   CheckedIndex.swift
-  CompilerCommandLineOption.swift
   IndexTaskDescription.swift
   IndexHooks.swift
   PreparationTaskDescription.swift