[Macros] Infer nonisolated conformances in macro-expanded code. #3069
Conversation
…ost-expansion adjustments This is preparation for additional adjustments.
…cro-expanded code
The combination of isolated conformance inference with
global-actor-isolated types has introduced a source compatibility issue
with existing macros that produce conformances that are meant to be
nonisolated. Introduce a syntactic transform that identifies the
presence of nonisolated members and makes the conformances introduced
by the enclosing group "nonisolated". For example, an extension macro
that expands to:
extension C: P {
nonisolated func f() { }
}
will have the macro expansion adjusted to:
extension C: nonisolated P {
nonisolated func f() { }
}
…of nonisolated conformances When this property returns true, infer nonisolated conformances for the produced macro expansion. Do this by default, so that existing macros will work with isolated conformance inference. Macros can opt out by returning false, in which case we will skip this transformation.
The compiler will need to be able to distinguish between macro implementations that know about infer-nonisolated-conformances (such that the plugin itself can apply the transform) vs. those that predate this transform (in which case the compiler will need to do it).
|
@swift-ci please test |
| @@ -30,6 +30,9 @@ import SwiftSyntaxMacros | |||
| @_spi(PluginMessage) | |||
| public enum PluginFeature: String { | |||
| case loadPluginLibrary = "load-plugin-library" | |||
|
|
|||
| /// Whether the plugin knows how to infer nonisolated conformances. | |||
| case inferNonisolatedConformances = "infer-nonisolated-conformances" | |||
There was a problem hiding this comment.
It ends up getting passed back to the compiler, which checks for its presence. See the compiler PR I just put up.
There was a problem hiding this comment.
Okay, but I consider PluginFeature is more like variation of plugin executables. E.g. single-module-plugin vs. plugin-server.
For something like this, I'd use PluginMessage.PROTOCOL_VERSION_NUMBER. Like, in PluginHost.swift in the swift repo:
extension CompilerPlugin.Capability {
hasIinferNonisolatedConformances: Bool { protcolVersion >= 8 }
}Also, you only added this feature to LibraryPluginProvider, which doesn't cover single-module executable plugins.
|
@swift-ci please test |
| InheritedTypeListSyntax(inheritedTypes) | ||
| ) | ||
| ) | ||
| ) |
There was a problem hiding this comment.
Returning something from SyntaxRewriter.visitAny(_:) prevents it from waling into children. If we what to rewrite things like
extension MyStruct: P {
nonisolated func f() { }
struct Inner: Q {
nonisolated func b() {}
}
}to
extension MyStruct: nonisolated P {
nonisolated func f() { }
struct Inner: nonisolated Q {
nonisolated func b() {}
}
}You'd need
.with(
\.memberBlock
self.rewrite(declGroup.memberBlock, detach: true)
)| } | ||
|
|
||
| return Syntax( | ||
| fromProtocol: declGroup.with( |
There was a problem hiding this comment.
| fromProtocol: declGroup.with( | |
| fromProtocol: declGroup.detached.with( |
so that it doesn't rewrite the whole tree. Same for other .with(..)
| } | ||
|
|
||
| return inheritedType.with(\.type, "nonisolated \(inheritedType.type)") | ||
| } |
There was a problem hiding this comment.
If this doesn't change anything, can we return nil here? To save the rewrite cost.
There was a problem hiding this comment.
Returning a non-nil result here means that we’ll traverse the children as well. Returning the node itself stops traversal. We do check the ID of the returned node and if it’s the same as the original, we don’t perform any rewriting.
| /// nonisolated func f() { } | ||
| /// } | ||
| @_spi(Testing) @_spi(Compiler) | ||
| public func inferNonisolatedConformances() -> Syntax { |
There was a problem hiding this comment.
This could be a computed property. Also, because it is non-mutating, it should be called inferringNonisolatedConformances
| //===----------------------------------------------------------------------===// | ||
| // | ||
| // This file implements inference of "nonisolated" on the conformances that | ||
| // occur within macro-expanded code. It's meant to provide source compatibility |
| } | ||
|
|
||
| return inheritedType.with(\.type, "nonisolated \(inheritedType.type)") | ||
| } |
There was a problem hiding this comment.
Returning a non-nil result here means that we’ll traverse the children as well. Returning the node itself stops traversal. We do check the ID of the returned node and if it’s the same as the original, we don’t perform any rewriting.
| // TODO: We could probably use a utility to syntactically recognize types | ||
| // from the |
| if text.starts(with: "Swift.") { | ||
| text = text.dropFirst(6) | ||
| } |
There was a problem hiding this comment.
It would be nice to instead look for a MemberTypeSyntax. The current implementation wouldn’t detect Swift . Int, which is valid.
| extension InheritedTypeSyntax { | ||
| /// Determine whether this inherited type "looks like" a raw type, e.g., | ||
| /// if it's one of the integer types or String. This can only be an heuristic, | ||
| /// because it does not |
|
|
||
| switch text { | ||
| case "Int", "Int8", "Int16", "Int32", "Int64", | ||
| "UInt", "UInt8", "UInt16", "UInt32", "UInt64", |
There was a problem hiding this comment.
Do we also need to include Int128 and UInt128?
| if skipFirst { | ||
| skipFirst = false | ||
| return inheritedType | ||
| } |
There was a problem hiding this comment.
Should this check be before the hasNonisolatedOrCustomAttribute check? Not that it makes much sense but we skip the second type using the skipFirst check if you have something like enum Foo: nonisolated Int, MyProtocol.
| /// | ||
| /// This operation defaults to `true`. Macros can implement it to return | ||
| /// `false` to prevent this adjustment to the macro-expanded code. | ||
| static var inferNonisolatedConformances: Bool { get } |
There was a problem hiding this comment.
Could you create an RFC for the API addition?
The combination of isolated conformance inference with global-actor-isolated types has introduced a source compatibility issue with existing macros that produce conformances that are meant to be nonisolated. Introduce a syntactic transform that identifies the presence of nonisolated members and makes the conformances introduced by the enclosing group "nonisolated". For example, an extension macro that expands to:
will have the macro expansion adjusted to:
This is controlled by the new macro implementation customization point
Macro.inferNonisolatedConformances, which defaults totruebut can be turned off for macro implementations that want to handle this inference themselves.Macro part of rdar://150419628