[Macros] Infer nonisolated conformances in macro-expanded code. #3069
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,147 @@ | ||||||
| //===----------------------------------------------------------------------===// | ||||||
| // | ||||||
| // This source file is part of the Swift.org open source project | ||||||
| // | ||||||
| // Copyright (c) 2014 - 2023 Apple Inc. and the Swift project authors | ||||||
| // Licensed under Apache License v2.0 with Runtime Library Exception | ||||||
| // | ||||||
| // See https://swift.org/LICENSE.txt for license information | ||||||
| // See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors | ||||||
| // | ||||||
| //===----------------------------------------------------------------------===// | ||||||
| // | ||||||
| // This file implements inference of "nonisolated" on the conformances that | ||||||
| // occur within macro-expanded code. It's meant to provide source compatibility | ||||||
|
|
||||||
| // | ||||||
|
|
||||||
| import SwiftSyntax | ||||||
|
|
||||||
| extension SyntaxProtocol { | ||||||
| /// Given some Swift syntax that may contain type definitions and extensions, | ||||||
| /// add "nonisolated" to protocol conformances when there are nonisolated | ||||||
| /// members. For example, given: | ||||||
| /// | ||||||
| /// extension X: P { | ||||||
| /// nonisolated func f() { } | ||||||
| /// } | ||||||
| /// | ||||||
| /// this operation will produce: | ||||||
| /// | ||||||
| /// extension X: nonisolated P { | ||||||
| /// 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 |
||||||
| let rewriter = NonisolatedConformanceRewriter() | ||||||
| return rewriter.rewrite(self) | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| fileprivate class NonisolatedConformanceRewriter: SyntaxRewriter { | ||||||
| override func visitAny(_ node: Syntax) -> Syntax? { | ||||||
| // We only care about decl groups (non-protocol nominal types + extensions) | ||||||
| // that have nonisolated members and an inheritance clause. | ||||||
| guard let declGroup = node.asProtocol(DeclGroupSyntax.self), | ||||||
| !declGroup.is(ProtocolDeclSyntax.self), | ||||||
| declGroup.containsNonisolatedMembers, | ||||||
| let inheritanceClause = declGroup.inheritanceClause | ||||||
| else { | ||||||
| return nil | ||||||
| } | ||||||
|
|
||||||
| var skipFirst = | ||||||
| declGroup.is(ClassDeclSyntax.self) | ||||||
| || (declGroup.is(EnumDeclSyntax.self) && inheritanceClause.inheritedTypes.first?.looksLikeEnumRawType ?? false) | ||||||
| let inheritedTypes = inheritanceClause.inheritedTypes.map { inheritedType in | ||||||
| // If there's already a 'nonisolated' or some kind of custom attribute | ||||||
| if inheritedType.type.hasNonisolatedOrCustomAttribute { | ||||||
| return inheritedType | ||||||
| } | ||||||
|
|
||||||
| if skipFirst { | ||||||
| skipFirst = false | ||||||
| return inheritedType | ||||||
| } | ||||||
|
Comment on lines
+61
to
+64
There was a problem hiding this comment. Should this check be before the |
||||||
|
|
||||||
| return inheritedType.with(\.type, "nonisolated \(inheritedType.type)") | ||||||
| } | ||||||
|
There was a problem hiding this comment. If this doesn't change anything, can we return 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. |
||||||
|
|
||||||
| return Syntax( | ||||||
| fromProtocol: declGroup.with( | ||||||
|
There was a problem hiding this comment.
Suggested change
so that it doesn't rewrite the whole tree. Same for other |
||||||
| \.inheritanceClause, | ||||||
| inheritanceClause.with( | ||||||
| \.inheritedTypes, | ||||||
| InheritedTypeListSyntax(inheritedTypes) | ||||||
| ) | ||||||
| ) | ||||||
| ) | ||||||
|
There was a problem hiding this comment. Returning something from 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)
) |
||||||
| } | ||||||
| } | ||||||
|
|
||||||
| extension TypeSyntax { | ||||||
| /// Determine whether the given type has a 'nonisolated' specifier or a | ||||||
| /// custom attribute (that could be a global actor). | ||||||
| fileprivate var hasNonisolatedOrCustomAttribute: Bool { | ||||||
| var type = self | ||||||
| while let attributedType = type.as(AttributedTypeSyntax.self) { | ||||||
| // nonisolated | ||||||
| let hasNonisolated = attributedType.specifiers.contains { specifier in | ||||||
| if case .nonisolatedTypeSpecifier = specifier { | ||||||
| return true | ||||||
| } | ||||||
|
|
||||||
| return false | ||||||
| } | ||||||
| if hasNonisolated { | ||||||
| return true | ||||||
| } | ||||||
|
|
||||||
| // Any attribute will do. | ||||||
| if !attributedType.attributes.isEmpty { | ||||||
| return true | ||||||
| } | ||||||
|
|
||||||
| type = attributedType.baseType | ||||||
| } | ||||||
|
|
||||||
| return false | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| 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 | ||||||
|
|
||||||
| fileprivate var looksLikeEnumRawType: Bool { | ||||||
| // TODO: We could probably use a utility to syntactically recognize types | ||||||
| // from the | ||||||
|
Comment on lines
+116
to
+117
|
||||||
| var text = type.trimmed.description[...] | ||||||
| if text.starts(with: "Swift.") { | ||||||
| text = text.dropFirst(6) | ||||||
| } | ||||||
|
Comment on lines
+119
to
+121
There was a problem hiding this comment. It would be nice to instead look for a |
||||||
|
|
||||||
| 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 |
||||||
| "String": | ||||||
| return true | ||||||
|
|
||||||
| default: return false | ||||||
| } | ||||||
| } | ||||||
| } | ||||||
| extension DeclModifierListSyntax { | ||||||
| /// Whether the modifier list contains "nonisolated". | ||||||
| fileprivate var hasNonisolated: Bool { | ||||||
| contains { $0.name.tokenKind == .keyword(.nonisolated) } | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| extension DeclGroupSyntax { | ||||||
| /// Determine whether any of members is marked "nonisolated. | ||||||
| fileprivate var containsNonisolatedMembers: Bool { | ||||||
| memberBlock.members.lazy.map(\.decl).contains { | ||||||
| $0.asProtocol(WithModifiersSyntax.self)?.modifiers.hasNonisolated ?? false | ||||||
| } | ||||||
| } | ||||||
| } | ||||||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 doesn't seem to be used.
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.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, but I consider
PluginFeatureis 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, inPluginHost.swiftin the swift repo:Also, you only added this feature to
LibraryPluginProvider, which doesn't cover single-module executable plugins.