Skip to content

[cxx-interop] Do not install binary .swiftmodule files for the overlays #81408

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
May 12, 2025

Conversation

egorzhdan
Copy link
Contributor

This fixes a deserialization failure in the compiler that occurred while loading the CxxStdlib overlay module:

Cross-reference to module 'Swift'
... Optional
... some
... with type <τ_0_0 where τ_0_0 : ~Copyable, τ_0_0 : ~Escapable> (Optional<τ_0_0>.Type) -> (τ_0_0) -> Optional<τ_0_0>

This was happening because the overlays were built against a different version of the Swift stdlib than is being used. The compiler is able to rebuild the Cxx and CxxStdlib modules from their textual interfaces. Let's use that feature unconditionally in production toolchains to avoid this kind of binary incompatibilities.

rdar://150416863

@egorzhdan egorzhdan requested review from xymus and edymtt May 9, 2025 17:02
@egorzhdan egorzhdan added the c++ interop Feature: Interoperability with C++ label May 9, 2025
@egorzhdan
Copy link
Contributor Author

@swift-ci please smoke test

@egorzhdan egorzhdan requested a review from porglezomp May 9, 2025 17:02
Copy link
Contributor

@j-hui j-hui left a comment

Choose a reason for hiding this comment

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

LGTM

@compnerd
Copy link
Member

compnerd commented May 9, 2025

CC: @etcwilde

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.

Swift interfaces are only valid when library evolution is enabled. The platforms that do not have ABI stability do not have library evolution enabled, and so must use the binary swift module. We will need to continue installing the module on those platforms until they either have library evolution enabled or the textual interfaces are able to describe the layout of structs.

@xymus
Copy link
Contributor

xymus commented May 9, 2025

@etcwilde Is it the new build system that doesn't enable library-evolution on Windows? In the log above I see the swiftinterface being installed to /usr/lib/swift/windows/CxxStdlib.swiftmodule/aarch64-unknown-windows-msvc.swiftinterface and I was under the impression that our libraries had the same configuration on all platforms library-evolution wise even if on some of them we don't rely on ABI stability.

@compnerd
Copy link
Member

compnerd commented May 9, 2025

AFAIK, library evolution is not enabled on Windows (unless someone changed the default underneath us). The old build system does emit the swift interface (and it is being shipped). The new build system implementation ties it explicitly to the library evolution mode, and I don't think that we should be enabling that on Windows currently.

…lays

This fixes a deserialization failure in the compiler that occurred while loading the CxxStdlib overlay module:
```
Cross-reference to module 'Swift'
... Optional
... some
... with type <τ_0_0 where τ_0_0 : ~Copyable, τ_0_0 : ~Escapable> (Optional<τ_0_0>.Type) -> (τ_0_0) -> Optional<τ_0_0>
```

This was happening because the overlays were built against a different version of the Swift stdlib than is being used. The compiler is able to rebuild the Cxx and CxxStdlib modules from their textual interfaces. Let's use that feature unconditionally in production toolchains to avoid this kind of binary incompatibilities.

rdar://150416863
@egorzhdan egorzhdan force-pushed the egorzhdan/do-not-install-swiftmodules branch from 4c954fe to 16b2808 Compare May 10, 2025 21:55
@egorzhdan egorzhdan requested a review from a team as a code owner May 10, 2025 21:55
@egorzhdan
Copy link
Contributor Author

Thanks @etcwilde @compnerd, I changed this to only exclude binary swiftmodules on Darwin.

@egorzhdan egorzhdan requested a review from etcwilde May 10, 2025 21:57
@egorzhdan
Copy link
Contributor Author

@swift-ci please smoke test

@egorzhdan
Copy link
Contributor Author

@swift-ci please build toolchain macOS

@egorzhdan
Copy link
Contributor Author

@swift-ci please test Windows

@egorzhdan egorzhdan merged commit ede6e46 into main May 12, 2025
4 checks passed
@egorzhdan egorzhdan deleted the egorzhdan/do-not-install-swiftmodules branch May 12, 2025 16:29
@egorzhdan
Copy link
Contributor Author

Oops, forgot to disable the auto-merger on this one... Happy to address any feedback post-merge.

hamishknight added a commit to hamishknight/swift that referenced this pull request May 22, 2025
…o-not-install-swiftmodules"

This reverts commit ede6e46.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c++ interop Feature: Interoperability with C++
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants