Validate C++ and ObjC related args for Darwin toolchain #184
Conversation
This commit validates the following arguments to driver: 1. link-objc-runtime by checkin arc path, 2. experimentalCxxStdlib flag to limit it to libc++ 3. -static-stdlib to flag that its not supported. This commit is for SR-13255
|
@swift-ci test |
There was a problem hiding this comment.
This looks good, thanks! I just added a few minor comments. To answer your question about formatting from the JIRA bug, we usually try to follow the standard library style as described in https://github.com/apple/swift/blob/master/docs/StandardLibraryProgrammersManual.md, but there's no automated tool and the codebase isn't entirely consistent.
| // TODO: Validating darwin unsupported -static-stdlib argument. | ||
| // TODO: If a C++ standard library is specified, it has to be libc++. | ||
| // Validating darwin unsupported -static-stdlib argument. | ||
| if parsedOptions.hasArgument(.staticStdlib) && targetTriple.isDarwin { |
There was a problem hiding this comment.
I don't think targetTriple.isDarwin is needed here since we've already selected a Darwin toolchain.
There was a problem hiding this comment.
Darwin toolchain can build non Darwin targets, right? Distinction between the host and target.
| @@ -1540,6 +1540,23 @@ final class SwiftDriverTests: XCTestCase { | |||
| return | |||
| } | |||
| } | |||
|
|
|||
| XCTAssertThrowsError(try Driver(args: ["swiftc", "-c", "-static-stdlib", | |||
There was a problem hiding this comment.
These tests might need to include something like "-target", "x86_64-apple-macosx10.4" so they select the right toolchain when run on Linux
There was a problem hiding this comment.
Sure, will add the args for all the three tests. I thought since the test method was called testDarwinToolchainArgumentValidationi thought it wont be tested in Linux by default.
There was a problem hiding this comment.
Sorry, accidentally clicked approve instead of comment, we'll need to fix the Linux tests before this is ready to merge. I think you can either add a Darwin -target to the driver arguments, or wrap the new tests in #if os(macOS).
Edit: The Linux test run wasn't reported on the PR, but you can see the results here: https://ci.swift.org/view/Pull%20Request/job/pr-swift-driver-linux/337/consoleFull#-16151394033122a513-f36a-4c87-8ed7-cbc36a1ec144
This commit if added will 1. Removes redundant isDarwin check for -static-stdlib DarwinToolchain arg validation , 2. Adds Darwin platform target for all 3 tests introduced,
|
@swift-ci test |
This commit validates the following arguments to driver:
This commit is for SR-13255 https://bugs.swift.org/browse/SR-13255