Skip to content
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

Suggest the minimal type disambiguation when an overload doesn't have any unique types #1087

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
6109d28
Suggested only the minimal type disambiguation
d-ronnqvist Nov 4, 2024
4d2d482
Support disambiguating using a mix of parameter types and return types
d-ronnqvist Nov 4, 2024
0b629a3
Skip checking columns that are common for all overloads
d-ronnqvist Nov 5, 2024
7684ae0
Use Swift Algorithms package for combinations
d-ronnqvist Nov 5, 2024
d9864ca
Use specialized Set implementation for few overloads and with types
d-ronnqvist Nov 5, 2024
b010bb9
Allow each Int Set to specialize its creation of combinations
d-ronnqvist Nov 5, 2024
bf66819
Avoid mapping combinations for large sizes to Set<Int>
d-ronnqvist Nov 5, 2024
2c2d7ef
Avoid reallocations when generating "tiny int set" combinations
d-ronnqvist Nov 5, 2024
4ce6917
Avoid indexing into a nested array
d-ronnqvist Nov 5, 2024
efa6fcc
Speed up _TinySmallValueIntSet iteration
d-ronnqvist Nov 5, 2024
596cd90
Avoid accessing a Set twice to check if a value exist and remove it
d-ronnqvist Nov 5, 2024
d48ac32
Avoid temporary allocation when creating set of remaining node IDs
d-ronnqvist Nov 5, 2024
b41fff4
Avoid reallocating the collisions list
d-ronnqvist Nov 5, 2024
07dad86
Use a custom `_TinySmallValueIntSet.isSuperset(of:)` implementation
d-ronnqvist Nov 6, 2024
bc5663f
Use `Table<String>` instead of indexing into `[[String]]`
d-ronnqvist Nov 5, 2024
e6f60c8
Avoid recomputing the type name combinations to check
d-ronnqvist Nov 6, 2024
16e15d0
Compare the type name lengths by number of UTF8 code units
d-ronnqvist Nov 6, 2024
b962d1c
Update code comments, variable names, and internal documentation
d-ronnqvist Nov 6, 2024
e29fc0f
Avoid recomputing type name overlap
d-ronnqvist Nov 6, 2024
a2337ad
Merge branch 'main' into suggest-minimal-type-disambiguation
d-ronnqvist Nov 8, 2024
16a3eef
Fix Swift 5.9 compatibility
d-ronnqvist Nov 11, 2024
770694e
Initialize each `Table` element. Linux requires this.
d-ronnqvist Nov 11, 2024
562d502
Address code review feedback:
d-ronnqvist Nov 12, 2024
e6a5d93
Add detailed comment with example about how to find the fewest type n…
d-ronnqvist Nov 12, 2024
ee94641
Merge branch 'main' into suggest-minimal-type-disambiguation
d-ronnqvist Nov 12, 2024
9170fa2
Don't use swift-algorithm as a _local_ dependency in Swift.org CI
d-ronnqvist Nov 12, 2024
639f131
Add additional test for 70 parameter type disambiguation
d-ronnqvist Nov 13, 2024
00da0a1
Add additional test that overloads with all the same parameters fallb…
d-ronnqvist Nov 13, 2024
178ef42
Remove Swift Algorithms dependency.
d-ronnqvist Nov 13, 2024
9c98702
Merge branch 'main' into suggest-minimal-type-disambiguation
d-ronnqvist Nov 13, 2024
06b4a69
Only try mixed type disambiguation when symbol has both parameters an…
d-ronnqvist Nov 13, 2024
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
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