Skip to content
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
13 changes: 9 additions & 4 deletions Sources/SwiftDriver/Driver/Driver.swift
Original file line number Diff line number Diff line change
Expand Up @@ -353,6 +353,9 @@ public struct Driver {
/// Original ObjC Header passed from command-line
let originalObjCHeaderFile: VirtualPath.Handle?

/// Whether to import the bridging header as internal (vs public).
let importBridgingHeaderAsInternal: Bool

/// Enable bridging header chaining.
let bridgingHeaderChaining: Bool

Expand Down Expand Up @@ -1165,10 +1168,12 @@ public struct Driver {
}
self.producePCHJob = maybeNeedPCH

if let objcHeaderPathArg = parsedOptions.getLastArgument(.importObjcHeader) {
self.originalObjCHeaderFile = try? VirtualPath.intern(path: objcHeaderPathArg.asSingle)
if let importBridgingHeaderOption = parsedOptions.last(for: .importBridgingHeader, .internalImportBridgingHeader) {
self.originalObjCHeaderFile = try? VirtualPath.intern(path: importBridgingHeaderOption.argument.asSingle)
self.importBridgingHeaderAsInternal = importBridgingHeaderOption.option == .internalImportBridgingHeader
} else {
self.originalObjCHeaderFile = nil
self.importBridgingHeaderAsInternal = false
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this is not quite correct. I think it maybe need to be internal if library evolution is enabled.

The special case that falls here is that the current module doesn't have a bridging header but there is dependency swift binary module that has bridging header, thus scanner can create a chained bridging for current module. In this case, there isn't a bridging header import flag that indicates if current module does internal import or not.

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't think I understand how to construct this special case. In the place where the driver used to always generate -import-pch for the chained header, it now checks whether the bridging header is internal and will pass the new -internal-import-pch instead (which treats the PCH as internal-only). Is that not sufficient?

Copy link
Contributor

Choose a reason for hiding this comment

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

A swift testcase for this is: https://github.com/swiftlang/swift/blob/main/test/ScanDependencies/bridging-header-autochaining.swift#L51-L55

Requirement is:

  • Module has no bridging header
  • Binary module dependency has bridging header
  • Enable auto chaining.

In this case, driver invocation will not have -*import-bridging-header, swift-driver needs to figure out which -import-pch flag flavor to pass the PCH flag to swift-frontend.

Copy link
Member Author

Choose a reason for hiding this comment

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

When the binary module dependency has an internal bridging header (the new thing), it's not recorded that it has a bridging header at all, so there is no chaining. No?

Copy link
Contributor

Choose a reason for hiding this comment

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

In my example, internal bridging header is not using internal bridging header, but the legacy bridging header.

}

if parsedOptions.hasFlag(positive: .autoBridgingHeaderChaining,
Expand Down Expand Up @@ -2059,8 +2064,8 @@ extension Driver {

// Put bridging header as first input if we have it
let allInputs: [TypedVirtualPath]
if let objcHeader = originalObjCHeaderFile {
allInputs = [TypedVirtualPath(file: objcHeader, type: .objcHeader)] + inputFiles
if let bridgingHeader = originalObjCHeaderFile {
allInputs = [TypedVirtualPath(file: bridgingHeader, type: .objcHeader)] + inputFiles
} else {
allInputs = inputFiles
}
Expand Down
16 changes: 9 additions & 7 deletions Sources/SwiftDriver/Jobs/FrontendJobHelpers.swift
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,6 @@ extension Driver {
try commandLine.appendLast(.nostdimport, from: &parsedOptions)
try commandLine.appendLast(.nostdlibimport, from: &parsedOptions)
try commandLine.appendLast(.parseStdlib, from: &parsedOptions)
try commandLine.appendLast(.solverMemoryThreshold, from: &parsedOptions)
try commandLine.appendLast(.valueRecursionThreshold, from: &parsedOptions)
try commandLine.appendLast(.warnSwift3ObjcInference, from: &parsedOptions)
try commandLine.appendLast(.remarkLoadingModule, from: &parsedOptions)
Expand Down Expand Up @@ -475,14 +474,17 @@ extension Driver {
try computeCanonicalObjCHeader(explicitModulePlanner: explicitModulePlanner)
let objcHeaderFile = (kind == .scanDependencies) ? originalObjCHeaderFile : importedObjCHeader
if let importedObjCHeader = objcHeaderFile, bridgingHeaderHandling != .ignored {
let importBridgingHeaderFlag: Option = importBridgingHeaderAsInternal
? .internalImportBridgingHeader
: .importObjcHeader
if bridgingHeaderHandling == .precompiled, let pch = precompiledObjCHeader {
// For explicit module build, we directly pass the compiled pch to
// swift-frontend, rather than rely on swift-frontend to locate
// the pch in the pchOutputDir and can start an implicit build in case
// of a lookup failure.
if parsedOptions.contains(.pchOutputDir) &&
!parsedOptions.contains(.driverExplicitModuleBuild) {
commandLine.appendFlag(.importObjcHeader)
commandLine.appendFlag(importBridgingHeaderFlag)
try addPathArgument(VirtualPath.lookup(importedObjCHeader), to:&commandLine, remap: jobNeedPathRemap)
try commandLine.appendLast(.pchOutputDir, from: &parsedOptions)
if !compilerMode.isSingleCompilation {
Expand All @@ -491,21 +493,21 @@ extension Driver {
} else {
// If header chaining is enabled, pass objc header through `-import-objc-header` and
// PCH file through `-import-pch`. Otherwise, pass either the PCH or header through
// `-import-objc-header` option.
// `-import-objc-header` option (or its internal variant).
if isFrontendArgSupported(.importPch), importedObjCHeader != originalObjCHeaderFile {
commandLine.appendFlag(.importPch)
commandLine.appendFlag(importBridgingHeaderAsInternal ? .internalImportPch : .importPch)
try addPathArgument(VirtualPath.lookup(pch), to:&commandLine, remap: jobNeedPathRemap)
if let originalHeader = originalObjCHeaderFile {
commandLine.appendFlag(.importObjcHeader)
commandLine.appendFlag(importBridgingHeaderFlag)
try addPathArgument(VirtualPath.lookup(originalHeader), to:&commandLine, remap: jobNeedPathRemap)
}
} else {
commandLine.appendFlag(.importObjcHeader)
commandLine.appendFlag(importBridgingHeaderFlag)
try addPathArgument(VirtualPath.lookup(pch), to:&commandLine, remap: jobNeedPathRemap)
}
}
} else {
commandLine.appendFlag(.importObjcHeader)
commandLine.appendFlag(importBridgingHeaderFlag)
try addPathArgument(VirtualPath.lookup(importedObjCHeader), to:&commandLine, remap: jobNeedPathRemap)
}
}
Expand Down
5 changes: 4 additions & 1 deletion Sources/SwiftDriver/Jobs/ReplJob.swift
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,10 @@ extension Driver {
try addCommonFrontendOptions(commandLine: &commandLine, inputs: &inputs, kind: .repl)
try addRuntimeLibraryFlags(commandLine: &commandLine)

try commandLine.appendLast(.importObjcHeader, from: &parsedOptions)
try commandLine.appendLast(
.importObjcHeader, .internalImportBridgingHeader,
from: &parsedOptions
)
toolchain.addLinkedLibArgs(
to: &commandLine,
parsedOptions: &parsedOptions
Expand Down
Loading