diff --git a/Documentation/Files_To_Reindex.md b/Documentation/Files_To_Reindex.md index 7127596ab..409d8cbfb 100644 --- a/Documentation/Files_To_Reindex.md +++ b/Documentation/Files_To_Reindex.md @@ -18,7 +18,9 @@ Alternatives would be: ## `.h` -Affects all files that include the header (including via other headers). For existing headers, we know which files import a header from the existing index. For new header files, we assume that it hasn’t been included in any file yet and so we don’t re-index any file. If it is, it’s likely that the user will modify the file that includes the new header file shortly after, which will re-index it. +All files that include the header (including via other headers) might be affected by the change, similar to how all `.swift` files that import a module might be affected. Similar to modules, we choose to not re-index all files that include the header because of the same considerations mentioned above. + +To re-index the header, we pick one main file that includes the header and re-index that, which will effectively update the index for the header. For existing headers, we know which files import a header from the existing index. For new header files, we assume that it hasn’t been included in any file yet and thus don't index it. If the user wrote an include to the new header before creating the header itself, we don't know about that include from the existing index. But it’s likely that the user will modify the file that includes the new header file shortly after, which will index the header and establish the header to main file connection. ## `.c` / `.cpp` / `.m` diff --git a/Sources/LSPLogging/NonDarwinLogging.swift b/Sources/LSPLogging/NonDarwinLogging.swift index b838a29db..94ab0d1ce 100644 --- a/Sources/LSPLogging/NonDarwinLogging.swift +++ b/Sources/LSPLogging/NonDarwinLogging.swift @@ -195,6 +195,17 @@ public struct NonDarwinLogInterpolation: StringInterpolationProtocol, Sendable { append(description: message.description, redactedDescription: message.redactedDescription, privacy: privacy) } + public mutating func appendInterpolation( + _ message: (some CustomLogStringConvertibleWrapper & Sendable)?, + privacy: NonDarwinLogPrivacy = .private + ) { + if let message { + self.appendInterpolation(message, privacy: privacy) + } else { + self.appendLiteral("") + } + } + public mutating func appendInterpolation(_ type: Any.Type, privacy: NonDarwinLogPrivacy = .public) { append(description: String(reflecting: type), redactedDescription: "", privacy: privacy) } diff --git a/Sources/SKCore/BuildSystemManager.swift b/Sources/SKCore/BuildSystemManager.swift index 33d755c57..a8c7d1b66 100644 --- a/Sources/SKCore/BuildSystemManager.swift +++ b/Sources/SKCore/BuildSystemManager.swift @@ -141,13 +141,10 @@ extension BuildSystemManager { /// Implementation detail of `buildSettings(for:language:)`. private func buildSettingsFromPrimaryBuildSystem( for document: DocumentURI, + in target: ConfiguredTarget?, language: Language ) async throws -> FileBuildSettings? { - guard let buildSystem else { - return nil - } - guard let target = await canonicalConfiguredTarget(for: document) else { - logger.error("Failed to get target for \(document.forLogging)") + guard let buildSystem, let target else { return nil } // FIXME: (async) We should only wait `fallbackSettingsTimeout` for build @@ -155,18 +152,25 @@ extension BuildSystemManager { // implement that with Swift concurrency. // For now, this should be fine because all build systems return // very quickly from `settings(for:language:)`. - guard let settings = try await buildSystem.buildSettings(for: document, in: target, language: language) else { - return nil - } - return settings + return try await buildSystem.buildSettings(for: document, in: target, language: language) } - private func buildSettings( + /// Returns the build settings for the given file in the given target. + /// + /// Only call this method if it is known that `document` is a main file. Prefer `buildSettingsInferredFromMainFile` + /// otherwise. If `document` is a header file, this will most likely return fallback settings because header files + /// don't have build settings by themselves. + public func buildSettings( for document: DocumentURI, + in target: ConfiguredTarget?, language: Language ) async -> FileBuildSettings? { do { - if let buildSettings = try await buildSettingsFromPrimaryBuildSystem(for: document, language: language) { + if let buildSettings = try await buildSettingsFromPrimaryBuildSystem( + for: document, + in: target, + language: language + ) { return buildSettings } } catch { @@ -194,11 +198,11 @@ extension BuildSystemManager { /// references to that C file in the build settings by the header file. public func buildSettingsInferredFromMainFile( for document: DocumentURI, - language: Language, - logBuildSettings: Bool = true + language: Language ) async -> FileBuildSettings? { let mainFile = await mainFile(for: document, language: language) - guard var settings = await buildSettings(for: mainFile, language: language) else { + let target = await canonicalConfiguredTarget(for: mainFile) + guard var settings = await buildSettings(for: mainFile, in: target, language: language) else { return nil } if mainFile != document { @@ -206,9 +210,7 @@ extension BuildSystemManager { // to reference `document` instead of `mainFile`. settings = settings.patching(newFile: document.pseudoPath, originalFile: mainFile.pseudoPath) } - if logBuildSettings { - await BuildSettingsLogger.shared.log(settings: settings, for: document) - } + await BuildSettingsLogger.shared.log(settings: settings, for: document) return settings } diff --git a/Sources/SemanticIndex/SemanticIndexManager.swift b/Sources/SemanticIndex/SemanticIndexManager.swift index 25c498aac..ca13b46d7 100644 --- a/Sources/SemanticIndex/SemanticIndexManager.swift +++ b/Sources/SemanticIndex/SemanticIndexManager.swift @@ -221,12 +221,14 @@ public final actor SemanticIndexManager { /// /// If `files` contains a header file, this will return a `FileToIndex` that re-indexes a main file which includes the /// header file to update the header file's index. - private func filesToIndex(toCover files: some Collection) async -> [FileToIndex] { + private func filesToIndex( + toCover files: some Collection + ) async -> [FileToIndex] { let sourceFiles = Set(await buildSystemManager.sourceFiles().map(\.uri)) let filesToReIndex = await files.asyncCompactMap { (uri) -> FileToIndex? in if sourceFiles.contains(uri) { // If this is a source file, just index it. - return FileToIndex(uri: uri, mainFile: nil) + return .indexableFile(uri) } // Otherwise, see if it is a header file. If so, index a main file that that imports it to update header file's // index. @@ -239,7 +241,7 @@ public final actor SemanticIndexManager { guard let mainFile else { return nil } - return FileToIndex(uri: uri, mainFile: mainFile) + return .headerFile(header: uri, mainFile: mainFile) } return filesToReIndex } @@ -338,10 +340,10 @@ public final actor SemanticIndexManager { } /// Update the index store for the given files, assuming that their targets have already been prepared. - private func updateIndexStore(for files: [FileToIndex], taskID: UUID, priority: TaskPriority?) async { + private func updateIndexStore(for filesAndTargets: [FileAndTarget], taskID: UUID, priority: TaskPriority?) async { let taskDescription = AnyIndexTaskDescription( UpdateIndexStoreTaskDescription( - filesToIndex: Set(files), + filesToIndex: filesAndTargets, buildSystemManager: self.buildSystemManager, index: index ) @@ -349,22 +351,22 @@ public final actor SemanticIndexManager { let updateIndexStoreTask = await self.indexTaskScheduler.schedule(priority: priority, taskDescription) { newState in switch newState { case .executing: - for file in files { - if case .scheduled((taskID, let task)) = self.indexStatus[file.uri] { - self.indexStatus[file.uri] = .executing((taskID, task)) + for fileAndTarget in filesAndTargets { + if case .scheduled((taskID, let task)) = self.indexStatus[fileAndTarget.file.sourceFile] { + self.indexStatus[fileAndTarget.file.sourceFile] = .executing((taskID, task)) } } case .cancelledToBeRescheduled: - for file in files { - if case .executing((taskID, let task)) = self.indexStatus[file.uri] { - self.indexStatus[file.uri] = .scheduled((taskID, task)) + for fileAndTarget in filesAndTargets { + if case .executing((taskID, let task)) = self.indexStatus[fileAndTarget.file.sourceFile] { + self.indexStatus[fileAndTarget.file.sourceFile] = .scheduled((taskID, task)) } } case .finished: - for file in files { - switch self.indexStatus[file.uri] { + for fileAndTarget in filesAndTargets { + switch self.indexStatus[fileAndTarget.file.sourceFile] { case .executing((taskID, _)): - self.indexStatus[file.uri] = .upToDate + self.indexStatus[fileAndTarget.file.sourceFile] = .upToDate default: break } @@ -383,22 +385,25 @@ public final actor SemanticIndexManager { priority: TaskPriority? ) async -> Task { let outOfDateFiles = await filesToIndex(toCover: files).filter { - if case .upToDate = indexStatus[$0.uri] { + if case .upToDate = indexStatus[$0.sourceFile] { return false } return true } - .sorted(by: { $0.uri.stringValue < $1.uri.stringValue }) // sort files to get deterministic indexing order + // sort files to get deterministic indexing order + .sorted(by: { $0.sourceFile.stringValue < $1.sourceFile.stringValue }) // Sort the targets in topological order so that low-level targets get built before high-level targets, allowing us // to index the low-level targets ASAP. var filesByTarget: [ConfiguredTarget: [FileToIndex]] = [:] - for file in outOfDateFiles { - guard let target = await buildSystemManager.canonicalConfiguredTarget(for: file.uri) else { - logger.error("Not indexing \(file.uri.forLogging) because the target could not be determined") + for fileToIndex in outOfDateFiles { + guard let target = await buildSystemManager.canonicalConfiguredTarget(for: fileToIndex.mainFile) else { + logger.error( + "Not indexing \(fileToIndex.forLogging) because the target could not be determined" + ) continue } - filesByTarget[target, default: []].append(file) + filesByTarget[target, default: []].append(fileToIndex) } var sortedTargets: [ConfiguredTarget] = @@ -438,7 +443,11 @@ public final actor SemanticIndexManager { // https://github.com/apple/sourcekit-lsp/issues/1268 for fileBatch in filesByTarget[target]!.partition(intoBatchesOfSize: 1) { taskGroup.addTask { - await self.updateIndexStore(for: fileBatch, taskID: taskID, priority: priority) + await self.updateIndexStore( + for: fileBatch.map { FileAndTarget(file: $0, target: target) }, + taskID: taskID, + priority: priority + ) } } } @@ -453,7 +462,7 @@ public final actor SemanticIndexManager { // setting it to `.scheduled` because we don't have an `await` call between the creation of `indexTask` and // this loop, so we still have exclusive access to the `SemanticIndexManager` actor and hence `updateIndexStore` // can't execute until we have set all index statuses to `.scheduled`. - indexStatus[file.uri] = .scheduled((taskID, indexTask)) + indexStatus[file.sourceFile] = .scheduled((taskID, indexTask)) } indexTasksWereScheduled(filesToIndex.count) } diff --git a/Sources/SemanticIndex/UpdateIndexStoreTaskDescription.swift b/Sources/SemanticIndex/UpdateIndexStoreTaskDescription.swift index bd4c5b749..07fa5d59d 100644 --- a/Sources/SemanticIndex/UpdateIndexStoreTaskDescription.swift +++ b/Sources/SemanticIndex/UpdateIndexStoreTaskDescription.swift @@ -22,14 +22,58 @@ import class TSCBasic.Process private nonisolated(unsafe) var updateIndexStoreIDForLogging = AtomicUInt32(initialValue: 1) -/// Information about a file that should be indexed. -/// -/// The URI of the file whose index should be updated. This could be a header file that can't actually be indexed on -/// its own. In that case `mainFile` is the file that should be indexed, which will effectively update the index of -/// `uri`. -struct FileToIndex: Hashable { - let uri: DocumentURI - let mainFile: DocumentURI? +enum FileToIndex: CustomLogStringConvertible { + /// A non-header file + case indexableFile(DocumentURI) + + /// A header file where `mainFile` should be indexed to update the index of `header`. + case headerFile(header: DocumentURI, mainFile: DocumentURI) + + /// The file whose index store should be updated. + /// + /// This file might be a header file that doesn't have build settings associated with it. For the actual compiler + /// invocation that updates the index store, the `mainFile` should be used. + var sourceFile: DocumentURI { + switch self { + case .indexableFile(let uri): return uri + case .headerFile(header: let header, mainFile: _): return header + } + } + + /// The file that should be used for compiler invocations that update the index. + /// + /// If the `sourceFile` is a header file, this will be a main file that includes the header. Otherwise, it will be the + /// same as `sourceFile`. + var mainFile: DocumentURI { + switch self { + case .indexableFile(let uri): return uri + case .headerFile(header: _, mainFile: let mainFile): return mainFile + } + } + + var description: String { + switch self { + case .indexableFile(let uri): + return uri.description + case .headerFile(header: let header, mainFile: let mainFile): + return "\(header.description) using main file \(mainFile.description)" + } + } + + var redactedDescription: String { + switch self { + case .indexableFile(let uri): + return uri.redactedDescription + case .headerFile(header: let header, mainFile: let mainFile): + return "\(header.redactedDescription) using main file \(mainFile.redactedDescription)" + } + } +} + +/// A file to index and the target in which the file should be indexed. +struct FileAndTarget { + let file: FileToIndex + let target: ConfiguredTarget } /// Describes a task to index a set of source files. @@ -40,7 +84,7 @@ public struct UpdateIndexStoreTaskDescription: IndexTaskDescription { public let id = updateIndexStoreIDForLogging.fetchAndIncrement() /// The files that should be indexed. - private let filesToIndex: Set + private let filesToIndex: [FileAndTarget] /// The build system manager that is used to get the toolchain and build settings for the files to index. private let buildSystemManager: BuildSystemManager @@ -63,7 +107,7 @@ public struct UpdateIndexStoreTaskDescription: IndexTaskDescription { } init( - filesToIndex: Set, + filesToIndex: [FileAndTarget], buildSystemManager: BuildSystemManager, index: UncheckedIndex ) { @@ -82,17 +126,19 @@ public struct UpdateIndexStoreTaskDescription: IndexTaskDescription { ) { let startDate = Date() - let filesToIndexDescription = filesToIndex.map { $0.uri.fileURL?.lastPathComponent ?? $0.uri.stringValue } - .joined(separator: ", ") + let filesToIndexDescription = filesToIndex.map { + $0.file.sourceFile.fileURL?.lastPathComponent ?? $0.file.sourceFile.stringValue + } + .joined(separator: ", ") logger.log( "Starting updating index store with priority \(Task.currentPriority.rawValue, privacy: .public): \(filesToIndexDescription)" ) - let filesToIndex = filesToIndex.sorted(by: { $0.uri.stringValue < $1.uri.stringValue }) + let filesToIndex = filesToIndex.sorted(by: { $0.file.sourceFile.stringValue < $1.file.sourceFile.stringValue }) // TODO (indexing): Once swiftc supports it, we should group files by target and index files within the same // target together in one swiftc invocation. // https://github.com/apple/sourcekit-lsp/issues/1268 for file in filesToIndex { - await updateIndexStoreForSingleFile(file) + await updateIndexStore(forSingleFile: file.file, in: file.target) } logger.log( "Finished updating index store in \(Date().timeIntervalSince(startDate) * 1000, privacy: .public)ms: \(filesToIndexDescription)" @@ -103,8 +149,11 @@ public struct UpdateIndexStoreTaskDescription: IndexTaskDescription { public func dependencies( to currentlyExecutingTasks: [UpdateIndexStoreTaskDescription] ) -> [TaskDependencyAction] { + let selfMainFiles = Set(filesToIndex.map(\.file.mainFile)) return currentlyExecutingTasks.compactMap { (other) -> TaskDependencyAction? in - guard !other.filesToIndex.intersection(filesToIndex).isEmpty else { + guard + !other.filesToIndex.lazy.map(\.file.mainFile).contains(where: { selfMainFiles.contains($0) }) + else { // Disjoint sets of files can be indexed concurrently. return nil } @@ -120,60 +169,62 @@ public struct UpdateIndexStoreTaskDescription: IndexTaskDescription { } } - private func updateIndexStoreForSingleFile(_ fileToIndex: FileToIndex) async { - let mainFileUri = fileToIndex.mainFile ?? fileToIndex.uri - guard let fileToIndexUrl = fileToIndex.uri.fileURL else { + private func updateIndexStore(forSingleFile file: FileToIndex, in target: ConfiguredTarget) async { + guard let sourceFileUrl = file.sourceFile.fileURL else { // The URI is not a file, so there's nothing we can index. return } - guard - !index.checked(for: .modifiedFiles).hasUpToDateUnit(for: fileToIndexUrl, mainFile: fileToIndex.mainFile?.fileURL) + guard !index.checked(for: .modifiedFiles).hasUpToDateUnit(for: sourceFileUrl, mainFile: file.mainFile.fileURL) else { - logger.debug("Not indexing \(fileToIndex.uri.forLogging) because index has an up-to-date unit") + logger.debug("Not indexing \(file.forLogging) because index has an up-to-date unit") // We consider a file's index up-to-date if we have any up-to-date unit. Changing build settings does not // invalidate the up-to-date status of the index. return } - if let mainFile = fileToIndex.mainFile { - logger.log("Updating index store of \(fileToIndex.uri.forLogging) using main file \(mainFile.forLogging)") + if file.mainFile != file.sourceFile { + logger.log("Updating index store of \(file.forLogging) using main file \(file.mainFile.forLogging)") } - guard let language = await buildSystemManager.defaultLanguage(for: mainFileUri) else { - logger.error("Not indexing \(fileToIndex.uri.forLogging) because its language could not be determined") + guard let language = await buildSystemManager.defaultLanguage(for: file.mainFile) else { + logger.error("Not indexing \(file.forLogging) because its language could not be determined") return } - let buildSettings = await buildSystemManager.buildSettingsInferredFromMainFile( - for: mainFileUri, - language: language, - logBuildSettings: false - ) + let buildSettings = await buildSystemManager.buildSettings(for: file.mainFile, in: target, language: language) guard let buildSettings else { - logger.error("Not indexing \(fileToIndex.uri.forLogging) because it has no compiler arguments") + logger.error("Not indexing \(file.forLogging) because it has no compiler arguments") return } - guard let toolchain = await buildSystemManager.toolchain(for: mainFileUri, language) else { + guard let toolchain = await buildSystemManager.toolchain(for: file.mainFile, language) else { logger.error( - "Not updating index store for \(mainFileUri.forLogging) because no toolchain could be determined for the document" + "Not updating index store for \(file.forLogging) because no toolchain could be determined for the document" ) return } switch language { case .swift: do { - try await updateIndexStore(forSwiftFile: mainFileUri, buildSettings: buildSettings, toolchain: toolchain) + try await updateIndexStore( + forSwiftFile: file.mainFile, + buildSettings: buildSettings, + toolchain: toolchain + ) } catch { - logger.error("Updating index store for \(fileToIndex.uri) failed: \(error.forLogging)") - BuildSettingsLogger.log(settings: buildSettings, for: mainFileUri) + logger.error("Updating index store for \(file.forLogging) failed: \(error.forLogging)") + BuildSettingsLogger.log(settings: buildSettings, for: file.mainFile) } case .c, .cpp, .objective_c, .objective_cpp: do { - try await updateIndexStore(forClangFile: mainFileUri, buildSettings: buildSettings, toolchain: toolchain) + try await updateIndexStore( + forClangFile: file.mainFile, + buildSettings: buildSettings, + toolchain: toolchain + ) } catch { - logger.error("Updating index store for \(fileToIndex.uri) failed: \(error.forLogging)") - BuildSettingsLogger.log(settings: buildSettings, for: mainFileUri) + logger.error("Updating index store for \(file) failed: \(error.forLogging)") + BuildSettingsLogger.log(settings: buildSettings, for: file.mainFile) } default: logger.error( - "Not updating index store for \(fileToIndex.uri) because it is a language that is not supported by background indexing" + "Not updating index store for \(file) because it is a language that is not supported by background indexing" ) } } @@ -401,3 +452,21 @@ private func adjustClangCompilerArgumentsForIndexStoreUpdate( ) return result } + +fileprivate extension Sequence { + /// Returns `true` if this sequence contains an element that is equal to an element in `otherSequence` when + /// considering two elements as equal if they satisfy `predicate`. + func hasIntersection( + with otherSequence: some Sequence, + where predicate: (Element, Element) -> Bool + ) -> Bool { + for outer in self { + for inner in otherSequence { + if predicate(outer, inner) { + return true + } + } + } + return false + } +} diff --git a/Sources/SourceKitLSP/Clang/ClangLanguageService.swift b/Sources/SourceKitLSP/Clang/ClangLanguageService.swift index 6e9a63d0e..5c1822c21 100644 --- a/Sources/SourceKitLSP/Clang/ClangLanguageService.swift +++ b/Sources/SourceKitLSP/Clang/ClangLanguageService.swift @@ -499,8 +499,6 @@ extension ClangLanguageService { return } let clangBuildSettings = await self.buildSettings(for: uri) - // FIXME: (logging) Log only the `-something` flags with paths redacted if private mode is enabled - logger.info("settings for \(uri.forLogging): \(clangBuildSettings?.compilerArgs.description ?? "nil")") // The compile command changed, send over the new one. // FIXME: what should we do if we no longer have valid build settings?