-
Notifications
You must be signed in to change notification settings - Fork 10.4k
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
[cxx-interop] Allow compiling with libc++ on Linux #75589
Conversation
e525f88
to
bd5a032
Compare
"module %0 was built with " | ||
"%select{platform default C++ stdlib|libc++}1, " | ||
"but current compilation uses " | ||
"%select{platform default C++ stdlib|libc++}2", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure that this is great - the platform default c++ runtime could be libc++. I'd rather that we use a string parameter and let the frontend specify the runtime name.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1. I think this only works if the options are platform default versus libc++, but the latter is the platform default in many cases, and on Linux in general I think it'd make sense to allow libstdc++ on distros where the default happens to be libc++.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I changed this to explicitly list the supported C++ stdlibs by name.
e2c4a8b
to
ca0add4
Compare
@swift-ci please smoke test |
a26196b
to
45ee808
Compare
28b8155
to
a9b5bfd
Compare
@swift-ci please smoke test |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should be using more than one bit in the swiftmodule
file, and that we should identify which C++ library the module was built with (rather than "default" versus "overridden", which is what we have now).
"module %0 was built with " | ||
"%select{platform default C++ stdlib|libc++}1, " | ||
"but current compilation uses " | ||
"%select{platform default C++ stdlib|libc++}2", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1. I think this only works if the options are platform default versus libc++, but the latter is the platform default in many cases, and on Linux in general I think it'd make sense to allow libstdc++ on distros where the default happens to be libc++.
include/swift/Basic/CXXStdlibKind.h
Outdated
enum class CXXStdlibKind : uint8_t { | ||
/// Use the default C++ stdlib for a particular platform: libc++ for Darwin, | ||
/// libstdc++ for most Linux distros. | ||
PlatformDefault = 0, | ||
|
||
/// Use libc++ instead of the default C++ stdlib on a platform where libc++ | ||
/// is not the default, e.g. Ubuntu. | ||
OverrideLibcxx = 1, | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
enum class CXXStdlibKind : uint8_t { | |
/// Use the default C++ stdlib for a particular platform: libc++ for Darwin, | |
/// libstdc++ for most Linux distros. | |
PlatformDefault = 0, | |
/// Use libc++ instead of the default C++ stdlib on a platform where libc++ | |
/// is not the default, e.g. Ubuntu. | |
OverrideLibcxx = 1, | |
}; | |
enum class CXXStdlibKind : uint8_t { | |
/// Use the default C++ stdlib for a particular platform: libc++ for Darwin, | |
/// libstdc++ for most Linux distros. This should never be stored in a | |
/// .swiftmodule file; we should always use one of the other constants | |
/// to indicate exactly what was used at build time. | |
PlatformDefault = 0, | |
Libcxx = 1, | |
Libstdcxx = 2, | |
Msvcprt = 3, | |
// Other C++ libraries may be supported in future | |
}; |
6378523
to
ff67d93
Compare
@swift-ci please smoke test |
I tested this on windows, with some additional changes for custom libc++ and split out MSVC c++ support using sealing like before, and this kind of approach works great. This looks good to me. |
ff67d93
to
c53b46a
Compare
Thanks @hyp for trying it out! |
@swift-ci please smoke test |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for all the iteration. This really seems significantly better than both the first revision and even some of the original code.
c53b46a
to
c93a4cb
Compare
c93a4cb
to
dedf76c
Compare
This makes sure that Swift respects `-Xcc -stdlib=libc++` flags. Clang already has existing logic to discover the system-wide libc++ installation on Linux. We rely on that logic here. Importing a Swift module that was built with a different C++ stdlib is not supported and emits an error. The Cxx module can be imported when compiling with any C++ stdlib. The synthesized conformances, e.g. to CxxRandomAccessCollection also work. However, CxxStdlib currently cannot be imported when compiling with libc++, since on Linux it refers to symbols from libstdc++ which have different mangled names in libc++. rdar://118357548 / #69825
dedf76c
to
059f0f9
Compare
@swift-ci please smoke test |
@swift-ci please build toolchain UBI9 |
This flag was added back in 2020, but it didn't function properly, since a lot of other code in the compiler assumed the platform-default C++ stdlib until recently (#75589). The recommended way to use a non-default C++ stdlib in Swift now is to pass `-Xcc -stdlib=xyz` argument to the compiler. This change removes the `-experimental-cxx-stdlib` flag.
This makes sure that Swift respects
-Xcc -stdlib=libc++
flags.Clang already has existing logic to discover the system-wide libc++ installation on Linux. We rely on that logic here.
Importing a Swift module that was built with a different C++ stdlib is not supported and emits an error.
The Cxx module can be imported when compiling with any C++ stdlib. The synthesized conformances, e.g. to CxxRandomAccessCollection also work. However, CxxStdlib currently cannot be imported when compiling with libc++, since on Linux it refers to symbols from libstdc++ which have different mangled names in libc++.
rdar://118357548 / #69825