-
Notifications
You must be signed in to change notification settings - Fork 446
Add support for WebAssembly Macros #2623
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
Changes from 1 commit
158ddd5
385cc0a
4dd83b3
0b6a5a6
7a6b576
f5d82ef
9f455cb
18814f1
a38baa4
a582c2f
85e0374
921b7ea
9fab7cc
d3495f2
76de84f
76f7d8f
3e2e4d4
cf060a4
4cd128c
84f3230
2242b64
054e5e3
595b0ae
c7466de
0a054ad
0e65034
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -86,10 +86,20 @@ public class CompilerPluginMessageListener<Connection: MessageConnection, Provid | |
/// Throws an error when it failed to send/receive the message, or failed | ||
/// to serialize/deserialize the message. | ||
public func main() throws { | ||
while let message = try connection.waitForNextMessage(HostToPluginMessage.self) { | ||
let result = handler.handleMessage(message) | ||
try connection.sendMessage(result) | ||
while try handleNextMessage() {} | ||
} | ||
|
||
/// Receives and handles a single message from the plugin host. | ||
/// | ||
/// - Returns: `true` if there was a message to read, `false` | ||
/// if the end-of-file was reached. | ||
public func handleNextMessage() throws -> Bool { | ||
guard let message = try connection.waitForNextMessage(HostToPluginMessage.self) else { | ||
return false | ||
} | ||
let result = handler.handleMessage(message) | ||
try connection.sendMessage(result) | ||
return true | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I opened a PR #2631 My intention is, in this PR, you'd make it like: public func main() {
#if os(WASI)
readabilityHandler = { _ = self.handleNextMessage() }
#else
while handleNextMessage() {}
#endif
}
func handleNextMessage() -> Bool {
do {
guard let message = try connection.waitForNextMessage(HostToPluginMessage.self) else {
return false
}
let result = handler.handleMessage(message)
try connection.sendMessage(result)
return true
} catch {
fputs("Internal Error: \(message)\n", _stderr)
exit(1)
}
} I think this should minimize |
||
} | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -48,6 +48,11 @@ public struct StandardIOMessageConnection: MessageConnection { | |
/// - Duplicate the original `stdin` and `stdout` for use as messaging | ||
/// pipes, and are not directly used by the plugin logic | ||
public init() throws { | ||
#if os(WASI) | ||
// Wasm doesn't support dup{,2} so we use the original file descriptors. | ||
let inputFD = fileno(_stdin) | ||
let outputFD = fileno(_stdout) | ||
#else | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Instead of #if os(WASI)
public init() throws {
self.init(
inputFileDescriptor: fileno(_stdin),
outputFileDescriptor: fileno(_stdout)
)
}
#else
... This way we can find |
||
// Duplicate the `stdin` file descriptor, which we will then use for | ||
// receiving messages from the plugin host. | ||
let inputFD = dup(fileno(_stdin)) | ||
|
@@ -80,6 +85,7 @@ public struct StandardIOMessageConnection: MessageConnection { | |
_ = _setmode(inputFD, _O_BINARY) | ||
_ = _setmode(outputFD, _O_BINARY) | ||
#endif | ||
#endif | ||
|
||
self.init(inputFileDescriptor: inputFD, outputFileDescriptor: outputFD) | ||
} | ||
|
@@ -146,6 +152,15 @@ public struct StandardIOMessageConnection: MessageConnection { | |
} | ||
} | ||
|
||
#if os(WASI) | ||
// fatalError writes to stdout, which is the message | ||
// output stream under WASI | ||
public func internalError(_ message: String) -> Never { | ||
fputs("Internal Error: \(message)\n", _stderr) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
As far as I can see it's But anyway, I think I'm going to use There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Indeed, looks like you're right. Not sure why I thought it wrote to stdout — might've forgotten to attach stderr to the WASI bridge at some point. |
||
exit(1) | ||
} | ||
#endif | ||
|
||
private enum IOError: Error, CustomStringConvertible { | ||
case readReachedEndOfInput | ||
case systemError(function: String, errno: CInt) | ||
|
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,35 @@ | ||
//===----------------------------------------------------------------------===// | ||
// | ||
// This source file is part of the Swift open source project | ||
// | ||
// Copyright (c) 2023 Apple Inc. and the Swift project authors | ||
// Licensed under Apache License v2.0 with Runtime Library Exception | ||
// | ||
// See http://swift.org/LICENSE.txt for license information | ||
// See http://swift.org/CONTRIBUTORS.txt for the list of Swift project authors | ||
// | ||
//===----------------------------------------------------------------------===// | ||
|
||
#if defined(__wasm32__) | ||
|
||
// uint32_t | ||
#define SWIFT_WASM_MACRO_ABI 1 | ||
|
||
#define _STR(X) #X | ||
#define STR(X) _STR(X) | ||
|
||
// LLVM has special-cased handling to map .custom_section.foo to | ||
// Wasm Custom Section "foo". this must be a metadata section rather | ||
// than a data section so we can't use __attribute__((section)) for it. | ||
// See: https://reviews.llvm.org/D43097 | ||
__asm__("\t.section .custom_section.swift_wasm_macro_abi,\"\",@\n\t.4byte " STR(SWIFT_WASM_MACRO_ABI) "\n"); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could you teach me what this does exactly? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This adds a custom Will add a brief explanation to the comment. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. hmm actually @kateinoigakukun what do you think about using the export name to indicate the ABI instead? E.g. we could export the pump function with the name There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Pushed this as a tentative change, happy to revert if the Custom Sections approach is preferable but I do like this atm because it's a bit more lightweight. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For posterity: discussed this with @kateinoigakukun here; we aligned on using the exported function name rather than a custom section, since some Wasm tooling (ahem wasm-strip) can be too trigger-happy about stripping custom sections. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think Swift tries to be conservative with custom sections because it's a terribly non-portable solution. Although portability isn't at issue here, it might maybe make sense to use an exported function name. That'd be the typical solution here if we were talking about a C function on another platform, I think. |
||
|
||
// defined in CompilerPlugin.swift | ||
void _swift_wasm_macro_pump(void); | ||
|
||
__attribute__((export_name("swift_wasm_macro_pump"))) | ||
void swift_wasm_macro_pump(void) { | ||
_swift_wasm_macro_pump(); | ||
} | ||
|
||
#endif |
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 feel this logic should be sinked to
impl.main()
, sohandleNextMessage()
can beinternal
.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.
hmm I'm worried that moving
readabilityHandler
intoSwiftCompilerPluginMessageHandling
would be leaky, since theswift_wasm_macro_pump
export (which invokesreadabilityHandler
) is an impl detail of theSwiftCompilerPlugin
target. Also worth mentioning thatCompilerPluginMessageListener
is@_spi(PluginMessage)
so I don't think we're creating any new API contracts even ifhandleNextMessage
is technically public.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.
Actually I just realized that
_SwiftSyntaxCShims
is a dependency ofSwiftCompilerPluginMessageHandling
so the latter is already strongly coupled to our wasm ABI. MovedreadabilityHandler
into that module.