Skip to content

Commit 3cfeeb6

Browse files
authored
Suggest the minimal type disambiguation when an overload doesn't have any unique types (#1087)
* Suggested only the minimal type disambiguation rdar://136207880 * Support disambiguating using a mix of parameter types and return types * Skip checking columns that are common for all overloads * Use Swift Algorithms package for combinations * Use specialized Set implementation for few overloads and with types * Allow each Int Set to specialize its creation of combinations * Avoid mapping combinations for large sizes to Set<Int> * Avoid reallocations when generating "tiny int set" combinations * Avoid indexing into a nested array * Speed up _TinySmallValueIntSet iteration * Avoid accessing a Set twice to check if a value exist and remove it * Avoid temporary allocation when creating set of remaining node IDs Also, ignore "sparse" nodes (without IDs) * Avoid reallocating the collisions list * Use a custom `_TinySmallValueIntSet.isSuperset(of:)` implementation * Use `Table<String>` instead of indexing into `[[String]]` * Avoid recomputing the type name combinations to check * Compare the type name lengths by number of UTF8 code units * Update code comments, variable names, and internal documentation * Avoid recomputing type name overlap * Fix Swift 5.9 compatibility * Initialize each `Table` element. Linux requires this. * Address code review feedback: - Use plural for local variable with array value - Explicitly initialize optional local variable with `nil` - Add assert about passing empty type names - Explain what `nil` return value means in local code comment - Add comment indicating where `_TinySmallValueIntSet` is defined - Use + to join two string instead of string interpolation - Use named arguments for `makeDisambiguation` closure * Add detailed comment with example about how to find the fewest type names that disambiguate an overload * Don't use swift-algorithm as a _local_ dependency in Swift.org CI * Add additional test for 70 parameter type disambiguation * Add additional test that overloads with all the same parameters fallback to hash disambiguation * Remove Swift Algorithms dependency. For the extremely rare case of overloads with more than 64 parameters we only try disambiguation by a single parameter type name. * Only try mixed type disambiguation when symbol has both parameters and return value
1 parent 50c9904 commit 3cfeeb6

File tree

4 files changed

+1117
-32
lines changed

4 files changed

+1117
-32
lines changed

Sources/SwiftDocC/Infrastructure/Link Resolution/PathHierarchy+DisambiguatedPaths.swift

+76-28
Original file line numberDiff line numberDiff line change
@@ -189,7 +189,7 @@ extension PathHierarchy {
189189
extension PathHierarchy.DisambiguationContainer {
190190

191191
static func disambiguatedValues(
192-
for elements: some Sequence<Element>,
192+
for elements: some Collection<Element>,
193193
includeLanguage: Bool = false,
194194
allowAdvancedDisambiguation: Bool = true
195195
) -> [(value: PathHierarchy.Node, disambiguation: Disambiguation)] {
@@ -203,13 +203,20 @@ extension PathHierarchy.DisambiguationContainer {
203203
}
204204

205205
private static func _disambiguatedValues(
206-
for elements: some Sequence<Element>,
206+
for elements: some Collection<Element>,
207207
includeLanguage: Bool,
208208
allowAdvancedDisambiguation: Bool
209209
) -> [(value: PathHierarchy.Node, disambiguation: Disambiguation)] {
210210
var collisions: [(value: PathHierarchy.Node, disambiguation: Disambiguation)] = []
211+
// Assume that all elements will find a disambiguation (or close to it)
212+
collisions.reserveCapacity(elements.count)
211213

212-
var remainingIDs = Set(elements.map(\.node.identifier))
214+
var remainingIDs = Set<ResolvedIdentifier>()
215+
remainingIDs.reserveCapacity(elements.count)
216+
for element in elements {
217+
guard let id = element.node.identifier else { continue }
218+
remainingIDs.insert(id)
219+
}
213220

214221
// Kind disambiguation is the most readable, so we start by checking if any element has a unique kind.
215222
let groupedByKind = [String?: [Element]](grouping: elements, by: \.kind)
@@ -233,7 +240,9 @@ extension PathHierarchy.DisambiguationContainer {
233240
collisions += _disambiguateByTypeSignature(
234241
elementsThatSupportAdvancedDisambiguation,
235242
types: \.returnTypes,
236-
makeDisambiguation: Disambiguation.returnTypes,
243+
makeDisambiguation: { _, disambiguatingTypeNames in
244+
.returnTypes(disambiguatingTypeNames)
245+
},
237246
remainingIDs: &remainingIDs
238247
)
239248
if remainingIDs.isEmpty {
@@ -243,7 +252,31 @@ extension PathHierarchy.DisambiguationContainer {
243252
collisions += _disambiguateByTypeSignature(
244253
elementsThatSupportAdvancedDisambiguation,
245254
types: \.parameterTypes,
246-
makeDisambiguation: Disambiguation.parameterTypes,
255+
makeDisambiguation: { _, disambiguatingTypeNames in
256+
.parameterTypes(disambiguatingTypeNames)
257+
},
258+
remainingIDs: &remainingIDs
259+
)
260+
if remainingIDs.isEmpty {
261+
return collisions
262+
}
263+
264+
collisions += _disambiguateByTypeSignature(
265+
elementsThatSupportAdvancedDisambiguation,
266+
types: { element in
267+
guard let parameterTypes = element.parameterTypes,
268+
!parameterTypes.isEmpty,
269+
let returnTypes = element.returnTypes,
270+
!returnTypes.isEmpty
271+
else {
272+
return nil
273+
}
274+
return parameterTypes + returnTypes
275+
},
276+
makeDisambiguation: { element, disambiguatingTypeNames in
277+
let numberOfReturnTypes = element.returnTypes?.count ?? 0
278+
return .mixedTypes(parameterTypes: disambiguatingTypeNames.dropLast(numberOfReturnTypes), returnTypes: disambiguatingTypeNames.suffix(numberOfReturnTypes))
279+
},
247280
remainingIDs: &remainingIDs
248281
)
249282
if remainingIDs.isEmpty {
@@ -341,6 +374,8 @@ extension PathHierarchy.DisambiguationContainer {
341374
case parameterTypes([String])
342375
/// This node is disambiguated by its return types.
343376
case returnTypes([String])
377+
/// This node is disambiguated by a mix of parameter types and return types.
378+
case mixedTypes(parameterTypes: [String], returnTypes: [String])
344379

345380
/// Makes a new disambiguation suffix string.
346381
func makeSuffix() -> String {
@@ -361,6 +396,9 @@ extension PathHierarchy.DisambiguationContainer {
361396
case .parameterTypes(let types):
362397
// For example: "-(String,_)" or "-(_,Int)"` (a certain parameter has a certain type), or "-()" (has no parameters).
363398
return "-(\(types.joined(separator: ",")))"
399+
400+
case .mixedTypes(parameterTypes: let parameterTypes, returnTypes: let returnTypes):
401+
return Self.parameterTypes(parameterTypes).makeSuffix() + Self.returnTypes(returnTypes).makeSuffix()
364402
}
365403
}
366404

@@ -373,7 +411,7 @@ extension PathHierarchy.DisambiguationContainer {
373411
return kind.map { .kind($0) } ?? self
374412
case .hash:
375413
return hash.map { .hash($0) } ?? self
376-
case .parameterTypes, .returnTypes:
414+
case .parameterTypes, .returnTypes, .mixedTypes:
377415
return self
378416
}
379417
}
@@ -382,36 +420,46 @@ extension PathHierarchy.DisambiguationContainer {
382420
private static func _disambiguateByTypeSignature(
383421
_ elements: [Element],
384422
types: (Element) -> [String]?,
385-
makeDisambiguation: ([String]) -> Disambiguation,
386-
remainingIDs: inout Set<ResolvedIdentifier?>
423+
makeDisambiguation: (Element, [String]) -> Disambiguation,
424+
remainingIDs: inout Set<ResolvedIdentifier>
387425
) -> [(value: PathHierarchy.Node, disambiguation: Disambiguation)] {
388426
var collisions: [(value: PathHierarchy.Node, disambiguation: Disambiguation)] = []
427+
// Assume that all elements will find a disambiguation (or close to it)
428+
collisions.reserveCapacity(elements.count)
389429

390-
let groupedByTypeCount = [Int?: [Element]](grouping: elements, by: { types($0)?.count })
391-
for (typesCount, elements) in groupedByTypeCount {
392-
guard let typesCount else { continue }
393-
guard elements.count > 1 else {
430+
typealias ElementAndTypeNames = (element: Element, typeNames: [String])
431+
var groupedByTypeCount: [Int: [ElementAndTypeNames]] = [:]
432+
for element in elements {
433+
guard let typeNames = types(element) else { continue }
434+
435+
groupedByTypeCount[typeNames.count, default: []].append((element, typeNames))
436+
}
437+
438+
for (numberOfTypeNames, elementAndTypeNamePairs) in groupedByTypeCount {
439+
guard elementAndTypeNamePairs.count > 1 else {
394440
// Only one element has this number of types. Disambiguate with only underscores.
395-
let element = elements.first!
396-
guard remainingIDs.contains(element.node.identifier) else { continue } // Don't disambiguate the same element more than once
397-
collisions.append((value: element.node, disambiguation: makeDisambiguation(.init(repeating: "_", count: typesCount))))
398-
remainingIDs.remove(element.node.identifier)
441+
let (element, _) = elementAndTypeNamePairs.first!
442+
guard remainingIDs.remove(element.node.identifier) != nil else {
443+
continue // Don't disambiguate the same element more than once
444+
}
445+
collisions.append((value: element.node, disambiguation: makeDisambiguation(element, .init(repeating: "_", count: numberOfTypeNames))))
399446
continue
400447
}
401-
guard typesCount > 0 else { continue } // Need at least one return value to disambiguate
402448

403-
for typeIndex in 0..<typesCount {
404-
let grouped = [String: [Element]](grouping: elements, by: { types($0)![typeIndex] })
405-
for (returnType, elements) in grouped where elements.count == 1 {
406-
// Only one element has this return type
407-
let element = elements.first!
408-
guard remainingIDs.contains(element.node.identifier) else { continue } // Don't disambiguate the same element more than once
409-
var disambiguation = [String](repeating: "_", count: typesCount)
410-
disambiguation[typeIndex] = returnType
411-
collisions.append((value: element.node, disambiguation: makeDisambiguation(disambiguation)))
412-
remainingIDs.remove(element.node.identifier)
413-
continue
449+
guard numberOfTypeNames > 0 else {
450+
continue // Need at least one type name to disambiguate (when there are multiple elements without parameters or return values)
451+
}
452+
453+
let suggestedDisambiguations = minimalSuggestedDisambiguation(forOverloadsAndTypeNames: elementAndTypeNamePairs)
454+
455+
for (pair, disambiguation) in zip(elementAndTypeNamePairs, suggestedDisambiguations) {
456+
guard let disambiguation else {
457+
continue // This element can't be uniquely disambiguated using these types
458+
}
459+
guard remainingIDs.remove(pair.element.node.identifier) != nil else {
460+
continue // Don't disambiguate the same element more than once
414461
}
462+
collisions.append((value: pair.element.node, disambiguation: makeDisambiguation(pair.element, disambiguation)))
415463
}
416464
}
417465
return collisions

0 commit comments

Comments
 (0)