From 06c4517c4e3b90bf638bbab0f290c93c690db8df Mon Sep 17 00:00:00 2001 From: Owen Voorhees Date: Sat, 29 Aug 2020 21:12:08 -0700 Subject: [PATCH] Preserve relative ordering in ParsedOptions.arguments(for options:) This is significant in at least one case (-F/-Fsystem ordering needs to be preserved), and isn't that expensive, so always preserve the ordering to ensure more predictable behavior --- Sources/SwiftOptions/ParsedOptions.swift | 10 ++++++---- Tests/SwiftDriverTests/SwiftDriverTests.swift | 13 +++++++++++++ 2 files changed, 19 insertions(+), 4 deletions(-) diff --git a/Sources/SwiftOptions/ParsedOptions.swift b/Sources/SwiftOptions/ParsedOptions.swift index 12ccc532b..de00727dc 100644 --- a/Sources/SwiftOptions/ParsedOptions.swift +++ b/Sources/SwiftOptions/ParsedOptions.swift @@ -191,11 +191,13 @@ extension ParsedOptions { } public mutating func arguments(for options: Option...) -> [ParsedOption] { - return options.flatMap { lookup($0) } + return arguments(for: options) } public mutating func arguments(for options: [Option]) -> [ParsedOption] { - return options.flatMap { lookup($0) } + // The relative ordering of different options is sometimes significant, so + // sort the results by their position in the command line. + return options.flatMap { lookup($0) }.sorted { $0.index < $1.index } } public mutating func arguments(in group: Option.Group) -> [ParsedOption] { @@ -203,11 +205,11 @@ extension ParsedOptions { } public mutating func last(for options: Option...) -> ParsedOption? { - return arguments(for: options).max { $0.index < $1.index } + return last(for: options) } public mutating func last(for options: [Option]) -> ParsedOption? { - return arguments(for: options).max { $0.index < $1.index } + return arguments(for: options).last } /// Return the last parsed options that matches the given predicate. diff --git a/Tests/SwiftDriverTests/SwiftDriverTests.swift b/Tests/SwiftDriverTests/SwiftDriverTests.swift index 53411a661..57e3e6140 100644 --- a/Tests/SwiftDriverTests/SwiftDriverTests.swift +++ b/Tests/SwiftDriverTests/SwiftDriverTests.swift @@ -124,6 +124,19 @@ final class SwiftDriverTests: XCTestCase { XCTAssertTrue(jobs[0].commandLine.contains(.joinedOptionAndPath("-F=", .relative(.init("other/relative/dir"))))) } + func testRelativeOptionOrdering() throws { + var driver = try Driver(args: ["swiftc", "foo.swift", + "-F", "/path/to/frameworks", + "-Fsystem", "/path/to/systemframeworks", + "-F", "/path/to/more/frameworks"]) + let jobs = try driver.planBuild() + XCTAssertEqual(jobs[0].kind, .compile) + // The relative ordering of -F and -Fsystem options should be preserved. + XCTAssertTrue(jobs[0].commandLine.contains(subsequence: [.flag("-F"), .path(.absolute(.init("/path/to/frameworks"))), + .flag("-Fsystem"), .path(.absolute(.init("/path/to/systemframeworks"))), + .flag("-F"), .path(.absolute(.init("/path/to/more/frameworks")))])) + } + func testBatchModeDiagnostics() throws { try assertNoDriverDiagnostics(args: "swiftc", "-enable-batch-mode") { driver in switch driver.compilerMode {