Skip to content

[embedded] Do not emit .swiftinterface files for fragile modules (embedded stdlib, Cxx.swiftmodule) #68786

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

Jump to bottom
Merged
merged 2 commits into from
Oct 3, 2023
Merged

[embedded] Do not emit .swiftinterface files for fragile modules (embedded stdlib, Cxx.swiftmodule) #68786

merged 2 commits into from
Oct 3, 2023

Conversation

kubamracek
Copy link
Contributor

Currently, we get warnings like this:

[1/13] Generating /Users/kuba/swift-github-main/build/Ninja-RelWithDebInfoAssert/swift-macosx-arm64/./lib/swift/macosx/Cxx.swiftmodule/arm64-apple-macos.swiftmodule
<unknown>:0: warning: module interfaces are only supported with -enable-library-evolution
<unknown>:0: warning: module interfaces are only supported with -enable-library-evolution
[13/13] Generating /Users/kuba/swift-github-main/build/Ninja-RelWithDebInfoAssert/swift-macosx-arm64/lib/swift/embedded/Swift.swiftmodule/arm64-apple-macos.swiftmodule
<unknown>:0: warning: module interfaces are only supported with -enable-library-evolution
<unknown>:0: warning: module interfaces are only supported with -enable-library-evolution

Let's fix it (by not generating the .swiftinterface for these fragile modules), for both the embedded stdlib, and the Cxx module.

@kubamracek
Copy link
Contributor Author

@swift-ci please test

kubamracek
kubamracek commented Sep 27, 2023
View reviewed changes
stdlib/cmake/modules/SwiftSource.cmake
@@ -646,7 +646,7 @@ function(_compile_swift_files
set(sibopt_file "${module_base}.O.sib")
set(sibgen_file "${module_base}.sibgen")

if(SWIFT_ENABLE_MODULE_INTERFACES)
if(SWIFT_ENABLE_MODULE_INTERFACES AND NOT SWIFTFILE_IS_FRAGILE)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note that Cxx is already passing IS_FRAGILE.

@kubamracek
Copy link
Contributor Author

@swift-ci please test Linux platform

@kubamracek kubamracek added the embedded Embedded Swift label Sep 27, 2023
@kubamracek
Copy link
Contributor Author

@swift-ci please test

etcwilde
etcwilde approved these changes Sep 28, 2023
View reviewed changes
Copy link
Member

@etcwilde etcwilde left a comment

Choose a reason for hiding this comment

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

I makes me a bit sad that SWIFT_ENABLE_MODULE_INTERFACES isn't configurable on a per-library basis so that we have to drill holes like this, but that seems to be par for the course.

@kubamracek
Copy link
Contributor Author

I makes me a bit sad that SWIFT_ENABLE_MODULE_INTERFACES isn't configurable on a per-library basis so that we have to drill holes like this, but that seems to be par for the course.

Isn't that exactly what "IS_FRAGILE" is? Maybe we could deprecate/remove SWIFT_ENABLE_MODULE_INTERFACES in favor of IS_FRAGILE?

@kubamracek kubamracek merged commit 61be599 into swiftlang:main Oct 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@etcwilde etcwilde etcwilde approved these changes

@DougGregor DougGregor DougGregor approved these changes

@zoecarver zoecarver zoecarver approved these changes

@eeckstein eeckstein eeckstein approved these changes

@hyp hyp Awaiting requested review from hyp

Assignees
No one assigned
Labels
embedded Embedded Swift
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@kubamracek @etcwilde @DougGregor @zoecarver @eeckstein