Skip to content

Commit

Permalink
Suggest the minimal type disambiguation when an overload doesn't have…
Browse files Browse the repository at this point in the history
… 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
  • Loading branch information
d-ronnqvist authored Nov 13, 2024
1 parent 50c9904 commit 3cfeeb6
Show file tree
Hide file tree
Showing 4 changed files with 1,117 additions and 32 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ extension PathHierarchy {
extension PathHierarchy.DisambiguationContainer {

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

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

var remainingIDs = Set(elements.map(\.node.identifier))
var remainingIDs = Set<ResolvedIdentifier>()
remainingIDs.reserveCapacity(elements.count)
for element in elements {
guard let id = element.node.identifier else { continue }
remainingIDs.insert(id)
}

// Kind disambiguation is the most readable, so we start by checking if any element has a unique kind.
let groupedByKind = [String?: [Element]](grouping: elements, by: \.kind)
Expand All @@ -233,7 +240,9 @@ extension PathHierarchy.DisambiguationContainer {
collisions += _disambiguateByTypeSignature(
elementsThatSupportAdvancedDisambiguation,
types: \.returnTypes,
makeDisambiguation: Disambiguation.returnTypes,
makeDisambiguation: { _, disambiguatingTypeNames in
.returnTypes(disambiguatingTypeNames)
},
remainingIDs: &remainingIDs
)
if remainingIDs.isEmpty {
Expand All @@ -243,7 +252,31 @@ extension PathHierarchy.DisambiguationContainer {
collisions += _disambiguateByTypeSignature(
elementsThatSupportAdvancedDisambiguation,
types: \.parameterTypes,
makeDisambiguation: Disambiguation.parameterTypes,
makeDisambiguation: { _, disambiguatingTypeNames in
.parameterTypes(disambiguatingTypeNames)
},
remainingIDs: &remainingIDs
)
if remainingIDs.isEmpty {
return collisions
}

collisions += _disambiguateByTypeSignature(
elementsThatSupportAdvancedDisambiguation,
types: { element in
guard let parameterTypes = element.parameterTypes,
!parameterTypes.isEmpty,
let returnTypes = element.returnTypes,
!returnTypes.isEmpty
else {
return nil
}
return parameterTypes + returnTypes
},
makeDisambiguation: { element, disambiguatingTypeNames in
let numberOfReturnTypes = element.returnTypes?.count ?? 0
return .mixedTypes(parameterTypes: disambiguatingTypeNames.dropLast(numberOfReturnTypes), returnTypes: disambiguatingTypeNames.suffix(numberOfReturnTypes))
},
remainingIDs: &remainingIDs
)
if remainingIDs.isEmpty {
Expand Down Expand Up @@ -341,6 +374,8 @@ extension PathHierarchy.DisambiguationContainer {
case parameterTypes([String])
/// This node is disambiguated by its return types.
case returnTypes([String])
/// This node is disambiguated by a mix of parameter types and return types.
case mixedTypes(parameterTypes: [String], returnTypes: [String])

/// Makes a new disambiguation suffix string.
func makeSuffix() -> String {
Expand All @@ -361,6 +396,9 @@ extension PathHierarchy.DisambiguationContainer {
case .parameterTypes(let types):
// For example: "-(String,_)" or "-(_,Int)"` (a certain parameter has a certain type), or "-()" (has no parameters).
return "-(\(types.joined(separator: ",")))"

case .mixedTypes(parameterTypes: let parameterTypes, returnTypes: let returnTypes):
return Self.parameterTypes(parameterTypes).makeSuffix() + Self.returnTypes(returnTypes).makeSuffix()
}
}

Expand All @@ -373,7 +411,7 @@ extension PathHierarchy.DisambiguationContainer {
return kind.map { .kind($0) } ?? self
case .hash:
return hash.map { .hash($0) } ?? self
case .parameterTypes, .returnTypes:
case .parameterTypes, .returnTypes, .mixedTypes:
return self
}
}
Expand All @@ -382,36 +420,46 @@ extension PathHierarchy.DisambiguationContainer {
private static func _disambiguateByTypeSignature(
_ elements: [Element],
types: (Element) -> [String]?,
makeDisambiguation: ([String]) -> Disambiguation,
remainingIDs: inout Set<ResolvedIdentifier?>
makeDisambiguation: (Element, [String]) -> Disambiguation,
remainingIDs: inout Set<ResolvedIdentifier>
) -> [(value: PathHierarchy.Node, disambiguation: Disambiguation)] {
var collisions: [(value: PathHierarchy.Node, disambiguation: Disambiguation)] = []
// Assume that all elements will find a disambiguation (or close to it)
collisions.reserveCapacity(elements.count)

let groupedByTypeCount = [Int?: [Element]](grouping: elements, by: { types($0)?.count })
for (typesCount, elements) in groupedByTypeCount {
guard let typesCount else { continue }
guard elements.count > 1 else {
typealias ElementAndTypeNames = (element: Element, typeNames: [String])
var groupedByTypeCount: [Int: [ElementAndTypeNames]] = [:]
for element in elements {
guard let typeNames = types(element) else { continue }

groupedByTypeCount[typeNames.count, default: []].append((element, typeNames))
}

for (numberOfTypeNames, elementAndTypeNamePairs) in groupedByTypeCount {
guard elementAndTypeNamePairs.count > 1 else {
// Only one element has this number of types. Disambiguate with only underscores.
let element = elements.first!
guard remainingIDs.contains(element.node.identifier) else { continue } // Don't disambiguate the same element more than once
collisions.append((value: element.node, disambiguation: makeDisambiguation(.init(repeating: "_", count: typesCount))))
remainingIDs.remove(element.node.identifier)
let (element, _) = elementAndTypeNamePairs.first!
guard remainingIDs.remove(element.node.identifier) != nil else {
continue // Don't disambiguate the same element more than once
}
collisions.append((value: element.node, disambiguation: makeDisambiguation(element, .init(repeating: "_", count: numberOfTypeNames))))
continue
}
guard typesCount > 0 else { continue } // Need at least one return value to disambiguate

for typeIndex in 0..<typesCount {
let grouped = [String: [Element]](grouping: elements, by: { types($0)![typeIndex] })
for (returnType, elements) in grouped where elements.count == 1 {
// Only one element has this return type
let element = elements.first!
guard remainingIDs.contains(element.node.identifier) else { continue } // Don't disambiguate the same element more than once
var disambiguation = [String](repeating: "_", count: typesCount)
disambiguation[typeIndex] = returnType
collisions.append((value: element.node, disambiguation: makeDisambiguation(disambiguation)))
remainingIDs.remove(element.node.identifier)
continue
guard numberOfTypeNames > 0 else {
continue // Need at least one type name to disambiguate (when there are multiple elements without parameters or return values)
}

let suggestedDisambiguations = minimalSuggestedDisambiguation(forOverloadsAndTypeNames: elementAndTypeNamePairs)

for (pair, disambiguation) in zip(elementAndTypeNamePairs, suggestedDisambiguations) {
guard let disambiguation else {
continue // This element can't be uniquely disambiguated using these types
}
guard remainingIDs.remove(pair.element.node.identifier) != nil else {
continue // Don't disambiguate the same element more than once
}
collisions.append((value: pair.element.node, disambiguation: makeDisambiguation(pair.element, disambiguation)))
}
}
return collisions
Expand Down
Loading

0 comments on commit 3cfeeb6

Please sign in to comment.