-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Mixed Swift/C++ targets support #14261
base: master
Are you sure you want to change the base?
Conversation
2ef529e
to
08b7ff3
Compare
|
||
# For linking with a Swift target in a Obj-C/C++ target, include the Swift target's generated header. | ||
if any({lang in target.compilers for lang in ['objc', 'cpp', 'objcpp']}): | ||
for header in self.determine_swift_dep_headers(target): |
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.
This unfortunately breaks if said target links to a part of a custom target (I'm aware this is WIP, but I'm working with a Swift project with a lot of Meson hacks and wanted to give this a go just in case :) ):
Traceback (most recent call last):
File "[...]/meson/mesonbuild/mesonmain.py", line 193, in run
return options.run_func(options)
^^^^^^^^^^^^^^^^^^^^^^^^^
File "[...]/meson/mesonbuild/msetup.py", line 365, in run
app.generate()
File "[...]/meson/mesonbuild/msetup.py", line 188, in generate
return self._generate(env, capture, vslite_ctx)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "[...]/meson/mesonbuild/msetup.py", line 253, in _generate
captured_compile_args = intr.backend.generate(capture, vslite_ctx)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "[...]/meson/mesonbuild/backend/ninjabackend.py", line 663, in generate
self.generate_target(t)
File "[...]/meson/mesonbuild/backend/ninjabackend.py", line 1021, in generate_target
for header in self.determine_swift_dep_headers(target):
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "[...]/meson/mesonbuild/backend/ninjabackend.py", line 2205, in determine_swift_dep_headers
if self.is_swift_target(l):
^^^^^^^^^^^^^^^^^^^^^^^
File "[...]/meson/mesonbuild/backend/backends.py", line 516, in is_swift_target
for s in target.sources:
^^^^^^^^^^^^^^
AttributeError: 'CustomTargetIndex' object has no attribute 'sources'
Something like this seems to help:
diff --git a/mesonbuild/backend/ninjabackend.py b/mesonbuild/backend/ninjabackend.py
index 2dda038ea..dc60a60ad 100644
--- a/mesonbuild/backend/ninjabackend.py
+++ b/mesonbuild/backend/ninjabackend.py
@@ -2202,7 +2202,7 @@ class NinjaBackend(backends.Backend):
def determine_swift_dep_headers(self, target):
result = []
for l in target.link_targets:
- if self.is_swift_target(l):
+ if isinstance(l, build.BuildTarget) and self.is_swift_target(l):
result.append(self.swift_generated_header_file_name(l))
return result
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.
Ah, thanks! That check should probably go into is_swift_target tbh (unless that already only accepts a BuildTarget per the parameter types).
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.
unless that already only accepts a BuildTarget per the parameter types
that's the case exactly:)
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 added it in is_swift_target after all and relaxed the type bounds since that seems less error-prone :)
Also, looks like mypy doesn't complain about calling is_swift_target with an invalid type even when are type annotations. Smh
mesonbuild/backend/ninjabackend.py
Outdated
if not isinstance(target, build.Executable): | ||
elem = NinjaBuildElement(self.all_outputs, out_module_name, rulename, abssrc) | ||
elem.add_dep(in_module_files + rel_generated) | ||
elem.add_item('ARGS', compile_args + abs_generated + module_includes + swiftc.get_mod_gen_args()) |
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.
This also needs header_imports
fwiw:) (otherwise if a header is imported .o builds fine, but .swiftmodule fails)
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 wanted to add this a couple times now because I keep seeing the mismatch between the args of the two targets -- but as far as I understand, using bridging headers in a library seems to be not supported: https://forums.swift.org/t/spm-library-with-bridging-header/4945
Not sure if that was the original intention of leaving this away or if it was forgotten (I didn't write these two arg lists)
It seems to be allowed for frameworks (https://developer.apple.com/documentation/swift/importing-objective-c-into-swift), so I assume it's related to the install interface. Packaging a Swift library would involve installing both the Swift module files but additionally the C header, and the information about which libraries to link, for client Swift libraries to be able to import it. I assume there's no defined interface for that.
I'd rather add a failure check for this. You should instead use a module map to create a C module that you can import from Swift. Here an example for the matio library:
CMatio/meson.build:
matio_dep = dependency('matio')
CMatio_dep = declare_dependency(
include_directories: ['.'],
dependencies: [matio_dep],
)
CMatio/matio.h:
#include_next <matio.h>
CMatio/module.modulemap:
module CMatio [extern_c] {
header "matio.h"
}
Then use CMatio_dep in your Swift library target.
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 see, you're right, I never noticed it was already missing before your changes. I was curious because my build, which relies on a header being included like that, works perfectly fine without this PR.
Turns out previously the .swiftmodule just wasn't being used, so it was never actually built I assume, hence no errors.
I wasn't aware this wasn't meant to be supported at all, in which case I agree blocking it - like Xcode does - sounds best.
mesonbuild/backend/ninjabackend.py
Outdated
def swift_generated_header_file_name(self, target): | ||
return os.path.join(self.get_target_private_dir(target), | ||
self.target_swift_modulename(target) + '-Swift.h') |
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.
with -working-directory
being set to our private dir, this and the module path above seem to need an absolute path:
def swift_generated_header_file_name(self, target): | |
return os.path.join(self.get_target_private_dir(target), | |
self.target_swift_modulename(target) + '-Swift.h') | |
def swift_generated_header_file_name(self, target): | |
return os.path.join(self.get_target_private_dir_abs(target), | |
self.target_swift_modulename(target) + '-Swift.h') |
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.
Does it? Works fine for me. These are used in the Ninja file where they're supposed to be relative to the top build dir, and in case of the header generation where it is passed to swiftc that isn't run in the private target dir but instead the top build dir.
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.
You're right of course, I had leftover local changes which affected this, so please ignore. Sorry for the noise.
As of Swift 5.9, C++/Objective-C++ code can be mixed with Swift, and C++ APIs can be imported to Swift. However, this must be explicitly enabled, as it is disabled by default. Xcode 15 introduces a new setting for this, so only set it on Xcode 15 and above.
Skip if Swift 5.9 or above is not detected.
As is the case with most other methods, it must be overriden in another compiler for it to have any use. Only the Swift compiler uses this method at this time.
Merge commit until PRs this builds upon are merged.
ed2bb13
to
27c599d
Compare
SourceKit-LSP likes to generate PCH files inside the working directory. Make sure it knows the absolute path, otherwise it places it outside the build directory. Also use an absolute directory for the working directory, since the PCH files otherwise end up flooding /tmp, never to be deleted.
Not all compilers (swiftc in this case) concatenate linker arguments with ','. This removes that assumption and instead always passes the arguments in a list where applicable. Some Apple ld arguments, notably -install_name, are also not exposed directly on the compiler options interface, hence add the linker prefix here.
This allows targets containing both C and Swift sources, and allows linking a C++ target against a Swift static library by using the Swift compiler as a linker.
Allows calling Swift code from C++ by adding the Swift library's private dir to the C++ target's include directories.
27c599d
to
9c93153
Compare
9c93153
to
29d5031
Compare
(Requires/follow-up to #13317. Actual diff of this PR without #13317.)
Closes #13203.
(Supersedes) Closes #14241.
This is a different approach to #14241 which does not require the incredibly ugly manual linking against Swift core libraries.
Makes Swift target generation use the main code path. This allows mixing Swift and C++ sources inside of a single target. (All Swift sources are still compiled with one swiftc invocation since per-file compilation is not supported.)
This also adds support for shared library Swift targets.
Also creates a Ninja target for a C++/Obj-C exported header, which those languages can use to call Swift code. This header is automatically added as a dependency to targets that can use it.
Caveat: using this to call Swift code from C++ requires manually setting CXX to the Swift-bundled clang++ (on non-macOS), since it requires special support in the compiler which the normal clang doesn't come with. Meson would automatically have to switch to using the Swift-bundled clang when a project uses both Swift and C/C++/Obj-C. Not sure if that is desirable or even doable. Could maybe throw an error instead when the mismatch is detected.
To do: