Skip to content

Verify usage of -o arguments in multi-threading mode. #711

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

Merged
merged 1 commit into from
Jun 11, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 28 additions & 0 deletions Sources/SwiftDriver/Driver/Driver.swift
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ public struct Driver {
case relativeFrontendPath(String)
case subcommandPassedToDriver
case integratedReplRemoved
case cannotSpecify_OForMultipleOutputs
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might help users to list the outputs in here, and print them with the error message.

case conflictingOptions(Option, Option)
case unableToLoadOutputFileMap(String)
case unableToDecodeFrontendTargetInfo(String?, [String], String)
Expand Down Expand Up @@ -57,6 +58,8 @@ public struct Driver {
return "subcommand passed to driver"
case .integratedReplRemoved:
return "Compiler-internal integrated REPL has been removed; use the LLDB-enhanced REPL instead."
case .cannotSpecify_OForMultipleOutputs:
return "cannot specify -o when generating multiple output files"
case .conflictingOptions(let one, let two):
return "conflicting options '\(one.spelling)' and '\(two.spelling)'"
case let .unableToDecodeFrontendTargetInfo(outputString, arguments, errorDesc):
Expand Down Expand Up @@ -693,6 +696,8 @@ public struct Driver {
compilerMode: compilerMode,
outputFileMap: self.outputFileMap,
moduleName: moduleOutputInfo.name)

try verifyOutputOptions()
}

public mutating func planBuild() throws -> [Job] {
Expand Down Expand Up @@ -778,6 +783,29 @@ extension Driver {
}
}

extension Driver {
// Detect mis-use of multi-threading and output file options
private func verifyOutputOptions() throws {
if compilerOutputType != .swiftModule,
parsedOptions.hasArgument(.o),
linkerOutputType == nil {
let shouldComplain: Bool
if numThreads > 0 {
// Multi-threading compilation has multiple outputs unless there's only
// one input.
shouldComplain = self.inputFiles.count > 1
} else {
// Single-threaded compilation is a problem if we're compiling more than
// one file.
shouldComplain = self.inputFiles.filter { $0.type.isPartOfSwiftCompilation }.count > 1 && .singleCompile != compilerMode
}
if shouldComplain {
diagnosticEngine.emit(Error.cannotSpecify_OForMultipleOutputs)
}
}
}
}

// MARK: - Response files.
extension Driver {
/// Tokenize a single line in a response file.
Expand Down
1 change: 0 additions & 1 deletion Sources/SwiftDriver/Jobs/Planning.swift
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,6 @@ extension Driver {
-> ([Job], IncrementalCompilationState?) {
precondition(compilerMode.isStandardCompilationForPlanning,
"compiler mode \(compilerMode) is handled elsewhere")

// Determine the initial state for incremental compilation that is required during
// the planning process. This state contains the module dependency graph and
// cross-module dependency information.
Expand Down
3 changes: 2 additions & 1 deletion Sources/SwiftDriverExecution/MultiJobExecutor.swift
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,8 @@ public final class MultiJobExecutor {
to producerMap: inout [VirtualPath.Handle: Int]
) {
for output in job.outputs {
if let otherJobIndex = producerMap.updateValue(index, forKey: output.fileHandle) {
if output.file != .standardOutput,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A comment explaining why the error is skipped for .standardOutput could help here.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, the updateValue gets skipped. Maybe producerMap should exclude stdouts? Does it already?? Is there a comment on the var??

let otherJobIndex = producerMap.updateValue(index, forKey: output.fileHandle) {
fatalError("multiple producers for output \(output.file): \(job) & \(knownJobs[otherJobIndex])")
}
producerMap[output.fileHandle] = index
Expand Down
10 changes: 10 additions & 0 deletions Tests/SwiftDriverTests/SwiftDriverTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -314,6 +314,16 @@ final class SwiftDriverTests: XCTestCase {
}
}

func testMultiThreadingOutputs() throws {
try assertDriverDiagnostics(args: "swiftc", "-c", "foo.swift", "bar.swift", "-o", "bar.ll", "-o", "foo.ll", "-num-threads", "2", "-whole-module-optimization") {
$1.expect(.error("cannot specify -o when generating multiple output files"))
}

try assertDriverDiagnostics(args: "swiftc", "-c", "foo.swift", "bar.swift", "-o", "bar.ll", "-o", "foo.ll", "-num-threads", "0") {
$1.expect(.error("cannot specify -o when generating multiple output files"))
}
}

func testBaseOutputPaths() throws {
// Test the combination of -c and -o includes the base output path.
do {
Expand Down