Skip to content

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

Merged
merged 26 commits into from
Jul 1, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
158ddd5
Rebase
kabiroberai Apr 28, 2024
385cc0a
Merge branch 'main' into kabir/wasm-plugins
kabiroberai Apr 30, 2024
4dd83b3
cleanup
kabiroberai Apr 30, 2024
0b6a5a6
We don’t need internalError
kabiroberai Apr 30, 2024
7a6b576
Comment
kabiroberai Apr 30, 2024
f5d82ef
Feedback
kabiroberai Apr 30, 2024
9f455cb
More feedback
kabiroberai Apr 30, 2024
18814f1
comment
kabiroberai Apr 30, 2024
a38baa4
comment
kabiroberai Apr 30, 2024
a582c2f
Merge branch 'main' into kabir/wasm-plugins
kabiroberai May 5, 2024
85e0374
Change wasm versioning approach
kabiroberai May 5, 2024
921b7ea
Merge branch 'main' into kabir/wasm-plugins
kabiroberai May 11, 2024
9fab7cc
Merge branch 'main' into kabir/wasm-plugins
kabiroberai May 25, 2024
d3495f2
Create PluginMessageHandler
kabiroberai May 25, 2024
76de84f
Make PluginMessage.Diagnostic.init SPI
kabiroberai May 25, 2024
76f7d8f
Use _expose
kabiroberai Jun 11, 2024
3e2e4d4
Merge branch 'main' into kabir/wasm-plugins
kabiroberai Jun 11, 2024
cf060a4
Feedback
kabiroberai Jun 12, 2024
4cd128c
Fix calling convention
kabiroberai Jun 12, 2024
84f3230
Update .spi.yml
kabiroberai Jun 13, 2024
2242b64
Format
kabiroberai Jun 13, 2024
054e5e3
Merge branch 'main' into kabir/wasm-plugins
kabiroberai Jun 14, 2024
595b0ae
re-format
kabiroberai Jun 18, 2024
c7466de
Merge branch 'main' into kabir/wasm-plugins
kabiroberai Jun 18, 2024
0a054ad
Merge branch 'main' into kabir/wasm-plugins
kabiroberai Jun 29, 2024
0e65034
Fix swiftlang/swift build
kabiroberai Jun 29, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 33 additions & 0 deletions Sources/SwiftCompilerPlugin/CompilerPlugin.swift
Original file line number Diff line number Diff line change
Expand Up @@ -112,12 +112,45 @@ extension CompilerPlugin {
let connection = try StandardIOMessageConnection()
let provider = MacroProviderAdapter(plugin: Self())
let impl = CompilerPluginMessageListener(connection: connection, provider: provider)
#if os(WASI)
// Rather than blocking on read(), let the host tell us when there's data.
readabilityHandler = {
do {
_ = try impl.handleNextMessage()
} catch {
internalError("\(error)")
}
}
#else
Copy link
Member

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(), so handleNextMessage() can be internal.

Copy link
Contributor Author

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 into SwiftCompilerPluginMessageHandling would be leaky, since the swift_wasm_macro_pump export (which invokes readabilityHandler) is an impl detail of the SwiftCompilerPlugin target. Also worth mentioning that CompilerPluginMessageListener is @_spi(PluginMessage) so I don't think we're creating any new API contracts even if handleNextMessage is technically public.

Copy link
Contributor Author

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 of SwiftCompilerPluginMessageHandling so the latter is already strongly coupled to our wasm ABI. Moved readabilityHandler into that module.

do {
try impl.main()
} catch {
// Emit a diagnostic and indicate failure to the plugin host,
// and exit with an error code.
fatalError("Internal Error: \(error)")
}
#endif
}
}

#if os(WASI)

/// A callback invoked by the Wasm Host when new data is available on `stdin`.
///
/// This is safe to access without serialization as Wasm plugins are single-threaded.
nonisolated(unsafe) private var readabilityHandler: () -> Void = {
internalError("""
CompilerPlugin.main wasn't called. Did you annotate your plugin with '@main'?
""")
}

// We can use @_expose(wasm, ...) with compiler >= 6.0
// but it causes build errors with older compilers.
// Instead, we export from wasm_support.c and trampoline
// to this method.
@_cdecl("_swift_wasm_macro_pump")
func wasmPump() {
readabilityHandler()
}

#endif
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Member

@rintaro rintaro Apr 29, 2024

Choose a reason for hiding this comment

The 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 #if os(WASI) branch code and improves the readability.

}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Member

@rintaro rintaro Apr 29, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of #if in the body, could you just wrap the whole decl? The body for wasm would be just

#if os(WASI)
  public init() throws {
    self.init(
      inputFileDescriptor: fileno(_stdin),
      outputFileDescriptor: fileno(_stdout)
    )
  } 
#else
...

This way we can find #endif much easier.

// Duplicate the `stdin` file descriptor, which we will then use for
// receiving messages from the plugin host.
let inputFD = dup(fileno(_stdin))
Expand Down Expand Up @@ -80,6 +85,7 @@ public struct StandardIOMessageConnection: MessageConnection {
_ = _setmode(inputFD, _O_BINARY)
_ = _setmode(outputFD, _O_BINARY)
#endif
#endif

self.init(inputFileDescriptor: inputFD, outputFileDescriptor: outputFD)
}
Expand Down Expand Up @@ -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)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fatalError writes to stdout

As far as I can see it's stderr. Am I missing anything? https://github.com/apple/swift/blob/77f53a5e50f0cf964ae9dcc42f78ec43227a3db2/stdlib/public/runtime/Errors.cpp#L323-L332

But anyway, I think I'm going to use fputs("Internal Error: \(message)\n", _stderr) in all platforms, (we don't want #file etc, and other magic in fatalError). So I will bring internalError() back to CompilerPlugin.swift

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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)
Expand Down
1 change: 0 additions & 1 deletion Sources/_SwiftSyntaxCShims/dummy.c

This file was deleted.

35 changes: 35 additions & 0 deletions Sources/_SwiftSyntaxCShims/wasm_support.c
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");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you teach me what this does exactly?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This adds a custom swift_wasm_macro_abi section to the binary with a little-endian uint32 value. We check the ABI version in the wasm executor in order to future-proof. See:

https://github.com/apple/swift/pull/73031/files#diff-2261ec558c289a4bd568f211772e906caa997f8c26b38e4c6d5d37b7b1554379R47

Will add a brief explanation to the comment.

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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 swift_wasm_macro_pump_v1 and the runtime could check for the presence of this to indicate the v1 ABI. The custom section support feels shaky to me — I wouldn't be surprised if there are tools that don't know how to handle custom wasm sections (or, alternatively, strip them out.)

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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.

Copy link
Contributor

Choose a reason for hiding this comment

The 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