-
Notifications
You must be signed in to change notification settings - Fork 10.3k
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
[Macros] Add support for wasm macros #73031
base: main
Are you sure you want to change the base?
Conversation
<!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN" "http:https://www.apple.com/DTDs/PropertyList-1.0.dtd"> | ||
<plist version="1.0"> | ||
<dict> | ||
<key>com.apple.security.cs.allow-jit</key> |
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.
Necessary in order for JSC to expose globalThis.WebAssembly
— I don't think there's any security risk here because /System/Library/Frameworks/JavaScriptCore.framework/Versions/Current/Helpers/jsc
already exposes significantly more surface area. Plus we're able to keep the restrictive sandbox from Sandbox::apply
intact when running this via swiftc.
paging @DougGregor @MaxDesiatov as stakeholders, would love to get your thoughts on this |
also wondering how I should add end-to-end tests for this: I would include some wasm blob fixtures but I don't want to be mistaken for the Second Coming of Jia Tan |
Another question: how much should we generalize this right now? As in, should we have a specialized wasm executor embedded into |
I like @kateinoigakukun 's idea of seeing whether we can slim down the WASI build to include those things that already work across the platforms and are needed by macros. |
To include Windows support of the WASI implementation.
@swift-ci smoke test |
The support for `pipe` on Windows was added by apple/swift-system@cfd3f9d but it is not included in the latest swift-system release 1.3.1. This commit adds a temporary inlined implementation of `pipe` API for Windows until we update to the next release of swift-system.
@swift-ci smoke test |
`%c-flags` and `%exe-linker-flags` are derived from `CMAKE_C_FLAGS` and `CMAKE_EXE_LINKER_FLAGS` respectively, which are not configured for Wasm targets. And also those variables are not set in Windows build. So, this patch removes the usage of those variables.
@swift-ci smoke test |
@kabiroberai Hmm, something wrong is happening on Windows 🤔 |
thanks for all the Windows-related fixes @kateinoigakukun, looks like we're near the finish line! I wonder if the current failures are related to dos/unix path formats (backslash vs forward slash)? ended up being busier than expected last week and might not have much time until EOW, but I'll try to configure a local testing setup on Windows by Saturday so that I can poke around. Wouldn't be surprised if it's a 1-2 line fix. |
c06a27a
to
3b13c6b
Compare
@swift-ci smoke test |
The ':' character can cause conflicts in Windows paths (e.g., C:\path\to\file). The '#' character is less likely to appear in file paths and we already use it for the second path separator in `-load-plugin` option
3b13c6b
to
0f39ec8
Compare
@swift-ci smoke test |
1 similar comment
@swift-ci smoke test |
Okay, finally CI checks are all green! |
@swift-ci please Build Toolchain Linux Platform |
@swift-ci please Build Toolchain macOS Platform |
@swift-ci build toolchain windows |
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 has ended up... almost straightforward in its integration. Very nice!
file(TO_CMAKE_PATH "${SWIFT_PATH_TO_SWIFT_SYNTAX_SOURCE}" swift_syntax_path) | ||
FetchContent_Declare(SwiftSystem SOURCE_DIR "${swift_syntax_path}/../swift-system") | ||
|
||
set(WASMKIT_BUILD_CLI OFF) | ||
FetchContent_Declare(WasmKit SOURCE_DIR "${swift_syntax_path}/../wasmkit") | ||
FetchContent_MakeAvailable(WasmKit) |
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 suppose the alternative to this is to thread new variables for these paths throughout build-script
and build.ps1
, which is... a lot of boilerplate.
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.
Indeed, do you think it's worth doing that right now? Or would it be preferable to merge as-is?
This PR introduces support for WebAssembly-based macros via
-load-plugin-executable Foo.wasm#Foo
.Wasm macros are blazing fast (no SwiftSyntax build required for consumers!) and secure (fully sandboxed on all OSes)
The approach I've taken here is to update
swift-plugin-server
to add special handling for Wasm macros. These macros are executed using WasmKit, but in the future we can add support for other (faster, JIT) runtimes like JavaScriptCore.Note also that these wasm macros currently have an ad-hoc ABI as I wanted to avoid a hard dependency on WASI. It's thus theoretically possible to build macros without a dependency on WASI, though I've had trouble doing so because Swift currently builds for wasm as WASI xor Embedded Mode — we'd need a middle ground with support for String etc. The current ABI is based on the one I built for Wacro. We'll have to iron out the execution model though: my current mental model is that plugins are request-response style (i.e. every HostToPluginMessage must be followed by exactly one PluginToHostMessage) and so the macro effectively exports a
char *macro_parse(char *request_json)
but I don't know if this is a guaranteed contract.There's currently a few things that still need to be done:
Followups:
.binaryMacro
target similar to.binaryTarget
, though this will make the edit-compile-run cycle non-trivial as it splits building the macro into a separate phase. (*)Marking this as a draft until the above items are done but I wanted to make this PR so I could get some input as well as to brainstorm (particularly about the items marked as (*))
Depends on swiftlang/swift-syntax#2623
There's also a corresponding PR in swift-driver, which is blocked by this one: swiftlang/swift-driver#1582