Fix swift-api-digester search paths
#7664
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Fixes an architecture and implementation bug where the include paths for `swift-api-digester` were being determined inside build plan instead of by the target build description. This is effectively the same change as #7621 but for api-digester rather than symbolgraph-extract. This commit also leaves hooks in to enable api-digester on clang modules as a future improvement.
rauhul
requested review from
bnbarham,
MaxDesiatov,
jakepetroules,
francescomikulis and
xedin
as code owners
|
@swift-ci test |
|
@swift-ci test linux |
MaxDesiatov
reviewed
Jun 14, 2024
| @@ -97,6 +97,16 @@ public struct IdentifiableSet<Element: Identifiable>: Collection { | |||
| public func contains(id: Element.ID) -> Bool { | |||
| self.storage.keys.contains(id) | |||
| } | |||
|
|
|||
| public func filter(_ isIncluded: (Self.Element) throws -> Bool) rethrows -> Self { | |||
There was a problem hiding this comment.
Suggested change
| public func filter(_ isIncluded: (Self.Element) throws -> Bool) rethrows -> Self { | |
| package func filter(_ isIncluded: (Self.Element) throws -> Bool) rethrows -> Self { |
MaxDesiatov
reviewed
Jun 14, 2024
| @@ -118,3 +128,9 @@ extension IdentifiableSet: Hashable { | |||
| } | |||
| } | |||
| } | |||
|
|
|||
| extension IdentifiableSet: ExpressibleByArrayLiteral { | |||
| public init(arrayLiteral elements: Element...) { | |||
There was a problem hiding this comment.
Suggested change
| public init(arrayLiteral elements: Element...) { | |
| package init(arrayLiteral elements: Element...) { |
|
CI compiler crash currently tracked as rdar://129847951 |
MaxDesiatov
reviewed
Jun 14, 2024
Comment on lines
+646
to
+658
| package func apiDigesterEmitBaselineArguments() throws -> [String] { | ||
| var args = [String]() | ||
| args += try self.apiDigesterCommonArguments() | ||
| return args | ||
| } | ||
|
|
||
| /// Determines the arguments needed to run `swift-api-digester` for | ||
| /// comparing to an API baseline for this module. | ||
| package func apiDigesterCompareBaselineArguments() throws -> [String] { | ||
| var args = [String]() | ||
| args += try self.apiDigesterCommonArguments() | ||
| return args | ||
| } |
There was a problem hiding this comment.
Unclear what these two methods are for if they just append the result of apiDigesterCommonArguments to an empty array?
MaxDesiatov
reviewed
Jun 14, 2024
Comment on lines
+613
to
+629
| /// Determines the arguments needed to run `swift-api-digester` for emitting | ||
| /// an API baseline for a particular module. | ||
| public func apiDigesterEmitBaselineArguments(for module: ResolvedModule) throws -> [String] { | ||
| guard let description = self.targetMap[module.id] else { | ||
| throw InternalError("Expected description for module \(module)") | ||
| } | ||
| return try description.apiDigesterEmitBaselineArguments() | ||
| } | ||
|
|
||
| /// Determines the arguments needed to run `swift-api-digester` for | ||
| /// comparing to an API baseline for a particular module. | ||
| public func apiDigesterCompareBaselineArguments(for module: ResolvedModule) throws -> [String] { | ||
| guard let description = self.targetMap[module.id] else { | ||
| throw InternalError("Expected description for module \(module)") | ||
| } | ||
| return try description.apiDigesterCompareBaselineArguments() | ||
| } |
There was a problem hiding this comment.
Ditto, it looks like these two functions are equivalent
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes an architecture and implementation bug where the include paths for
swift-api-digesterwere being determined inside build plan instead of by the target build description.This is effectively the same change as #7621 but for api-digester rather than symbolgraph-extract.
This commit also leaves hooks in to enable api-digester on clang modules as a future improvement.