Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Wire up support for module wrapping on non-Darwin platforms #145

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

owenv
Copy link
Collaborator

@owenv owenv commented Feb 12, 2025

No description provided.

@owenv
Copy link
Collaborator Author

owenv commented Feb 12, 2025

@swift-ci test

@owenv
Copy link
Collaborator Author

owenv commented Feb 12, 2025

fyi @cmcgee1024 since I think we discussed this at some point

switch self {
case .macho:
return false
default:
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: can we prefer to list the cases explicitly? I like to stay away from default since it forces re-evaluation of use sites when new cases are added. Not that adding new formats is particularly likely, but I could at least see a new one for wasm being added in the future.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is really an is-not-macho check, I don't expect we will ever move away from module wrap for any future object formats

@owenv
Copy link
Collaborator Author

owenv commented Feb 12, 2025

@swift-ci test

@@ -2127,6 +2127,11 @@ public final class SwiftCompilerSpec : CompilerSpec, SpecIdentifierType, SwiftDi
return (inputs, outputs)
}()

if cbc.scope.evaluate(BuiltinMacros.PLATFORM_REQUIRES_SWIFT_MODULEWRAP) && cbc.scope.evaluate(BuiltinMacros.GCC_GENERATE_DEBUGGING_SYMBOLS) {
let moduleWrapOutput = Path(moduleFilePath.withoutSuffix + ".o")
Copy link
Member

Choose a reason for hiding this comment

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

Windows really should use .obj instead of .o.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I filed #149 to track this because it needs to be done in a few other places at the same time to avoid a regression

Copy link
Collaborator

Choose a reason for hiding this comment

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

@compnerd I think it's more like "MSVC ABI should use .obj and gcc/mingw ABI should use .o", right?

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants