-
Notifications
You must be signed in to change notification settings - Fork 440
Macro protocols lack async
from expansion
function requirements despite proposals stating they should have it
#2803
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
Comments
Synced to Apple’s issue tracker as rdar://133763644 |
The swift-evolution proposals for macros were inconsistent about whether Given that swift-syntax has always had synchronous expansion functions, I’m updating the proposals to also state synchronous expansions functions. swiftlang/swift-evolution#2546 |
@ahoppen that would be the easiest way to solve this issue, but wouldn't it be better for |
What kind of async operations would you like to do in the macro? |
Use |
What kind of concurrent operations would you like to perform that need to be asynchronous? I’m trying to understand the use cases in which an asynchronous expansion function would help. |
The ability to perform expensive operations in parallel could significantly speed up the expansion of computationally heavy macros. Being able to wait for completion and results of async functions is also an important feature in itself, especially for cases where a function you need to use has no sync alternatives. Are there downsides to supporting async expansion? The change wouldn't break existing macros. |
I don’t think there any fundamental obstacles to providing asynchronous My thinking is that async expansion functions wouldn’t offer huge benefits though because macros can’t do IO or network requests, which are traditionally a good use case for asynchronous functions and operate on sufficiently small inputs that the computation should be sufficiently quick that it wouldn’t benefit from multi-threading. If you have concrete counter-examples I’m happy to hear them and prove my assumptions wrong. |
Whether a macro benefits from multi-threading depends on what it does, not necessarily on the size of the inputs it receives. Example of a macro that could hugely benefit from multi-threading if @freestanding(expression)
public macro lookupTable(input: [Input]) -> [Input: Output] = #externalMacro(module: "LookupTableMacros", type: "LookupTableMacro")
public struct LookupTableMacro: ExpressionMacro {
public static func expansion(of node: some FreestandingMacroExpansionSyntax, in context: some MacroExpansionContext) -> ExprSyntax {
let inputs: [Input] = getInputs(node)
var lookupTable = [String](repeating: String(), count: inputs.count)
for i in 0..<inputs.count {
let input = inputs[i]
let output = expensiveOperation(input)
lookupTable[i] = "\(input): \(output)"
}
return "[\n\(raw: lookupTable.joined(separator: ",\n"))\n]"
}
}
let lookupTable = #lookupTable(input: [
<#Input 1#>,
<#Input 2#>,
<#Input 3#>,
...
])
/* Expansion:
[
<#Input 1#>: <#Output 1#>,
<#Input 2#>: <#Output 2#>,
<#Input 3#>: <#Output 3#>,
...
]*/ The code above depicts a macro that could be used to generate a compile-time lookup table for the specified inputs. If Let's say that:
Then the expansion could theoretically take:
Is this example sufficient @ahoppen ? Also, regarding the engineering work required for providing asynchronous |
I understand how parallel processing helps speed up things. I am just wondering what kind of operation I think the most tricky part about this change is considering API compatibility. If |
Thanks for bringing up So yes, to be able to call However, as The amount of code this change would break should be relatively small, considering that macros have been around for less than a year at the time of writing. There have been similarly and more drastically API-incompatible changes made to swift-syntax in the past. The benefits of async As the swift-syntax package uses semantic versioning, minor API breakage is acceptable between major releases. Unlike some of the previous API-incompatible changes like renaming a property for clearer meaning, changing |
I don’t think it’s a small change. Macros tend to have tests and all of them need to be manually updated, so it’s certainly has a significant impact. It’s certainly not the end of the world but it needs to be factored in. |
@ahoppen I might have found a solution that avoids breaking anything: protocol Macro {
func expansion() // must be implemented
func expansion() async // can be implemented if required
}
// default async expansion is sync expansion
extension Macro {
func expansion() async {
let syncExpansion = { expansion() }
syncExpansion()
}
}
// calls sync expansion
func assertMacroExpansion(macro: some Macro) {
macro.expansion()
}
// calls async expansion
func assertMacroExpansion(macro: some Macro) async {
await macro.expansion()
}
struct SimpleMacro : Macro {
func expansion() {}
}
struct AsyncOnlyMacro : Macro {
func expansion() {
fatalError("must use async assertMacroExpansion")
}
func expansion() async {}
}
// calling macro always calls async expansion
#simpleMacro() // calls sync expansion through async expansion (default)
#asyncOnlyMacro() // calls async expansion |
The above solution would require some code duplication, but shouldn't break anything. If wanted, sync expansion could be deprecated in a later version and eventually be removed to eliminate the duplicate code. |
I think this could be an interesting approach for migration. I’m happy to review it. Would you like to flesh this out in a PR? |
Gladly :D I don't have much experience contributing to open-source projects, but I can try my best to make a PR. Not sure how long it will take, could be days or weeks, but I can start working on it tomorrow. |
Thank you @roopekv. Let me know if you have any questions. https://github.com/swiftlang/swift-syntax/blob/main/CONTRIBUTING.md should be a good place to start. |
Uh oh!
There was an error while loading. Please reload this page.
Issue
Swift's concurrency features cannot be fully utilized in macros without being able to implement
expansion
function requirements of macro protocols asasync
functions.Cause
While the Swift Evolution proposals for expression and attached macros state that the
expansion
function requirements of macro protocols should beasync
, the macro protocol declarations in swift-syntax are missing theasync
keyword from theirexpansion
function requirements (see comparison below), making it impossible to implement them asasync
functions.Solution
Changing the declarations in swift-syntax to
async
shouldn't break existing macros, asasync
function requirements can be implemented as non-async functions.Example
I acknowledge that additional changes would have to be made within the swift-syntax package (changes at the expansion function call sites and possible further structural changes to facilitate asynchronous code), but depending on how they are implemented, the changes don't have to affect the user-facing parts of the package in a source breaking way.
Differences between Swift Evolution proposals and swift-syntax
ExpressionMacro
Proposal
swift-syntax
swift-syntax/Sources/SwiftSyntaxMacros/MacroProtocols/ExpressionMacro.swift
Lines 20 to 27 in 0b324f8
PeerMacro
Proposal
swift-syntax
swift-syntax/Sources/SwiftSyntaxMacros/MacroProtocols/PeerMacro.swift
Lines 17 to 29 in 0b324f8
MemberMacro
Proposal
swift-syntax
swift-syntax/Sources/SwiftSyntaxMacros/MacroProtocols/MemberMacro.swift
Lines 20 to 71 in 0b324f8
AccessorMacro
Proposal
swift-syntax
swift-syntax/Sources/SwiftSyntaxMacros/MacroProtocols/AccessorMacro.swift
Lines 18 to 27 in 0b324f8
MemberAttributeMacro
Proposal
swift-syntax
swift-syntax/Sources/SwiftSyntaxMacros/MacroProtocols/MemberAttributeMacro.swift
Lines 21 to 38 in 0b324f8
Additional quotes from Expression Macros proposal
The text was updated successfully, but these errors were encountered: