Skip to content

Index clang files #1289

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 14, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
117 changes: 106 additions & 11 deletions Sources/SemanticIndex/UpdateIndexStoreTaskDescription.swift
Original file line number Diff line number Diff line change
Expand Up @@ -163,9 +163,12 @@ public struct UpdateIndexStoreTaskDescription: TaskDescriptionProtocol {
BuildSettingsLogger.log(settings: buildSettings, for: uri)
}
case .c, .cpp, .objective_c, .objective_cpp:
// TODO (indexing): Support indexing of clang files, including headers.
// https://github.com/apple/sourcekit-lsp/issues/1253
break
do {
try await updateIndexStore(forClangFile: uri, buildSettings: buildSettings, toolchain: toolchain)
} catch {
logger.error("Updating index store for \(uri) failed: \(error.forLogging)")
BuildSettingsLogger.log(settings: buildSettings, for: uri)
}
default:
logger.error(
"Not updating index store for \(uri) because it is a language that is not supported by background indexing"
Expand All @@ -190,10 +193,49 @@ public struct UpdateIndexStoreTaskDescription: TaskDescriptionProtocol {
fileToIndex: uri
)

let process = try Process.launch(
arguments: [swiftc.pathString] + indexingArguments,
try await runIndexingProcess(
indexFile: uri,
buildSettings: buildSettings,
processArguments: [swiftc.pathString] + indexingArguments,
workingDirectory: buildSettings.workingDirectory.map(AbsolutePath.init(validating:))
)
}

private func updateIndexStore(
forClangFile uri: DocumentURI,
buildSettings: FileBuildSettings,
toolchain: Toolchain
) async throws {
guard let clang = toolchain.clang else {
logger.error(
"Not updating index store for \(uri.forLogging) because toolchain \(toolchain.identifier) does not contain clang"
)
return
}

let indexingArguments = adjustClangCompilerArgumentsForIndexStoreUpdate(
buildSettings.compilerArguments,
fileToIndex: uri
)

try await runIndexingProcess(
indexFile: uri,
buildSettings: buildSettings,
processArguments: [clang.pathString] + indexingArguments,
workingDirectory: buildSettings.workingDirectory.map(AbsolutePath.init(validating:))
)
}

private func runIndexingProcess(
indexFile: DocumentURI,
buildSettings: FileBuildSettings,
processArguments: [String],
workingDirectory: AbsolutePath?
) async throws {
let process = try Process.launch(
arguments: processArguments,
workingDirectory: workingDirectory
)
let result = try await process.waitUntilExitSendingSigIntOnTaskCancellation()
switch result.exitStatus.exhaustivelySwitchable {
case .terminated(code: 0):
Expand All @@ -205,26 +247,26 @@ public struct UpdateIndexStoreTaskDescription: TaskDescriptionProtocol {
// Indexing will frequently fail if the source code is in an invalid state. Thus, log the failure at a low level.
logger.debug(
"""
Updating index store for Swift file \(uri.forLogging) terminated with non-zero exit code \(code)
Updating index store for \(indexFile.forLogging) terminated with non-zero exit code \(code)
Stderr:
\(stderr)
Stdout:
\(stdout)
"""
)
BuildSettingsLogger.log(level: .debug, settings: buildSettings, for: uri)
BuildSettingsLogger.log(level: .debug, settings: buildSettings, for: indexFile)
case .signalled(signal: let signal):
if !Task.isCancelled {
// The indexing job finished with a signal. Could be because the compiler crashed.
// Ignore signal exit codes if this task has been cancelled because the compiler exits with SIGINT if it gets
// interrupted.
logger.error("Updating index store for Swift file \(uri.forLogging) signaled \(signal)")
BuildSettingsLogger.log(level: .error, settings: buildSettings, for: uri)
logger.error("Updating index store for \(indexFile.forLogging) signaled \(signal)")
BuildSettingsLogger.log(level: .error, settings: buildSettings, for: indexFile)
}
case .abnormal(exception: let exception):
if !Task.isCancelled {
logger.error("Updating index store for Swift file \(uri.forLogging) exited abnormally \(exception)")
BuildSettingsLogger.log(level: .error, settings: buildSettings, for: uri)
logger.error("Updating index store for \(indexFile.forLogging) exited abnormally \(exception)")
BuildSettingsLogger.log(level: .error, settings: buildSettings, for: indexFile)
}
}
}
Expand Down Expand Up @@ -304,3 +346,56 @@ private func adjustSwiftCompilerArgumentsForIndexStoreUpdate(
]
return result
}

/// Adjust compiler arguments that were created for building to compiler arguments that should be used for indexing.
///
/// This removes compiler arguments that produce output files and adds arguments to index the file.
private func adjustClangCompilerArgumentsForIndexStoreUpdate(
_ compilerArguments: [String],
fileToIndex: DocumentURI
) -> [String] {
let removeFlags: Set<String> = [
// Disable writing of a depfile
"-M",
"-MD",
"-MMD",
"-MG",
"-MM",
"-MV",
// Don't create phony targets
"-MP",
// Don't writ out compilation databases
"-MJ",
// Continue in the presence of errors during indexing
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this the reason? Or is it just that we don't have a decent timestamp to use for the "build session" here? Or does having this enable verification where we wouldn't otherwise?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Honestly, I don’t know. I was trying to infer it but maybe I was wrong. I just removed the explanation for now.

"-fmodules-validate-once-per-build-session",
// Don't compile
"-c",
]

let removeArguments: Set<String> = [
// Disable writing of a depfile
"-MT",
"-MF",
"-MQ",
// Don't write serialized diagnostic files
"--serialize-diagnostics",
]

var result: [String] = []
result.reserveCapacity(compilerArguments.count)
var iterator = compilerArguments.makeIterator()
while let argument = iterator.next() {
if removeFlags.contains(argument) || argument.starts(with: "-fbuild-session-file=") {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does serailize-diagnostics also allow =? Does -fbuild-session-file allow a separate arg rather than =?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, serailize-diagnostics does not allow = and -fbuild-session-file requires a =. But eg. -MT doesn’t require a space. And serialize-diagnostics can be spelled with a single or multiple spaces.

I ended up generalizing the entire command line argument matching in #1314

continue
}
if removeArguments.contains(argument) {
_ = iterator.next()
continue
}
result.append(argument)
}
result.append(
"-fsyntax-only"
)
return result
}
44 changes: 44 additions & 0 deletions Tests/SourceKitLSPTests/BackgroundIndexingTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -284,4 +284,48 @@ final class BackgroundIndexingTests: XCTestCase {
]
)
}

func testIndexCFile() async throws {
let project = try await SwiftPMTestProject(
files: [
"MyLibrary/include/dummy.h": "",
"MyFile.c": """
void 1️⃣someFunc() {}

void 2️⃣test() {
3️⃣someFunc();
}
""",
],
serverOptions: backgroundIndexingOptions
)

let (uri, positions) = try project.openDocument("MyFile.c")
let prepare = try await project.testClient.send(
CallHierarchyPrepareRequest(textDocument: TextDocumentIdentifier(uri), position: positions["1️⃣"])
)
let calls = try await project.testClient.send(
CallHierarchyIncomingCallsRequest(item: try XCTUnwrap(prepare?.only))
)
XCTAssertEqual(
calls,
[
CallHierarchyIncomingCall(
from: CallHierarchyItem(
name: "test",
kind: .function,
tags: nil,
uri: uri,
range: Range(positions["2️⃣"]),
selectionRange: Range(positions["2️⃣"]),
data: .dictionary([
"usr": .string("c:@F@test"),
"uri": .string(uri.stringValue),
])
),
fromRanges: [Range(positions["3️⃣"])]
)
]
)
}
}