[cxx-interop] Include Cxx and CxxStdlib modules in no-stdlib builds
#65055
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
These modules are shipped with the toolchain, while the stdlib might be built and shipped separately. rdar://107780733
|
@swift-ci please smoke test |
|
@swift-ci please test |
hyp
approved these changes
Apr 11, 2023
|
@swift-ci please test macOS |
Cxx & CxxStdlib modules are Swift-only, they do not require invoking clang directly. When building with `SWIFT_INCLUDE_TOOLS=NO`, Clang is not available as a CMake target (see `swift_common_standalone_build_config`). rdar://107780733
|
@swift-ci please smoke test |
zoecarver
approved these changes
Apr 11, 2023
|
@swift-ci please smoke test macOS |
|
The test failure on macOS is unrelated and should be fixed by #65069 |
|
@swift-ci please smoke test macOS |
5 similar comments
|
@swift-ci please smoke test macOS |
|
@swift-ci please smoke test macOS |
|
@swift-ci please smoke test macOS |
|
@swift-ci please smoke test macOS |
|
@swift-ci please smoke test macOS |
|
This adversely effects the Windows toolchain builds :-(. Building in stages where the runtime is not built, this results in failures as this categorises Would you be opposed to introducing an |
drodriguez
added a commit
to drodriguez/swift
that referenced
this pull request
Apr 14, 2023
With the changes introduced in swiftlang#65055, the Cxx module will build even when not building the stdlib as long as `SWIFT_BUILD_STDLIB_EXTRA_TOOLCHAIN_CONTENT` was set. With the further changes in swiftlang#65122 the previous flag is not necessary, and only `SWIFT_BUILD_STDLIB_CXX_MODULE` (which defaults to `TRUE`) is necessary. However, `stdlib/public/Cxx` did rely on `StdlibOptions.cmake` to be include before it, or the functions that build the target libraries will find a half configured state. `StdlibOptions.cmake` are included in `stdlib/toolchain`, in `stdlib/` and `StandaloneOverlay.cmake`. Before swiftlang#65122 it was working just because `stdlib/toolchain` was added just before. The second changes adds dependencies on the legacy layouts and the clang headers. The legacy layouts are a dependency only added to the libraries of the stdlib core (IS_STDLIB_CORE flag). Since enabling that flag also enables a bunch other stuff, and I am not sure that Cxx should be classified as "core" anyway, I preferred to add the dependency. The clang headers are another dependency of the swiftCore, which otherwise causes problems to not find the compiler headers when building Cxx. This is necessary for builds that use a previously built compiler to compile the stdlib.
drodriguez
added a commit
to drodriguez/swift
that referenced
this pull request
Apr 14, 2023
With the changes introduced in swiftlang#65055, the Cxx module will build even when not building the stdlib as long as `SWIFT_BUILD_STDLIB_EXTRA_TOOLCHAIN_CONTENT` was set. With the further changes in swiftlang#65122 the previous flag is not necessary, and only `SWIFT_BUILD_STDLIB_CXX_MODULE` (which defaults to `TRUE`) is necessary. However, `stdlib/public/Cxx` did rely on `StdlibOptions.cmake` to be include before it, or the functions that build the target libraries will find a half configured state. `StdlibOptions.cmake` are included in `stdlib/toolchain`, in `stdlib/` and `StandaloneOverlay.cmake`. Before swiftlang#65122 it was working just because `stdlib/toolchain` was added just before. The second changes adds dependencies on the legacy layouts and the clang headers. The legacy layouts are a dependency only added to the libraries of the stdlib core (IS_STDLIB_CORE flag). Since enabling that flag also enables a bunch other stuff, and I am not sure that Cxx should be classified as "core" anyway, I preferred to add the dependency. The clang headers are another dependency of the swiftCore, which otherwise causes problems to not find the compiler headers when building Cxx. This is necessary for builds that use a previously built compiler to compile the stdlib.
drodriguez
added a commit
that referenced
this pull request
Apr 14, 2023
With the changes introduced in #65055, the Cxx module will build even when not building the stdlib as long as `SWIFT_BUILD_STDLIB_EXTRA_TOOLCHAIN_CONTENT` was set. With the further changes in #65122 the previous flag is not necessary, and only `SWIFT_BUILD_STDLIB_CXX_MODULE` (which defaults to `TRUE`) is necessary. However, `stdlib/public/Cxx` did rely on `StdlibOptions.cmake` to be include before it, or the functions that build the target libraries will find a half configured state. `StdlibOptions.cmake` are included in `stdlib/toolchain`, in `stdlib/` and `StandaloneOverlay.cmake`. Before #65122 it was working just because `stdlib/toolchain` was added just before. The second changes adds dependencies on the legacy layouts and the clang headers. The legacy layouts are a dependency only added to the libraries of the stdlib core (IS_STDLIB_CORE flag). Since enabling that flag also enables a bunch other stuff, and I am not sure that Cxx should be classified as "core" anyway, I preferred to add the dependency. The clang headers are another dependency of the swiftCore, which otherwise causes problems to not find the compiler headers when building Cxx. This is necessary for builds that use a previously built compiler to compile the stdlib.
drodriguez
added a commit
that referenced
this pull request
Apr 17, 2023
In #65172 I tried to fix a problem when the Cxx module is enabled, but we are not building the stdlib. The fix work for `swiftCxx`, but failed to to realize that `swiftCxxStdlib` overlay is also built, which needs parts of the stdlib. The original #65055 only built the Cxx module when `EXTRA_TOOLCHAIN_CONTENT` was set, but #65122 changed so it was two independent checks. A more faithful fix will had been to nest the `if` as I am trying to do with this PR. Parts of #65172 are still necessary to build in every case, though: the `swiftCxx` target is dependent on the Clang headers and the legacy layouts. Those two pieces are normally in place because of other targets in upstream builds, but fail to materialize when the stdlib is trying to be built with the host compiler. This one and #65172 will need to be cherry-picked to 5.9 because #65055 and #65122 where cherry-picked, so 5.9 is having the same problems as `main`.
drodriguez
added a commit
to drodriguez/swift
that referenced
this pull request
Apr 17, 2023
…65172) With the changes introduced in swiftlang#65055, the Cxx module will build even when not building the stdlib as long as `SWIFT_BUILD_STDLIB_EXTRA_TOOLCHAIN_CONTENT` was set. With the further changes in swiftlang#65122 the previous flag is not necessary, and only `SWIFT_BUILD_STDLIB_CXX_MODULE` (which defaults to `TRUE`) is necessary. However, `stdlib/public/Cxx` did rely on `StdlibOptions.cmake` to be include before it, or the functions that build the target libraries will find a half configured state. `StdlibOptions.cmake` are included in `stdlib/toolchain`, in `stdlib/` and `StandaloneOverlay.cmake`. Before swiftlang#65122 it was working just because `stdlib/toolchain` was added just before. The second changes adds dependencies on the legacy layouts and the clang headers. The legacy layouts are a dependency only added to the libraries of the stdlib core (IS_STDLIB_CORE flag). Since enabling that flag also enables a bunch other stuff, and I am not sure that Cxx should be classified as "core" anyway, I preferred to add the dependency. The clang headers are another dependency of the swiftCore, which otherwise causes problems to not find the compiler headers when building Cxx. This is necessary for builds that use a previously built compiler to compile the stdlib. (cherry picked from commit 539fc69)
drodriguez
added a commit
to drodriguez/swift
that referenced
this pull request
Apr 17, 2023
In swiftlang#65172 I tried to fix a problem when the Cxx module is enabled, but we are not building the stdlib. The fix work for `swiftCxx`, but failed to to realize that `swiftCxxStdlib` overlay is also built, which needs parts of the stdlib. The original swiftlang#65055 only built the Cxx module when `EXTRA_TOOLCHAIN_CONTENT` was set, but swiftlang#65122 changed so it was two independent checks. A more faithful fix will had been to nest the `if` as I am trying to do with this PR. Parts of swiftlang#65172 are still necessary to build in every case, though: the `swiftCxx` target is dependent on the Clang headers and the legacy layouts. Those two pieces are normally in place because of other targets in upstream builds, but fail to materialize when the stdlib is trying to be built with the host compiler. This one and swiftlang#65172 will need to be cherry-picked to 5.9 because swiftlang#65055 and swiftlang#65122 where cherry-picked, so 5.9 is having the same problems as `main`. (cherry picked from commit ccf57da)
meg-gupta
pushed a commit
to meg-gupta/swift
that referenced
this pull request
Apr 18, 2023
In swiftlang#65172 I tried to fix a problem when the Cxx module is enabled, but we are not building the stdlib. The fix work for `swiftCxx`, but failed to to realize that `swiftCxxStdlib` overlay is also built, which needs parts of the stdlib. The original swiftlang#65055 only built the Cxx module when `EXTRA_TOOLCHAIN_CONTENT` was set, but swiftlang#65122 changed so it was two independent checks. A more faithful fix will had been to nest the `if` as I am trying to do with this PR. Parts of swiftlang#65172 are still necessary to build in every case, though: the `swiftCxx` target is dependent on the Clang headers and the legacy layouts. Those two pieces are normally in place because of other targets in upstream builds, but fail to materialize when the stdlib is trying to be built with the host compiler. This one and swiftlang#65172 will need to be cherry-picked to 5.9 because swiftlang#65055 and swiftlang#65122 where cherry-picked, so 5.9 is having the same problems as `main`.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
These modules are shipped with the toolchain, while the stdlib might be built and shipped separately.
rdar://107840627