Skip to content

Verify emitted module interfaces by default and introduce blocklist #1543

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.

Sign up for GitHub

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
Feb 9, 2024
Merged

Verify emitted module interfaces by default and introduce blocklist #1543

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
29 changes: 21 additions & 8 deletions Sources/SwiftDriver/Jobs/Planning.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -550,18 +550,14 @@ extension Driver {

private mutating func addVerifyJobs(emitModuleJob: Job, addJob: (Job) -> Void )
throws {
// Turn this flag on by default with the env var or for public frameworks.
let onByDefault = env["ENABLE_DEFAULT_INTERFACE_VERIFIER"] != nil ||
parsedOptions.getLastArgument(.libraryLevel)?.asSingle == "api"

guard
// Only verify modules with library evolution.
parsedOptions.hasArgument(.enableLibraryEvolution),

// Only verify when requested, on by default and not disabled.
parsedOptions.hasFlag(positive: .verifyEmittedModuleInterface,
negative: .noVerifyEmittedModuleInterface,
default: onByDefault),
default: true),

// Don't verify by default modules emitted from a merge-module job
// as it's more likely to be invalid.
Expand All @@ -571,8 +567,25 @@ extension Driver {
default: false)
else { return }

let optIn = env["ENABLE_DEFAULT_INTERFACE_VERIFIER"] != nil ||
parsedOptions.hasArgument(.verifyEmittedModuleInterface)
// Downgrade errors to a warning for modules expected to fail this check.
var knownFailingModules: Set = ["TestBlocklistedModule"]
knownFailingModules = knownFailingModules.union(
Driver.getAllConfiguredModules(withKey: "SkipModuleInterfaceVerify",
getAdopterConfigsFromXcodeDefaultToolchain()))

let moduleName = parsedOptions.getLastArgument(.moduleName)?.asSingle
let reportAsError = !knownFailingModules.contains(moduleName ?? "") ||
env["ENABLE_DEFAULT_INTERFACE_VERIFIER"] != nil ||
parsedOptions.hasFlag(positive: .verifyEmittedModuleInterface,
negative: .noVerifyEmittedModuleInterface,
default: false)

if !reportAsError {
diagnosticEngine
.emit(
.remark(
"Verification of module interfaces for '\(moduleName ?? "No module name")' set to warning only by blocklist"))
}

enum InterfaceMode {
case Public, Private, Package
Expand Down Expand Up @@ -601,7 +614,7 @@ extension Driver {
"Merge module job should only have one swiftinterface output")
let job = try verifyModuleInterfaceJob(interfaceInput: mergeInterfaceOutputs[0],
emitModuleJob: emitModuleJob,
optIn: optIn)
reportAsError: reportAsError)
addJob(job)
}
try addVerifyJob(for: .Public)
Expand Down
4 changes: 2 additions & 2 deletions Sources/SwiftDriver/Jobs/VerifyModuleInterfaceJob.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ extension Driver {
return isFrontendArgSupported(.inputFileKey)
}

mutating func verifyModuleInterfaceJob(interfaceInput: TypedVirtualPath, emitModuleJob: Job, optIn: Bool) throws -> Job {
mutating func verifyModuleInterfaceJob(interfaceInput: TypedVirtualPath, emitModuleJob: Job, reportAsError: Bool) throws -> Job {
var commandLine: [Job.ArgTemplate] = swiftCompilerPrefixArgs.map { Job.ArgTemplate.flag($0) }
var inputs: [TypedVirtualPath] = [interfaceInput]
commandLine.appendFlags("-frontend", "-typecheck-module-from-interface")
Expand All @@ -55,7 +55,7 @@ extension Driver {

// TODO: remove this because we'd like module interface errors to fail the build.
if isFrontendArgSupported(.downgradeTypecheckInterfaceError) &&
(!optIn ||
(!reportAsError ||
// package interface is new and should not be a blocker for now
interfaceInput.type == .packageSwiftInterface) {
commandLine.appendFlag(.downgradeTypecheckInterfaceError)
Expand Down
113 changes: 77 additions & 36 deletions Tests/SwiftDriverTests/SwiftDriverTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2855,8 +2855,9 @@ final class SwiftDriverTests: XCTestCase {
"-enable-library-evolution"])

let plannedJobs = try driver1.planBuild()
XCTAssertEqual(plannedJobs.count, 2)
let emitInterfaceJob = plannedJobs[0]
XCTAssertEqual(plannedJobs.count, 3)

let emitInterfaceJob = try plannedJobs.findJob(.emitModule)
XCTAssertTrue(emitInterfaceJob.commandLine.contains(.flag("-emit-module-interface-path")))
XCTAssertTrue(emitInterfaceJob.commandLine.contains(.flag("-emit-private-module-interface-path")))
}
Expand Down Expand Up @@ -5559,23 +5560,31 @@ final class SwiftDriverTests: XCTestCase {
func testVerifyEmittedInterfaceJob() throws {
// Evolution enabled
var envVars = ProcessEnv.vars
envVars["ENABLE_DEFAULT_INTERFACE_VERIFIER"] = "YES"
do {
var driver = try Driver(args: ["swiftc", "foo.swift", "-emit-module", "-module-name",
"foo", "-emit-module-interface",
"-emit-private-module-interface-path", "foo.private.swiftinterface",
"-verify-emitted-module-interface",
"-enable-library-evolution"])
let plannedJobs = try driver.planBuild()
XCTAssertEqual(plannedJobs.count, 3)
XCTAssertEqual(plannedJobs.count, 4)

// Emit-module should emit both module interface files
let emitJob = try plannedJobs.findJob(.emitModule)
let verifyJob = try plannedJobs.findJob(.verifyModuleInterface)
let mergeInterfaceOutputs = emitJob.outputs.filter { $0.type == .swiftInterface }
XCTAssertTrue(mergeInterfaceOutputs.count == 1,
"Merge module job should only have one swiftinterface output")
XCTAssertTrue(verifyJob.inputs.count == 1)
XCTAssertTrue(verifyJob.inputs[0] == mergeInterfaceOutputs[0])
XCTAssertTrue(verifyJob.outputs.isEmpty)
XCTAssertTrue(verifyJob.commandLine.contains(.path(mergeInterfaceOutputs[0].file)))
let publicModuleInterface = emitJob.outputs.filter { $0.type == .swiftInterface }
XCTAssertEqual(publicModuleInterface.count, 1)
let privateModuleInterface = emitJob.outputs.filter { $0.type == .privateSwiftInterface }
XCTAssertEqual(privateModuleInterface.count, 1)

// Each verify job should either check the public or the private module interface, not both.
let verifyJobs = plannedJobs.filter { $0.kind == .verifyModuleInterface }
XCTAssertEqual(verifyJobs.count, 2)
for verifyJob in verifyJobs {
let publicVerify = verifyJob.inputs.contains(try XCTUnwrap(publicModuleInterface.first))
let privateVerify = verifyJob.inputs.contains(try XCTUnwrap(privateModuleInterface.first))
XCTAssertNotEqual(publicVerify, privateVerify)
XCTAssertFalse(verifyJob.commandLine.contains("-downgrade-typecheck-interface-error"))
}
}

// No Evolution
Expand All @@ -5584,6 +5593,7 @@ final class SwiftDriverTests: XCTestCase {
"foo", "-emit-module-interface", "-verify-emitted-module-interface"], env: envVars)
let plannedJobs = try driver.planBuild()
XCTAssertEqual(plannedJobs.count, 2)
XCTAssertFalse(plannedJobs.containsJob(.verifyModuleInterface))
}

// Explicitly disabled
Expand All @@ -5594,6 +5604,7 @@ final class SwiftDriverTests: XCTestCase {
"-no-verify-emitted-module-interface"], env: envVars)
let plannedJobs = try driver.planBuild()
XCTAssertEqual(plannedJobs.count, 2)
XCTAssertFalse(plannedJobs.containsJob(.verifyModuleInterface))
let emitJob = try plannedJobs.findJob(.emitModule)
XCTAssertTrue(emitJob.commandLine.contains("-no-verify-emitted-module-interface"))
}
Expand All @@ -5606,16 +5617,7 @@ final class SwiftDriverTests: XCTestCase {
"-no-emit-module-separately"], env: envVars)
let plannedJobs = try driver.planBuild()
XCTAssertEqual(plannedJobs.count, 2)
}

// Disabled when no "ENABLE_DEFAULT_INTERFACE_VERIFIER" found in the environment
do {
var driver = try Driver(args: ["swiftc", "foo.swift", "-emit-module", "-module-name",
"foo", "-emit-module-interface",
"-enable-library-evolution",
"-experimental-emit-module-separately"])
let plannedJobs = try driver.planBuild()
XCTAssertEqual(plannedJobs.count, 2)
XCTAssertFalse(plannedJobs.containsJob(.verifyModuleInterface))
}

// Emit-module separately
Expand All @@ -5634,6 +5636,7 @@ final class SwiftDriverTests: XCTestCase {
XCTAssertTrue(verifyJob.inputs.count == 1)
XCTAssertTrue(verifyJob.inputs[0] == emitInterfaceOutput[0])
XCTAssertTrue(verifyJob.commandLine.contains(.path(emitInterfaceOutput[0].file)))
XCTAssertFalse(verifyJob.commandLine.contains("-downgrade-typecheck-interface-error"))
XCTAssertFalse(emitJob.commandLine.contains("-no-verify-emitted-module-interface"))
XCTAssertFalse(emitJob.commandLine.contains("-verify-emitted-module-interface"))
}
Expand All @@ -5656,6 +5659,7 @@ final class SwiftDriverTests: XCTestCase {
XCTAssertTrue(verifyJob.inputs.count == 1)
XCTAssertTrue(verifyJob.inputs[0] == emitInterfaceOutput[0])
XCTAssertTrue(verifyJob.commandLine.contains(.path(emitInterfaceOutput[0].file)))
XCTAssertFalse(verifyJob.commandLine.contains("-downgrade-typecheck-interface-error"))
}

// Test the `-no-verify-emitted-module-interface` flag with whole-module
Expand All @@ -5680,21 +5684,63 @@ final class SwiftDriverTests: XCTestCase {
"-library-level", "api"])
let plannedJobs = try driver.planBuild()
XCTAssertEqual(plannedJobs.count, 2)
XCTAssertTrue(plannedJobs.containsJob(.verifyModuleInterface))
let verifyJob = try plannedJobs.findJob(.verifyModuleInterface)
XCTAssertFalse(verifyJob.commandLine.contains("-downgrade-typecheck-interface-error"))
}

// Not enabled by default when the library-level is spi.
// Enabled by default when the library-level is spi.
do {
var driver = try Driver(args: ["swiftc", "foo.swift", "-emit-module", "-module-name",
"foo", "-emit-module-interface",
"-enable-library-evolution",
"-whole-module-optimization",
"-library-level", "spi"])
let plannedJobs = try driver.planBuild()
XCTAssertEqual(plannedJobs.count, 1)
XCTAssertEqual(plannedJobs[0].kind, .compile)
let compileJob = try plannedJobs.findJob(.compile)
XCTAssertFalse(compileJob.commandLine.contains("-no-verify-emitted-module-interface"))
XCTAssertEqual(plannedJobs.count, 2)
let verifyJob = try plannedJobs.findJob(.verifyModuleInterface)
XCTAssertFalse(verifyJob.commandLine.contains("-downgrade-typecheck-interface-error"))
}

// Errors downgraded to a warning when a module is blocklisted.
try assertDriverDiagnostics(args: ["swiftc", "foo.swift", "-emit-module", "-module-name",
"TestBlocklistedModule", "-emit-module-interface",
"-enable-library-evolution",
"-whole-module-optimization",
"-library-level", "api"]) { driver, verify in
let plannedJobs = try driver.planBuild()
XCTAssertEqual(plannedJobs.count, 2)
let verifyJob = try plannedJobs.findJob(.verifyModuleInterface)
if driver.isFrontendArgSupported(.downgradeTypecheckInterfaceError) {
XCTAssertTrue(verifyJob.commandLine.contains("-downgrade-typecheck-interface-error"))
}

verify.expect(.remark("Verification of module interfaces for 'TestBlocklistedModule' set to warning only by blocklist"))
}

// Don't downgrade to error blocklisted modules when the env var is set.
do {
envVars["ENABLE_DEFAULT_INTERFACE_VERIFIER"] = "YES"
var driver = try Driver(args: ["swiftc", "foo.swift", "-emit-module", "-module-name",
"TestBlocklistedModule", "-emit-module-interface",
"-enable-library-evolution",
"-whole-module-optimization"], env: envVars)
let plannedJobs = try driver.planBuild()
XCTAssertEqual(plannedJobs.count, 2)
let verifyJob = try plannedJobs.findJob(.verifyModuleInterface)
XCTAssertFalse(verifyJob.commandLine.contains("-downgrade-typecheck-interface-error"))
}

// Don't downgrade to error blocklisted modules if the verify flag is set.
do {
var driver = try Driver(args: ["swiftc", "foo.swift", "-emit-module", "-module-name",
"TestBlocklistedModule", "-emit-module-interface",
"-enable-library-evolution",
"-whole-module-optimization",
"-verify-emitted-module-interface"])
let plannedJobs = try driver.planBuild()
XCTAssertEqual(plannedJobs.count, 2)
let verifyJob = try plannedJobs.findJob(.verifyModuleInterface)
XCTAssertFalse(verifyJob.commandLine.contains("-downgrade-typecheck-interface-error"))
}

// The flag -check-api-availability-only is not passed down to the verify job.
Expand Down Expand Up @@ -5726,9 +5772,6 @@ final class SwiftDriverTests: XCTestCase {
}

func testVerifyEmittedPackageInterface() throws {
var envVars = ProcessEnv.vars
envVars["ENABLE_DEFAULT_INTERFACE_VERIFIER"] = "YES"

// Evolution enabled
do {
var driver = try Driver(args: ["swiftc", "foo.swift", "-emit-module",
Expand All @@ -5737,11 +5780,9 @@ final class SwiftDriverTests: XCTestCase {
"-emit-module-interface",
"-emit-package-module-interface-path", "foo.package.swiftinterface",
"-verify-emitted-module-interface",
"-enable-library-evolution"], env: envVars)
"-enable-library-evolution"])

let plannedJobs = try driver.planBuild()
let x = plannedJobs.first?.commandLine.joinedUnresolvedArguments ?? ""
print(x)
XCTAssertEqual(plannedJobs.count, 4)
let emitJob = try plannedJobs.findJob(.emitModule)
let verifyJob = try plannedJobs.findJob(.verifyModuleInterface)
Expand All @@ -5764,7 +5805,7 @@ final class SwiftDriverTests: XCTestCase {
"-emit-module-interface",
"-emit-package-module-interface-path", "foo.package.swiftinterface",
"-enable-library-evolution",
"-no-verify-emitted-module-interface"], env: envVars)
"-no-verify-emitted-module-interface"])
let plannedJobs = try driver.planBuild()
XCTAssertEqual(plannedJobs.count, 2)
}
Expand All @@ -5777,7 +5818,7 @@ final class SwiftDriverTests: XCTestCase {
"-emit-module-interface",
"-emit-package-module-interface-path", "foo.package.swiftinterface",
"-enable-library-evolution",
"-experimental-emit-module-separately"], env: envVars)
"-experimental-emit-module-separately"])
let plannedJobs = try driver.planBuild()
XCTAssertEqual(plannedJobs.count, 4)
let emitJob = try plannedJobs.findJob(.emitModule)
Expand Down