Skip to content

Commit 16c60b5

Browse files
authored
Merge pull request #2322 from ahoppen/build-settings-from-sibling
Infer build settings from sibling file if a file’s target cannot be determined
2 parents 515260f + 76847db commit 16c60b5

File tree

3 files changed

+196
-92
lines changed

3 files changed

+196
-92
lines changed

Sources/BuildServerIntegration/BuildServerManager.swift

Lines changed: 130 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -1071,8 +1071,6 @@ package actor BuildServerManager: QueueBasedMessageHandler {
10711071

10721072
/// Returns the build settings for the given file in the given target.
10731073
///
1074-
/// If no target is given, this always returns fallback build settings.
1075-
///
10761074
/// Only call this method if it is known that `document` is a main file. Prefer `buildSettingsInferredFromMainFile`
10771075
/// otherwise. If `document` is a header file, this will most likely return fallback settings because header files
10781076
/// don't have build settings by themselves.
@@ -1081,42 +1079,88 @@ package actor BuildServerManager: QueueBasedMessageHandler {
10811079
/// `SourceKitLSPOptions.buildSettingsTimeoutOrDefault`.
10821080
package func buildSettings(
10831081
for document: DocumentURI,
1084-
in target: BuildTargetIdentifier?,
1082+
in target: BuildTargetIdentifier,
10851083
language: Language,
10861084
fallbackAfterTimeout: Bool
10871085
) async -> FileBuildSettings? {
1088-
if let target {
1089-
let buildSettingsFromBuildServer = await orLog("Getting build settings") {
1090-
if fallbackAfterTimeout {
1091-
try await withTimeout(options.buildSettingsTimeoutOrDefault) {
1092-
return try await self.buildSettingsFromBuildServer(for: document, in: target, language: language)
1093-
} resultReceivedAfterTimeout: { _ in
1094-
await self.filesBuildSettingsChangedDebouncer.scheduleCall([document])
1095-
}
1096-
} else {
1097-
try await self.buildSettingsFromBuildServer(for: document, in: target, language: language)
1086+
let buildSettingsFromBuildServer = await orLog("Getting build settings") {
1087+
if fallbackAfterTimeout {
1088+
try await withTimeout(options.buildSettingsTimeoutOrDefault) {
1089+
return try await self.buildSettingsFromBuildServer(for: document, in: target, language: language)
1090+
} resultReceivedAfterTimeout: { _ in
1091+
await self.filesBuildSettingsChangedDebouncer.scheduleCall([document])
10981092
}
1099-
}
1100-
if let buildSettingsFromBuildServer {
1101-
return buildSettingsFromBuildServer
1093+
} else {
1094+
try await self.buildSettingsFromBuildServer(for: document, in: target, language: language)
11021095
}
11031096
}
1104-
1105-
guard
1106-
var settings = fallbackBuildSettings(
1097+
guard let buildSettingsFromBuildServer else {
1098+
return fallbackBuildSettings(
11071099
for: document,
11081100
language: language,
11091101
options: options.fallbackBuildSystemOrDefault
11101102
)
1111-
else {
1103+
}
1104+
return buildSettingsFromBuildServer
1105+
1106+
}
1107+
1108+
/// Try finding a source file with the same language as `document` in the same directory as `document` and patch its
1109+
/// build settings to provide more accurate fallback settings than the generic fallback settings.
1110+
private func fallbackBuildSettingsInferredFromSiblingFile(
1111+
of document: DocumentURI,
1112+
target explicitlyRequestedTarget: BuildTargetIdentifier?,
1113+
language: Language?,
1114+
fallbackAfterTimeout: Bool
1115+
) async throws -> FileBuildSettings? {
1116+
guard let documentFileURL = document.fileURL else {
11121117
return nil
11131118
}
1114-
if buildServerAdapter == nil {
1115-
// If there is no build server and we only have the fallback build server, we will never get real build settings.
1116-
// Consider the build settings non-fallback.
1117-
settings.isFallback = false
1119+
let directory = documentFileURL.deletingLastPathComponent()
1120+
guard let language = language ?? Language(inferredFromFileExtension: document) else {
1121+
return nil
11181122
}
1119-
return settings
1123+
let siblingFile = try await self.sourceFilesAndDirectories().files.compactMap { (uri, info) -> DocumentURI? in
1124+
guard info.isBuildable, uri.fileURL?.deletingLastPathComponent() == directory else {
1125+
return nil
1126+
}
1127+
if let explicitlyRequestedTarget, !info.targets.contains(explicitlyRequestedTarget) {
1128+
return nil
1129+
}
1130+
// Only consider build settings from sibling files that appear to have the same language. In theory, we might skip
1131+
// valid sibling files because of this since non-standard file extension might be mapped to `language` by the
1132+
// build server, but this is a good first check to avoid requesting build settings for too many documents. And
1133+
// since all of this is fallback-logic, skipping over possibly valid files is not a correctness issue.
1134+
guard let siblingLanguage = Language(inferredFromFileExtension: uri), siblingLanguage == language else {
1135+
return nil
1136+
}
1137+
return uri
1138+
}.sorted(by: { $0.pseudoPath < $1.pseudoPath }).first
1139+
1140+
guard let siblingFile else {
1141+
return nil
1142+
}
1143+
1144+
let siblingSettings = await self.buildSettingsInferredFromMainFile(
1145+
for: siblingFile,
1146+
target: explicitlyRequestedTarget,
1147+
language: language,
1148+
fallbackAfterTimeout: fallbackAfterTimeout,
1149+
allowInferenceFromSiblingFile: false
1150+
)
1151+
guard var siblingSettings, !siblingSettings.isFallback else {
1152+
return nil
1153+
}
1154+
siblingSettings.isFallback = true
1155+
switch language.semanticKind {
1156+
case .swift:
1157+
siblingSettings.compilerArguments += [try documentFileURL.filePath]
1158+
case .clang:
1159+
siblingSettings = siblingSettings.patching(newFile: document, originalFile: siblingFile)
1160+
case nil:
1161+
return nil
1162+
}
1163+
return siblingSettings
11201164
}
11211165

11221166
/// Returns the build settings for the given document.
@@ -1135,27 +1179,49 @@ package actor BuildServerManager: QueueBasedMessageHandler {
11351179
for document: DocumentURI,
11361180
target explicitlyRequestedTarget: BuildTargetIdentifier? = nil,
11371181
language: Language?,
1138-
fallbackAfterTimeout: Bool
1182+
fallbackAfterTimeout: Bool,
1183+
allowInferenceFromSiblingFile: Bool = true
11391184
) async -> FileBuildSettings? {
1185+
if buildServerAdapter == nil {
1186+
guard let language = language ?? Language(inferredFromFileExtension: document) else {
1187+
return nil
1188+
}
1189+
guard
1190+
var settings = fallbackBuildSettings(
1191+
for: document,
1192+
language: language,
1193+
options: options.fallbackBuildSystemOrDefault
1194+
)
1195+
else {
1196+
return nil
1197+
}
1198+
// If there is no build server and we only have the fallback build server, we will never get real build settings.
1199+
// Consider the build settings non-fallback.
1200+
settings.isFallback = false
1201+
return settings
1202+
}
1203+
11401204
func mainFileAndSettings(
11411205
basedOn document: DocumentURI
11421206
) async -> (mainFile: DocumentURI, settings: FileBuildSettings)? {
11431207
let mainFile = await self.mainFile(for: document, language: language)
1144-
let settings: FileBuildSettings? = await orLog("Getting build settings") {
1145-
let target =
1208+
let settings: FileBuildSettings? = await orLog("Getting build settings") { () -> FileBuildSettings? in
1209+
let target: WithTimeoutResult<BuildTargetIdentifier?> =
11461210
if let explicitlyRequestedTarget {
1147-
explicitlyRequestedTarget
1211+
.result(explicitlyRequestedTarget)
11481212
} else {
1149-
try await withTimeout(options.buildSettingsTimeoutOrDefault) {
1150-
await self.canonicalTarget(for: mainFile)
1213+
try await withTimeoutResult(options.buildSettingsTimeoutOrDefault) {
1214+
return await self.canonicalTarget(for: mainFile)
11511215
} resultReceivedAfterTimeout: { _ in
11521216
await self.filesBuildSettingsChangedDebouncer.scheduleCall([document])
11531217
}
11541218
}
11551219
var languageForFile: Language
11561220
if let language {
11571221
languageForFile = language
1158-
} else if let target, let language = await self.defaultLanguage(for: mainFile, in: target) {
1222+
} else if case let .result(target?) = target,
1223+
let language = await self.defaultLanguage(for: mainFile, in: target)
1224+
{
11591225
languageForFile = language
11601226
} else if let language = Language(inferredFromFileExtension: mainFile) {
11611227
languageForFile = language
@@ -1164,12 +1230,38 @@ package actor BuildServerManager: QueueBasedMessageHandler {
11641230
// settings.
11651231
return nil
11661232
}
1167-
return await self.buildSettings(
1168-
for: mainFile,
1169-
in: target,
1170-
language: languageForFile,
1171-
fallbackAfterTimeout: fallbackAfterTimeout
1172-
)
1233+
switch target {
1234+
case .result(let target?):
1235+
return await self.buildSettings(
1236+
for: mainFile,
1237+
in: target,
1238+
language: languageForFile,
1239+
fallbackAfterTimeout: fallbackAfterTimeout
1240+
)
1241+
case .result(nil):
1242+
if allowInferenceFromSiblingFile {
1243+
let settingsFromSibling = await orLog("Inferring build settings from sibling file") {
1244+
try await self.fallbackBuildSettingsInferredFromSiblingFile(
1245+
of: document,
1246+
target: explicitlyRequestedTarget,
1247+
language: language,
1248+
fallbackAfterTimeout: fallbackAfterTimeout
1249+
)
1250+
}
1251+
if let settingsFromSibling {
1252+
return settingsFromSibling
1253+
}
1254+
}
1255+
fallthrough
1256+
case .timedOut:
1257+
// If we timed out, we don't want to try inferring the build settings from a sibling since that would kick off
1258+
// new requests to the build server, which will likely also time out.
1259+
return fallbackBuildSettings(
1260+
for: document,
1261+
language: languageForFile,
1262+
options: options.fallbackBuildSystemOrDefault
1263+
)
1264+
}
11731265
}
11741266
guard let settings else {
11751267
return nil

Sources/SwiftExtensions/AsyncUtils.swift

Lines changed: 33 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -263,23 +263,28 @@ package func withTimeout<T: Sendable>(
263263
}
264264
}
265265

266-
/// Executes `body`. If it doesn't finish after `duration`, return `nil` and continue running body. When `body` returns
267-
/// a value after the timeout, `resultReceivedAfterTimeout` is called.
266+
package enum WithTimeoutResult<T> {
267+
case result(T)
268+
case timedOut
269+
}
270+
271+
/// Executes `body`. If it doesn't finish after `duration`, return `.timed` and continue running body. When `body`
272+
/// returns a value after the timeout, `resultReceivedAfterTimeout` is called.
268273
///
269274
/// - Important: `body` will not be cancelled when the timeout is received. Use the other overload of `withTimeout` if
270275
/// `body` should be cancelled after `timeout`.
271-
package func withTimeout<T: Sendable>(
276+
package func withTimeoutResult<T: Sendable>(
272277
_ timeout: Duration,
273278
body: @escaping @Sendable () async throws -> T,
274279
resultReceivedAfterTimeout: @escaping @Sendable (_ result: T) async -> Void
275-
) async throws -> T? {
280+
) async throws -> WithTimeoutResult<T> {
276281
let didHitTimeout = AtomicBool(initialValue: false)
277282

278-
let stream = AsyncThrowingStream<T?, Error> { continuation in
283+
let stream = AsyncThrowingStream<WithTimeoutResult<T>, Error> { continuation in
279284
Task {
280285
try await Task.sleep(for: timeout)
281286
didHitTimeout.value = true
282-
continuation.yield(nil)
287+
continuation.yield(.timedOut)
283288
}
284289

285290
Task {
@@ -288,7 +293,7 @@ package func withTimeout<T: Sendable>(
288293
if didHitTimeout.value {
289294
await resultReceivedAfterTimeout(result)
290295
}
291-
continuation.yield(result)
296+
continuation.yield(.result(result))
292297
} catch {
293298
continuation.yield(with: .failure(error))
294299
}
@@ -307,6 +312,27 @@ package func withTimeout<T: Sendable>(
307312
}
308313
}
309314

315+
/// Executes `body`. If it doesn't finish after `duration`, return `nil` and continue running body. When `body` returns
316+
/// a value after the timeout, `resultReceivedAfterTimeout` is called.
317+
///
318+
/// - Important: `body` will not be cancelled when the timeout is received. Use the other overload of `withTimeout` if
319+
/// `body` should be cancelled after `timeout`.
320+
package func withTimeout<T: Sendable>(
321+
_ timeout: Duration,
322+
body: @escaping @Sendable () async throws -> T,
323+
resultReceivedAfterTimeout: @escaping @Sendable (_ result: T) async -> Void
324+
) async throws -> T? {
325+
let timeoutResult: WithTimeoutResult<T> = try await withTimeoutResult(
326+
timeout,
327+
body: body,
328+
resultReceivedAfterTimeout: resultReceivedAfterTimeout
329+
)
330+
switch timeoutResult {
331+
case .timedOut: return nil
332+
case .result(let result): return result
333+
}
334+
}
335+
310336
/// Same as `withTimeout` above but allows `body` to return an optional value.
311337
package func withTimeout<T: Sendable>(
312338
_ timeout: Duration,

0 commit comments

Comments
 (0)