Skip to content

[cxx-interop] Import libstdc++ as a Clang module #41953

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
Apr 8, 2022

Conversation

egorzhdan
Copy link
Contributor

This change adds a module map for libstdc++, which allows it to be properly imported into Swift as a module. The module map is installed into usr/lib/swift/linux/{arch} similarly to glibc.modulemap, and is passed to Clang as -fmodule-map-file.

That means it is now possible to import std directly from Swift on Linux, when C++ interop is enabled.

The module map currently declares a single std module without splitting the headers into submodules. This is going to change in the near future.

rdar://87654514

@egorzhdan egorzhdan requested review from hyp and zoecarver March 22, 2022 17:37
@egorzhdan egorzhdan force-pushed the egorzhdan/libstdcxx-module branch 4 times, most recently from 0ed5bec to 1adbc40 Compare March 23, 2022 13:33
@egorzhdan
Copy link
Contributor Author

@swift-ci please smoke test

@egorzhdan egorzhdan added the c++ interop Feature: Interoperability with C++ label Mar 23, 2022
@egorzhdan egorzhdan force-pushed the egorzhdan/libstdcxx-module branch 5 times, most recently from bfc86e0 to 4f5ad83 Compare April 1, 2022 23:32
@egorzhdan egorzhdan force-pushed the egorzhdan/libstdcxx-module branch 3 times, most recently from e8b7d5e to 1172138 Compare April 6, 2022 13:01
@egorzhdan
Copy link
Contributor Author

This patch depends on #42222 being merged first.

Apologies for spamming this PR with git rebases, the test failure wasn't reproducible for me locally.

@egorzhdan egorzhdan force-pushed the egorzhdan/libstdcxx-module branch from 1172138 to 1e7489c Compare April 7, 2022 21:59
@egorzhdan
Copy link
Contributor Author

@swift-ci please smoke test

Copy link
Contributor

@zoecarver zoecarver left a comment

Choose a reason for hiding this comment

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

Thank you Egor. This looks great (as usual).

@egorzhdan
Copy link
Contributor Author

@swift-ci please smoke test Windows

@egorzhdan egorzhdan force-pushed the egorzhdan/libstdcxx-module branch from 1e7489c to 003aea6 Compare April 8, 2022 12:23
@egorzhdan
Copy link
Contributor Author

@swift-ci please smoke test

@egorzhdan
Copy link
Contributor Author

@swift-ci please smoke test Windows

This change adds a module map for libstdc++, which allows it to be properly imported into Swift as a module. The module map is installed into `usr/lib/swift/linux/{arch}` similarly to `glibc.modulemap`, and is passed to Clang as `-fmodule-map-file`.

That means it is now possible to import std directly from Swift on Linux, when C++ interop is enabled.

The module map currently declares a single `std` module without splitting the headers into submodules. This is going to change in the near future.

rdar://87654514
@egorzhdan egorzhdan force-pushed the egorzhdan/libstdcxx-module branch from 003aea6 to a3e9143 Compare April 8, 2022 13:10
@egorzhdan
Copy link
Contributor Author

@swift-ci please smoke test

@egorzhdan
Copy link
Contributor Author

@swift-ci please smoke test Windows

@egorzhdan egorzhdan merged commit a0141bd into main Apr 8, 2022
@egorzhdan egorzhdan deleted the egorzhdan/libstdcxx-module branch April 8, 2022 14:52
@finagolfin
Copy link
Member

@egorzhdan, I cross-compile the trunk toolchain for Android occasionally, and this pull broke cross-compiling the libswift portion of the compiler, because the Android NDK has a C++ modulemap (for some reason, this works fine when natively building the Apr. 21 trunk snapshot on my Android phone). I then disabled the new C++ modulemap portion of this pull for Android, and it cross-compiles and works fine.

Is that what you recommend for such platforms that have a C++ modulemap of their own, or is there something else I should do instead?

@egorzhdan
Copy link
Contributor Author

@buttaface oh this is interesting. Looks like Swift on Android uses libc++, but when cross-compiling, it uses libstdc++? I'm not familiar with Android, sorry about that. In general, I think the best approach is to always use the system-wide default C++ standard library. If the default stdlib is libc++, which already has a modulemap, then the logic in this PR should not be used.
Does compilation break when building natively with your patch? If it doesn't, then we can just remove NOT "${sdk}" STREQUAL "ANDROID" AND, like you did in your patch.

@finagolfin
Copy link
Member

Thanks for the quick response.

Looks like Swift on Android uses libc++, but when cross-compiling, it uses libstdc++?

No, Android always uses libc++, and the Android C++ module map exists in both the cross-compilation and native build environments.

I took a closer look at the build commands and the only difference is that cross-compiling this libswift file specifies the -sdk and -resource-dir flags explicitly, whereas natively bootstrapping doesn't add those two flags. That means the native build never passes your new module map file to the Clang Importer, so there is no conflict.

If that Swift file can be built without the -resource-dir flag, maybe it shouldn't be specified when cross-compiling either?

If the default stdlib is libc++, which already has a modulemap, then the logic in this PR should not be used.
Does compilation break when building natively with your patch? If it doesn't, then we can just remove NOT "${sdk}" STREQUAL "ANDROID" AND, like you did in your patch.

I haven't tried removing it when natively building, but given my above finding that the native build never used your new module map and what you said about libc++ not needing it, removing that line seems like the solution.

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.

3 participants