Skip to content

Commit abc099c

Browse files
committed
[xcodegen] Match common prefix in CommandArgTree
Rather than forming a set intersection, only look for a common prefix with the parent arguments. This is both quicker and more correct since arguments can be order-dependent. This does mean we'll end up with more per-file arguments for "umbrella" targets, but that at least will no longer prevent us from forming buildable folders for them.
1 parent 22f1105 commit abc099c

File tree

4 files changed

+29
-74
lines changed

4 files changed

+29
-74
lines changed

utils/swift-xcodegen/Sources/SwiftXcodeGen/BuildArgs/ClangBuildArgsProvider.swift

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -58,17 +58,16 @@ struct ClangBuildArgsProvider {
5858

5959
/// Retrieve the arguments at a given path, including those in the parent.
6060
func getArgs(for path: RelativePath) -> BuildArgs {
61-
// Sort the arguments to get a deterministic ordering.
6261
// FIXME: We ought to get the command from the arg tree.
63-
.init(for: .clang, args: args.getArgs(for: path).sorted())
62+
.init(for: .clang, args: args.getArgs(for: path))
6463
}
6564

6665
/// Retrieve the arguments at a given path, excluding those already covered
6766
/// by a parent.
6867
func getUniqueArgs(
6968
for path: RelativePath, parent: RelativePath, infer: Bool = false
7069
) -> BuildArgs {
71-
var fileArgs: Set<Command.Argument> = []
70+
var fileArgs: [Command.Argument] = []
7271
if hasBuildArgs(for: path) {
7372
fileArgs = args.getUniqueArgs(for: path, parent: parent)
7473
} else if infer {
@@ -78,14 +77,8 @@ struct ClangBuildArgsProvider {
7877
fileArgs = args.getUniqueArgs(for: component, parent: parent)
7978
}
8079
}
81-
// Sort the arguments to get a deterministic ordering.
8280
// FIXME: We ought to get the command from the arg tree.
83-
return .init(for: .clang, args: fileArgs.sorted())
84-
}
85-
86-
/// Whether the given path has any unique args not covered by `parent`.
87-
func hasUniqueArgs(for path: RelativePath, parent: RelativePath) -> Bool {
88-
args.hasUniqueArgs(for: path, parent: parent)
81+
return .init(for: .clang, args: fileArgs)
8982
}
9083

9184
/// Whether the given file has build arguments.

utils/swift-xcodegen/Sources/SwiftXcodeGen/BuildArgs/CommandArgTree.swift

Lines changed: 14 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -11,49 +11,43 @@
1111
//===----------------------------------------------------------------------===//
1212

1313
/// A tree of compile command arguments, indexed by path such that those unique
14-
/// to a particular file can be queried, with common arguments associated
15-
/// with a common parent.
14+
/// to a particular file can be queried, with common-prefixed arguments
15+
/// associated with a common parent.
1616
struct CommandArgTree {
17-
private var storage: [RelativePath: Set<Command.Argument>]
17+
private var storage: [RelativePath: [Command.Argument]]
1818

1919
init() {
2020
self.storage = [:]
2121
}
2222

2323
mutating func insert(_ args: [Command.Argument], for path: RelativePath) {
24-
let args = Set(args)
2524
for component in path.stackedComponents {
2625
// If we haven't added any arguments, add them. If we're adding arguments
2726
// for the file itself, this is the only way we'll add arguments,
28-
// otherwise we can form an intersection with the other arguments.
27+
// otherwise we can form a common prefix with the other arguments.
2928
let inserted = storage.insertValue(args, for: component)
3029
guard !inserted && component != path else { continue }
3130

32-
// We use subscript(_:default:) to mutate in-place without CoW.
33-
storage[component, default: []].formIntersection(args)
31+
// We use withValue(for:default:) to mutate in-place without CoW.
32+
storage.withValue(for: component, default: []) { existingArgs in
33+
let slice = existingArgs.commonPrefix(with: args)
34+
existingArgs.removeSubrange(slice.endIndex...)
35+
}
3436
}
3537
}
3638

3739
/// Retrieve the arguments at a given path, including those in the parent.
38-
func getArgs(for path: RelativePath) -> Set<Command.Argument> {
40+
func getArgs(for path: RelativePath) -> [Command.Argument] {
3941
storage[path] ?? []
4042
}
4143

4244
/// Retrieve the arguments at a given path, excluding those already covered
4345
/// by a given parent.
4446
func getUniqueArgs(
4547
for path: RelativePath, parent: RelativePath
46-
) -> Set<Command.Argument> {
47-
getArgs(for: path).subtracting(getArgs(for: parent))
48-
}
49-
50-
/// Whether the given path has any unique args not covered by `parent`.
51-
func hasUniqueArgs(for path: RelativePath, parent: RelativePath) -> Bool {
52-
let args = getArgs(for: path)
53-
guard !args.isEmpty else { return false }
54-
// Assuming `parent` is an ancestor of path, the arguments for parent is
55-
// guaranteed to be a subset of the arguments for `path`. As such, we
56-
// only have to compare sizes here.
57-
return args.count != getArgs(for: parent).count
48+
) -> [Command.Argument] {
49+
let childArgs = getArgs(for: path)
50+
let parentArgs = getArgs(for: parent)
51+
return Array(childArgs[parentArgs.count...])
5852
}
5953
}

utils/swift-xcodegen/Sources/SwiftXcodeGen/Command/Command.swift

Lines changed: 0 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -240,47 +240,3 @@ extension Command.Flag: CustomStringConvertible {
240240
extension Command.Option: CustomStringConvertible {
241241
var description: String { printed }
242242
}
243-
244-
// MARK: Comparable
245-
// We sort the resulting command-line arguments to ensure deterministic
246-
// ordering.
247-
248-
extension Command.Flag: Comparable {
249-
static func < (lhs: Self, rhs: Self) -> Bool {
250-
guard lhs.dash == rhs.dash else {
251-
return lhs.dash < rhs.dash
252-
}
253-
return lhs.name.rawValue < rhs.name.rawValue
254-
}
255-
}
256-
257-
extension Command.Option: Comparable {
258-
static func < (lhs: Self, rhs: Self) -> Bool {
259-
guard lhs.flag == rhs.flag else {
260-
return lhs.flag < rhs.flag
261-
}
262-
guard lhs.spacing == rhs.spacing else {
263-
return lhs.spacing < rhs.spacing
264-
}
265-
return lhs.value < rhs.value
266-
}
267-
}
268-
269-
extension Command.Argument: Comparable {
270-
static func < (lhs: Self, rhs: Self) -> Bool {
271-
switch (lhs, rhs) {
272-
// Sort flags < options < values
273-
case (.flag, .option): true
274-
case (.flag, .value): true
275-
case (.option, .value): true
276-
277-
case (.option, .flag): false
278-
case (.value, .flag): false
279-
case (.value, .option): false
280-
281-
case (.flag(let lhs), .flag(let rhs)): lhs < rhs
282-
case (.option(let lhs), .option(let rhs)): lhs < rhs
283-
case (.value(let lhs), .value(let rhs)): lhs < rhs
284-
}
285-
}
286-
}

utils/swift-xcodegen/Sources/SwiftXcodeGen/Utils.swift

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,18 @@ extension Sequence {
3434
}
3535
}
3636

37+
extension Collection where Element: Equatable {
38+
func commonPrefix(with other: some Collection<Element>) -> SubSequence {
39+
var (i, j) = (self.startIndex, other.startIndex)
40+
while i < self.endIndex, j < other.endIndex {
41+
guard self[i] == other[j] else { break }
42+
self.formIndex(after: &i)
43+
other.formIndex(after: &j)
44+
}
45+
return self[..<i]
46+
}
47+
}
48+
3749
extension String {
3850
init(utf8 buffer: UnsafeRawBufferPointer) {
3951
guard !buffer.isEmpty else {

0 commit comments

Comments
 (0)