-
Notifications
You must be signed in to change notification settings - Fork 159
Support documenting platform specific properties and return values #979
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -278,7 +278,35 @@ struct ParametersAndReturnValidator { | |||||||||||||||||
| guard let signature = mixin.getValueIfPresent(for: SymbolGraph.Symbol.FunctionSignature.self) else { | ||||||||||||||||||
| continue | ||||||||||||||||||
| } | ||||||||||||||||||
| signatures[DocumentationDataVariantsTrait(for: selector)] = signature | ||||||||||||||||||
|
|
||||||||||||||||||
| let trait = DocumentationDataVariantsTrait(for: selector) | ||||||||||||||||||
| // Check if we've already encountered a different signature for another platform | ||||||||||||||||||
| guard var existing = signatures.removeValue(forKey: trait) else { | ||||||||||||||||||
| signatures[trait] = signature | ||||||||||||||||||
| continue | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| // An internal helper function that compares parameter names | ||||||||||||||||||
| func hasSameNames(_ lhs: SymbolGraph.Symbol.FunctionSignature.FunctionParameter, _ rhs: SymbolGraph.Symbol.FunctionSignature.FunctionParameter) -> Bool { | ||||||||||||||||||
| lhs.name == rhs.name && lhs.externalName == rhs.externalName | ||||||||||||||||||
| } | ||||||||||||||||||
| // If the two signatures have different parameters, add any missing parameters. | ||||||||||||||||||
| // This allows for documenting parameters that are only available on some platforms. | ||||||||||||||||||
| // | ||||||||||||||||||
| // Note: Doing this redundant `elementsEqual(_:by:)` check is significantly faster in the common case when all platforms have the same signature. | ||||||||||||||||||
| // In the rare case where platforms have different signatures, the overhead of checking `elementsEqual(_:by:)` first is too small to measure. | ||||||||||||||||||
| if !existing.parameters.elementsEqual(signature.parameters, by: hasSameNames) { | ||||||||||||||||||
| for case .insert(offset: let offset, element: let element, _) in signature.parameters.difference(from: existing.parameters, by: hasSameNames) { | ||||||||||||||||||
| existing.parameters.insert(element, at: offset) | ||||||||||||||||||
| } | ||||||||||||||||||
| } | ||||||||||||||||||
|
Comment on lines
+298
to
+302
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The extra if-statement isn't necessary but it's faster in the common case where the API has the same signature on all platforms. Here's a chart (that I made just now) of a microbenchmark where I create 1000 fake symbols, that each exist on two platforms and have an increasing number of fake parameters named "AAAAAAA", "BBBBBBB", "CCCCCCC", etc. on both platforms.
You can see that the If I update the benchmark to have minor differences between the parameters of each platform for all 1000 fake symbols, the overhead of the redundant
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'll add a shorter version of this comment to the code so that someone else (including me when I inevitably forget about this) doesn't remove the if-statement in the future. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thank you for the deep-dive, sounds great! |
||||||||||||||||||
|
|
||||||||||||||||||
| // If the already encountered signature has a void return type, replace it with the non-void return type. | ||||||||||||||||||
| // This allows for documenting the return values that are only available on some platforms. | ||||||||||||||||||
| if existing.returns != signature.returns, existing.returns == knownVoidReturnValuesByLanguage[.init(id: selector.interfaceLanguage)] { | ||||||||||||||||||
| existing.returns = signature.returns | ||||||||||||||||||
| } | ||||||||||||||||||
| signatures[trait] = existing | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| guard !signatures.isEmpty else { return nil } | ||||||||||||||||||
|
|
||||||||||||||||||


Uh oh!
There was an error while loading. Please reload this page.