From 1dee7576c2923c92496c1937029db9547eea726a Mon Sep 17 00:00:00 2001 From: Konrad `ktoso` Malawski Date: Wed, 17 Apr 2024 21:31:58 +0900 Subject: [PATCH 1/6] [Concurrency] Reimplement @TaskLocal as a macro resolves rdar://120914014 --- lib/Macros/Sources/SwiftMacros/CMakeLists.txt | 1 + .../Sources/SwiftMacros/TaskLocalMacro.swift | 182 ++++++++++++++++++ stdlib/public/Concurrency/TaskLocal.swift | 16 +- .../Runtime/async_task_locals_basic.swift | 2 +- .../async_task_locals_copy_to_async.swift | 2 +- .../async_task_locals_copy_to_sync.swift | 2 +- .../Runtime/async_task_locals_groups.swift | 2 +- ...ent_illegal_use_discarding_taskgroup.swift | 2 +- ...locals_prevent_illegal_use_taskgroup.swift | 2 +- .../Runtime/async_task_locals_spawn_let.swift | 2 +- .../async_task_locals_synchronous_bind.swift | 2 +- .../Runtime/async_task_locals_wrapper.swift | 2 +- .../async_task_locals_basic_warnings.swift | 6 +- ..._locals_basic_warnings_bug_isolation.swift | 6 +- test/Concurrency/task_local.swift | 23 ++- 15 files changed, 225 insertions(+), 27 deletions(-) create mode 100644 lib/Macros/Sources/SwiftMacros/TaskLocalMacro.swift diff --git a/lib/Macros/Sources/SwiftMacros/CMakeLists.txt b/lib/Macros/Sources/SwiftMacros/CMakeLists.txt index c4339a4d9d20f..f33a5465c2ee0 100644 --- a/lib/Macros/Sources/SwiftMacros/CMakeLists.txt +++ b/lib/Macros/Sources/SwiftMacros/CMakeLists.txt @@ -14,6 +14,7 @@ add_swift_macro_library(SwiftMacros OptionSetMacro.swift DebugDescriptionMacro.swift DistributedResolvableMacro.swift + TaskLocalMacro.swift SWIFT_DEPENDENCIES SwiftDiagnostics SwiftOperators diff --git a/lib/Macros/Sources/SwiftMacros/TaskLocalMacro.swift b/lib/Macros/Sources/SwiftMacros/TaskLocalMacro.swift new file mode 100644 index 0000000000000..fb7447b2ca3a4 --- /dev/null +++ b/lib/Macros/Sources/SwiftMacros/TaskLocalMacro.swift @@ -0,0 +1,182 @@ +//===----------------------------------------------------------------------===// +// +// This source file is part of the Swift.org open source project +// +// Copyright (c) 2022-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 +// +//===----------------------------------------------------------------------===// + +import SwiftSyntax +import SwiftSyntaxMacros +import SwiftDiagnostics + +// TODO: docs +public enum TaskLocalMacro {} + +extension TaskLocalMacro: PeerMacro { + public static func expansion( + of node: AttributeSyntax, + providingPeersOf declaration: some DeclSyntaxProtocol, + in context: some MacroExpansionContext + ) throws -> [DeclSyntax] { + guard let varDecl = try requireVar(declaration, diagnose: false) else { + return [] + } + guard try requireModifier(varDecl, .static, diagnose: false) else { + return [] + } + + guard let firstBinding = varDecl.bindings.first else { + return [] // TODO: make error + } + + guard let name = firstBinding.pattern.as(IdentifierPatternSyntax.self)?.identifier else { + return [] // TODO: make error + } + + let type = firstBinding.typeAnnotation?.type + let explicitType: String = + if let type { + ": TaskLocal<\(type.trimmed)>" + } else { + "" + } + + let initialValue: Any + if let initializerValue = firstBinding.initializer?.value { + initialValue = initializerValue + } else if let type, type.isOptional { + initialValue = "nil" + } else { + throw DiagnosticsError( + syntax: declaration, + message: "'@TaskLocal' property must have default value, or be optional", id: .mustBeVar) + } + + return [ + """ + static let $\(name)\(raw: explicitType) = TaskLocal(wrappedValue: \(raw: initialValue)) + """ + ] + } +} + +extension TaskLocalMacro: AccessorMacro { + public static func expansion( + of node: AttributeSyntax, + providingAccessorsOf declaration: some DeclSyntaxProtocol, + in context: some MacroExpansionContext + ) throws -> [AccessorDeclSyntax] { + // We very specifically have to fail and diagnose in the accessor macro, + // rather than in the peer macro, since returning [] from the accessor + // macro adds another series of errors about it missing to emit a decl. + guard let varDecl = try requireVar(declaration) else { + return [] + } + try requireModifier(varDecl, .static) + + guard let firstBinding = varDecl.bindings.first else { + return [] // TODO: make error + } + + guard let name = firstBinding.pattern.as(IdentifierPatternSyntax.self)?.identifier else { + return [] // TODO: make error + } + + return ["get { $\(name).get() }"] + } +} + +@discardableResult +private func requireVar(_ decl: some DeclSyntaxProtocol, + diagnose: Bool = true) throws -> VariableDeclSyntax? { + if let varDecl = decl.as(VariableDeclSyntax.self) { + return varDecl + } + if diagnose { + throw DiagnosticsError( + syntax: decl, + message: "'@TaskLocal' can only be applied to properties", id: .mustBeVar) + } + + return nil +} + +@discardableResult +private func requireModifier(_ decl: VariableDeclSyntax, + _ keyword: Keyword, + diagnose: Bool = true) throws -> Bool { + let isStatic = decl.modifiers.contains { modifier in + modifier.name.text == "\(keyword)" + } + + if !isStatic { + if diagnose { + throw DiagnosticsError( + syntax: decl, + message: "'@TaskLocal' can only be applied to 'static' property", id: .mustBeStatic) + } else { + return false + } + } + + return true +} + +extension TypeSyntax { + // This isn't great since we can't handle type aliases since the macro + // has no type information, but at least for the common case for Optional + // and T? we can detect the optional. + fileprivate var isOptional: Bool { + let strRepr = "\(self)" + return strRepr.last == "?" || + strRepr.starts(with: "Optional<") || + strRepr.starts(with: "Swift.Optional<") + } +} + +struct TaskLocalMacroDiagnostic: DiagnosticMessage { + enum ID: String { + case mustBeStatic = "must be static" + case mustBeVar = "must be var" + } + + var message: String + var diagnosticID: MessageID + var severity: DiagnosticSeverity + + init(message: String, diagnosticID: SwiftDiagnostics.MessageID, severity: SwiftDiagnostics.DiagnosticSeverity = .error) { + self.message = message + self.diagnosticID = diagnosticID + self.severity = severity + } + + init(message: String, domain: String, id: ID, severity: SwiftDiagnostics.DiagnosticSeverity = .error) { + self.message = message + self.diagnosticID = MessageID(domain: domain, id: id.rawValue) + self.severity = severity + } +} + +extension DiagnosticsError { + init( + syntax: S, + message: String, + domain: String = "Swift", + id: TaskLocalMacroDiagnostic.ID, + severity: SwiftDiagnostics.DiagnosticSeverity = .error) { + self.init(diagnostics: [ + Diagnostic( + node: Syntax(syntax), + message: TaskLocalMacroDiagnostic( + message: message, + domain: domain, + id: id, + severity: severity)) + ]) + } +} diff --git a/stdlib/public/Concurrency/TaskLocal.swift b/stdlib/public/Concurrency/TaskLocal.swift index 7dcb689faacf9..19bfb96197582 100644 --- a/stdlib/public/Concurrency/TaskLocal.swift +++ b/stdlib/public/Concurrency/TaskLocal.swift @@ -13,7 +13,20 @@ import Swift @_implementationOnly import _SwiftConcurrencyShims -/// Property wrapper that defines a task-local value key. + +// Macros are disabled when Swift is built without swift-syntax. +#if $Macros && hasAttribute(attached) + +// TODO: docs +@available(SwiftStdlib 5.1, *) +@attached(accessor) +@attached(peer, names: prefixed(`$`)) +public macro TaskLocal() = + #externalMacro(module: "SwiftMacros", type: "TaskLocalMacro") + +#endif + +/// Wrapper that defines a task-local value key. /// /// A task-local value is a value that can be bound and read in the context of a /// `Task`. It is implicitly carried with the task, and is accessible by any @@ -137,7 +150,6 @@ import Swift /// read() // traceID: nil /// } /// } -@propertyWrapper @available(SwiftStdlib 5.1, *) public final class TaskLocal: Sendable, CustomStringConvertible { let defaultValue: Value diff --git a/test/Concurrency/Runtime/async_task_locals_basic.swift b/test/Concurrency/Runtime/async_task_locals_basic.swift index 8aae82873b7bd..6ce6cada44a1e 100644 --- a/test/Concurrency/Runtime/async_task_locals_basic.swift +++ b/test/Concurrency/Runtime/async_task_locals_basic.swift @@ -1,4 +1,4 @@ -// RUN: %target-run-simple-swift( -Xfrontend -disable-availability-checking -parse-as-library %import-libdispatch) | %FileCheck %s +// RUN: %target-run-simple-swift( -plugin-path %swift-plugin-dir -Xfrontend -disable-availability-checking -parse-as-library %import-libdispatch) | %FileCheck %s // REQUIRES: executable_test // REQUIRES: concurrency diff --git a/test/Concurrency/Runtime/async_task_locals_copy_to_async.swift b/test/Concurrency/Runtime/async_task_locals_copy_to_async.swift index 6e584a7eb7f28..d7e0186f4ae8b 100644 --- a/test/Concurrency/Runtime/async_task_locals_copy_to_async.swift +++ b/test/Concurrency/Runtime/async_task_locals_copy_to_async.swift @@ -1,5 +1,5 @@ // REQUIRES: rdar80824152 -// RUN: %target-run-simple-swift( -Xfrontend -disable-availability-checking -parse-as-library %import-libdispatch) | %FileCheck %s +// RUN: %target-run-simple-swift( -plugin-path %swift-plugin-dir -Xfrontend -disable-availability-checking -parse-as-library %import-libdispatch) | %FileCheck %s // REQUIRES: executable_test // REQUIRES: concurrency diff --git a/test/Concurrency/Runtime/async_task_locals_copy_to_sync.swift b/test/Concurrency/Runtime/async_task_locals_copy_to_sync.swift index 99f81b654839d..c0f7bda2805d7 100644 --- a/test/Concurrency/Runtime/async_task_locals_copy_to_sync.swift +++ b/test/Concurrency/Runtime/async_task_locals_copy_to_sync.swift @@ -1,4 +1,4 @@ -// RUN: %target-run-simple-swift( -Xfrontend -disable-availability-checking -parse-as-library %import-libdispatch) | %FileCheck %s +// RUN: %target-run-simple-swift( -plugin-path %swift-plugin-dir -Xfrontend -disable-availability-checking -parse-as-library %import-libdispatch) | %FileCheck %s // REQUIRES: executable_test // REQUIRES: concurrency diff --git a/test/Concurrency/Runtime/async_task_locals_groups.swift b/test/Concurrency/Runtime/async_task_locals_groups.swift index efef04dba4bff..daeed0fb19608 100644 --- a/test/Concurrency/Runtime/async_task_locals_groups.swift +++ b/test/Concurrency/Runtime/async_task_locals_groups.swift @@ -1,4 +1,4 @@ -// RUN: %target-run-simple-swift( -Xfrontend -disable-availability-checking -parse-as-library %import-libdispatch) | %FileCheck %s +// RUN: %target-run-simple-swift( -plugin-path %swift-plugin-dir -Xfrontend -disable-availability-checking -parse-as-library %import-libdispatch) | %FileCheck %s // REQUIRES: executable_test // REQUIRES: concurrency diff --git a/test/Concurrency/Runtime/async_task_locals_prevent_illegal_use_discarding_taskgroup.swift b/test/Concurrency/Runtime/async_task_locals_prevent_illegal_use_discarding_taskgroup.swift index d2cd2dbc2187d..49768f61c824e 100644 --- a/test/Concurrency/Runtime/async_task_locals_prevent_illegal_use_discarding_taskgroup.swift +++ b/test/Concurrency/Runtime/async_task_locals_prevent_illegal_use_discarding_taskgroup.swift @@ -1,4 +1,4 @@ -// RUN: %target-fail-simple-swift( -Xfrontend -disable-availability-checking -parse-as-library %import-libdispatch) 2>&1 | %FileCheck %s +// RUN: %target-fail-simple-swift( -plugin-path %swift-plugin-dir -Xfrontend -disable-availability-checking -parse-as-library %import-libdispatch) 2>&1 | %FileCheck %s // // // TODO: could not figure out how to use 'not --crash' it never is used with target-run-simple-swift // This test is intended to *crash*, so we're using target-fail-simple-swift diff --git a/test/Concurrency/Runtime/async_task_locals_prevent_illegal_use_taskgroup.swift b/test/Concurrency/Runtime/async_task_locals_prevent_illegal_use_taskgroup.swift index fb2b4efd96522..b0709ed0c12e1 100644 --- a/test/Concurrency/Runtime/async_task_locals_prevent_illegal_use_taskgroup.swift +++ b/test/Concurrency/Runtime/async_task_locals_prevent_illegal_use_taskgroup.swift @@ -1,4 +1,4 @@ -// RUN: %target-fail-simple-swift( -Xfrontend -disable-availability-checking -parse-as-library %import-libdispatch) 2>&1 | %FileCheck %s +// RUN: %target-fail-simple-swift( -plugin-path %swift-plugin-dir -Xfrontend -disable-availability-checking -parse-as-library %import-libdispatch) 2>&1 | %FileCheck %s // // // TODO: could not figure out how to use 'not --crash' it never is used with target-run-simple-swift // This test is intended to *crash*, so we're using target-fail-simple-swift diff --git a/test/Concurrency/Runtime/async_task_locals_spawn_let.swift b/test/Concurrency/Runtime/async_task_locals_spawn_let.swift index 743af54ddcc44..78a4f5f8d20f3 100644 --- a/test/Concurrency/Runtime/async_task_locals_spawn_let.swift +++ b/test/Concurrency/Runtime/async_task_locals_spawn_let.swift @@ -1,4 +1,4 @@ -// RUN: %target-run-simple-swift( -Xfrontend -disable-availability-checking -parse-as-library) | %FileCheck %s +// RUN: %target-run-simple-swift( -plugin-path %swift-plugin-dir -Xfrontend -disable-availability-checking -parse-as-library) | %FileCheck %s // REQUIRES: executable_test // REQUIRES: concurrency diff --git a/test/Concurrency/Runtime/async_task_locals_synchronous_bind.swift b/test/Concurrency/Runtime/async_task_locals_synchronous_bind.swift index ee6b792c76fd7..69872e78774d6 100644 --- a/test/Concurrency/Runtime/async_task_locals_synchronous_bind.swift +++ b/test/Concurrency/Runtime/async_task_locals_synchronous_bind.swift @@ -1,4 +1,4 @@ -// RUN: %target-run-simple-swift( -Xfrontend -disable-availability-checking -parse-as-library) | %FileCheck %s +// RUN: %target-run-simple-swift( -plugin-path %swift-plugin-dir -Xfrontend -disable-availability-checking -parse-as-library) | %FileCheck %s // REQUIRES: executable_test // REQUIRES: concurrency diff --git a/test/Concurrency/Runtime/async_task_locals_wrapper.swift b/test/Concurrency/Runtime/async_task_locals_wrapper.swift index de5a6fe97de91..5697c46640585 100644 --- a/test/Concurrency/Runtime/async_task_locals_wrapper.swift +++ b/test/Concurrency/Runtime/async_task_locals_wrapper.swift @@ -1,4 +1,4 @@ -// RUN: %target-run-simple-swift( -Xfrontend -disable-availability-checking -parse-as-library) | %FileCheck %s +// RUN: %target-run-simple-swift( -plugin-path %swift-plugin-dir -Xfrontend -disable-availability-checking -parse-as-library) | %FileCheck %s // REQUIRES: executable_test // REQUIRES: concurrency diff --git a/test/Concurrency/async_task_locals_basic_warnings.swift b/test/Concurrency/async_task_locals_basic_warnings.swift index 95bfcc7f76f06..2a66d880e1c01 100644 --- a/test/Concurrency/async_task_locals_basic_warnings.swift +++ b/test/Concurrency/async_task_locals_basic_warnings.swift @@ -1,8 +1,8 @@ // RUN: %empty-directory(%t) -// RUN: %target-swift-frontend -emit-module -emit-module-path %t/OtherActors.swiftmodule -module-name OtherActors %S/Inputs/OtherActors.swift -disable-availability-checking +// RUN: %target-swift-frontend -plugin-path %swift-plugin-dir -emit-module -emit-module-path %t/OtherActors.swiftmodule -module-name OtherActors %S/Inputs/OtherActors.swift -disable-availability-checking -// RUN: %target-swift-frontend -I %t -disable-availability-checking -strict-concurrency=complete -parse-as-library %s -emit-sil -o /dev/null -verify -// RUN: %target-swift-frontend -I %t -disable-availability-checking -strict-concurrency=complete -parse-as-library %s -emit-sil -o /dev/null -verify -enable-upcoming-feature RegionBasedIsolation +// RUN: %target-swift-frontend -I %t -plugin-path %swift-plugin-dir -disable-availability-checking -strict-concurrency=complete -parse-as-library %s -emit-sil -o /dev/null -verify +// RUN: %target-swift-frontend -I %t -plugin-path %swift-plugin-dir -disable-availability-checking -strict-concurrency=complete -parse-as-library %s -emit-sil -o /dev/null -verify -enable-upcoming-feature RegionBasedIsolation // REQUIRES: concurrency // REQUIRES: asserts diff --git a/test/Concurrency/async_task_locals_basic_warnings_bug_isolation.swift b/test/Concurrency/async_task_locals_basic_warnings_bug_isolation.swift index 849d4f857d38f..8a8349dbff1b8 100644 --- a/test/Concurrency/async_task_locals_basic_warnings_bug_isolation.swift +++ b/test/Concurrency/async_task_locals_basic_warnings_bug_isolation.swift @@ -1,8 +1,8 @@ // RUN: %empty-directory(%t) -// RUN: %target-swift-frontend -emit-module -emit-module-path %t/OtherActors.swiftmodule -module-name OtherActors %S/Inputs/OtherActors.swift -disable-availability-checking +// RUN: %target-swift-frontend -plugin-path %swift-plugin-dir -emit-module -emit-module-path %t/OtherActors.swiftmodule -module-name OtherActors %S/Inputs/OtherActors.swift -disable-availability-checking -// RUN: %target-swift-frontend -I %t -disable-availability-checking -strict-concurrency=complete -parse-as-library %s -emit-sil -o /dev/null -verify -// RUN: %target-swift-frontend -I %t -disable-availability-checking -strict-concurrency=complete -parse-as-library %s -emit-sil -o /dev/null -verify -enable-upcoming-feature RegionBasedIsolation +// RUN: %target-swift-frontend -I %t -plugin-path %swift-plugin-dir -disable-availability-checking -strict-concurrency=complete -parse-as-library %s -emit-sil -o /dev/null -verify +// RUN: %target-swift-frontend -I %t -plugin-path %swift-plugin-dir -disable-availability-checking -strict-concurrency=complete -parse-as-library %s -emit-sil -o /dev/null -verify -enable-upcoming-feature RegionBasedIsolation // REQUIRES: concurrency // REQUIRES: asserts diff --git a/test/Concurrency/task_local.swift b/test/Concurrency/task_local.swift index 58124e627382e..42ac29b63e89e 100644 --- a/test/Concurrency/task_local.swift +++ b/test/Concurrency/task_local.swift @@ -1,27 +1,27 @@ -// RUN: %target-swift-frontend -strict-concurrency=targeted -disable-availability-checking -emit-sil -verify -o /dev/null %s -// RUN: %target-swift-frontend -strict-concurrency=complete -verify-additional-prefix complete- -disable-availability-checking -emit-sil -verify -o /dev/null %s -// RUN: %target-swift-frontend -strict-concurrency=complete -verify-additional-prefix complete- -disable-availability-checking -emit-sil -verify -o /dev/null %s -enable-upcoming-feature RegionBasedIsolation +// RUN: %target-swift-frontend -plugin-path %swift-plugin-dir -strict-concurrency=targeted -disable-availability-checking -emit-sil -verify -o /dev/null %s +// RUN: %target-swift-frontend -plugin-path %swift-plugin-dir -strict-concurrency=complete -verify-additional-prefix complete- -disable-availability-checking -emit-sil -verify -o /dev/null %s +// RUN: %target-swift-frontend -plugin-path %swift-plugin-dir -strict-concurrency=complete -verify-additional-prefix complete- -disable-availability-checking -emit-sil -verify -o /dev/null %s -enable-upcoming-feature RegionBasedIsolation // REQUIRES: concurrency // REQUIRES: asserts @available(SwiftStdlib 5.1, *) struct TL { - @TaskLocal + @TaskLocal // expected-note{{in expansion of macro 'TaskLocal' on static property 'number' here}} static var number: Int = 0 @TaskLocal static var someNil: Int? - @TaskLocal - static var noValue: Int // expected-error{{'static var' declaration requires an initializer expression or an explicitly stated getter}} - // expected-note@-1{{add an initializer to silence this error}} + // expected-note@+1{{in expansion of macro 'TaskLocal' on static property 'noValue' here}} + @TaskLocal // expected-error{{@TaskLocal' property must have default value, or be optional}} + static var noValue: Int // expected-note{{'noValue' declared here}} - @TaskLocal - var notStatic: String? // expected-error{{property 'notStatic', must be static because property wrapper 'TaskLocal' can only be applied to static properties}} + @TaskLocal // expected-error{{'@TaskLocal' can only be applied to 'static' property}} + var notStatic: String? } -@TaskLocal // expected-error{{property wrappers are not yet supported in top-level code}} +@TaskLocal // expected-error{{'@TaskLocal' can only be applied to 'static' property}} var global: Int = 0 class NotSendable {} @@ -29,7 +29,10 @@ class NotSendable {} @available(SwiftStdlib 5.1, *) func test () async { TL.number = 10 // expected-error{{cannot assign to property: 'number' is a get-only property}} + TL.$number = 10 // expected-error{{cannot assign value of type 'Int' to type 'TaskLocal'}} + // expected-error@-1{{cannot assign to property: '$number' is a 'let' constant}} + let _: Int = TL.number let _: Int = TL.$number.get() } From 70ac50b0ccee52d1967124a3e62f19b83eb12fa6 Mon Sep 17 00:00:00 2001 From: Konrad `ktoso` Malawski Date: Thu, 18 Apr 2024 17:52:02 +0900 Subject: [PATCH 2/6] [Concurrency] Allow @TaskLocal on global variables --- .../Sources/SwiftMacros/TaskLocalMacro.swift | 74 ++++++++++++------- lib/Sema/TypeCheckConcurrency.cpp | 9 --- ...wift => async_task_locals_async_let.swift} | 0 test/Concurrency/task_local.swift | 11 ++- 4 files changed, 58 insertions(+), 36 deletions(-) rename test/Concurrency/Runtime/{async_task_locals_spawn_let.swift => async_task_locals_async_let.swift} (100%) diff --git a/lib/Macros/Sources/SwiftMacros/TaskLocalMacro.swift b/lib/Macros/Sources/SwiftMacros/TaskLocalMacro.swift index fb7447b2ca3a4..b2cc6580a257d 100644 --- a/lib/Macros/Sources/SwiftMacros/TaskLocalMacro.swift +++ b/lib/Macros/Sources/SwiftMacros/TaskLocalMacro.swift @@ -14,7 +14,10 @@ import SwiftSyntax import SwiftSyntaxMacros import SwiftDiagnostics -// TODO: docs +/// Macro implementing the TaskLocal functionality. +/// +/// It introduces a peer `static let $name: TaskLocal` as well as a getter +/// that assesses accesses the task local storage. public enum TaskLocalMacro {} extension TaskLocalMacro: PeerMacro { @@ -26,25 +29,29 @@ extension TaskLocalMacro: PeerMacro { guard let varDecl = try requireVar(declaration, diagnose: false) else { return [] } - guard try requireModifier(varDecl, .static, diagnose: false) else { + guard try requireStaticContext(varDecl, in: context, diagnose: false) else { return [] } guard let firstBinding = varDecl.bindings.first else { - return [] // TODO: make error + throw DiagnosticsError( + syntax: declaration, + message: "'@TaskLocal' property must have declared binding", id: .incompatibleDecl) } guard let name = firstBinding.pattern.as(IdentifierPatternSyntax.self)?.identifier else { - return [] // TODO: make error + throw DiagnosticsError( + syntax: declaration, + message: "'@TaskLocal' property must have name", id: .incompatibleDecl) } let type = firstBinding.typeAnnotation?.type - let explicitType: String = - if let type { - ": TaskLocal<\(type.trimmed)>" - } else { - "" - } + let explicitType: String + if let type { + explicitType = ": TaskLocal<\(type.trimmed)>" + } else { + explicitType = "" + } let initialValue: Any if let initializerValue = firstBinding.initializer?.value { @@ -57,9 +64,18 @@ extension TaskLocalMacro: PeerMacro { message: "'@TaskLocal' property must have default value, or be optional", id: .mustBeVar) } + // If the property is global, do not prefix the synthesised decl with 'static' + let isGlobal = context.lexicalContext.isEmpty + let staticKeyword: String + if isGlobal { + staticKeyword = "" + } else { + staticKeyword = "static " + } + return [ """ - static let $\(name)\(raw: explicitType) = TaskLocal(wrappedValue: \(raw: initialValue)) + \(raw: staticKeyword)let $\(name)\(raw: explicitType) = TaskLocal(wrappedValue: \(raw: initialValue)) """ ] } @@ -77,7 +93,7 @@ extension TaskLocalMacro: AccessorMacro { guard let varDecl = try requireVar(declaration) else { return [] } - try requireModifier(varDecl, .static) + try requireStaticContext(varDecl, in: context) guard let firstBinding = varDecl.bindings.first else { return [] // TODO: make error @@ -107,24 +123,29 @@ private func requireVar(_ decl: some DeclSyntaxProtocol, } @discardableResult -private func requireModifier(_ decl: VariableDeclSyntax, - _ keyword: Keyword, - diagnose: Bool = true) throws -> Bool { +private func requireStaticContext(_ decl: VariableDeclSyntax, + in context: some MacroExpansionContext, + diagnose: Bool = true) throws -> Bool { let isStatic = decl.modifiers.contains { modifier in - modifier.name.text == "\(keyword)" + modifier.name.text == "\(Keyword.static)" } - if !isStatic { - if diagnose { - throw DiagnosticsError( - syntax: decl, - message: "'@TaskLocal' can only be applied to 'static' property", id: .mustBeStatic) - } else { - return false - } + if isStatic { + return true + } + + let isGlobal = context.lexicalContext.isEmpty + if isGlobal { + return true } - return true + if diagnose { + throw DiagnosticsError( + syntax: decl, + message: "'@TaskLocal' can only be applied to 'static' property", id: .mustBeStatic) + } + + return false } extension TypeSyntax { @@ -141,8 +162,9 @@ extension TypeSyntax { struct TaskLocalMacroDiagnostic: DiagnosticMessage { enum ID: String { - case mustBeStatic = "must be static" case mustBeVar = "must be var" + case mustBeStatic = "must be static" + case incompatibleDecl = "incompatible declaration" } var message: String diff --git a/lib/Sema/TypeCheckConcurrency.cpp b/lib/Sema/TypeCheckConcurrency.cpp index edca26a413238..4cc97c466a014 100644 --- a/lib/Sema/TypeCheckConcurrency.cpp +++ b/lib/Sema/TypeCheckConcurrency.cpp @@ -4980,15 +4980,6 @@ ActorIsolation ActorIsolationRequest::evaluate( if (var->isGlobalStorage() && !isActorType) { auto *diagVar = var; if (auto *originalVar = var->getOriginalWrappedProperty()) { - // temporary 5.10 checking bypass for @TaskLocal - // TODO: @TaskLocal should be a macro - if (auto *classDecl = - var->getInterfaceType()->getClassOrBoundGenericClass()) { - auto &ctx = var->getASTContext(); - if (classDecl == ctx.getTaskLocalDecl()) { - return isolation; - } - } diagVar = originalVar; } if (var->isLet()) { diff --git a/test/Concurrency/Runtime/async_task_locals_spawn_let.swift b/test/Concurrency/Runtime/async_task_locals_async_let.swift similarity index 100% rename from test/Concurrency/Runtime/async_task_locals_spawn_let.swift rename to test/Concurrency/Runtime/async_task_locals_async_let.swift diff --git a/test/Concurrency/task_local.swift b/test/Concurrency/task_local.swift index 42ac29b63e89e..b4c5e977bdbae 100644 --- a/test/Concurrency/task_local.swift +++ b/test/Concurrency/task_local.swift @@ -21,7 +21,7 @@ struct TL { var notStatic: String? } -@TaskLocal // expected-error{{'@TaskLocal' can only be applied to 'static' property}} +@TaskLocal var global: Int = 0 class NotSendable {} @@ -36,3 +36,12 @@ func test () async { let _: Int = TL.number let _: Int = TL.$number.get() } + +@TaskLocal // expected-error{{'accessor' macro cannot be attached to global function ('test')}} +func test() {} + +class X { + @TaskLocal // expected-error{{'accessor' macro cannot be attached to static method ('test')}} + static func test() { + } +} \ No newline at end of file From 490e67dcd8e713db684a1c5edae645ed97afdad7 Mon Sep 17 00:00:00 2001 From: Konrad `ktoso` Malawski Date: Thu, 18 Apr 2024 18:34:42 +0900 Subject: [PATCH 3/6] [Concurrency] Improve new TaskLocal macro documentation --- CHANGELOG.md | 29 +++++++++++ .../Sources/SwiftMacros/TaskLocalMacro.swift | 52 ++++++++++++------- stdlib/public/Concurrency/TaskLocal.swift | 21 +++++--- 3 files changed, 77 insertions(+), 25 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 74e9dc83c9ba1..1b84df2d6911e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,35 @@ ## Swift 6.0 +* Since its introduction in Swift 5.1 the @TaskLocal property wrapper was used to + create and access task-local value bindings. Property wrappers introduce mutable storage, + which was now properly flagged as potential source of concurrency unsafety. + + In order for Swift 6 language mode to not flag task-locals as potentially thread-unsafe, + task locals are now implemented using a macro. The macro has the same general semantics + and usage patterns, however there are two source-break situations which the Swift 6 + task locals cannot handle: + + Using an implicit default `nil` value for task local initialization, when combined with a type alias: + ```swift + // allowed in Swift 5.x, not allowed in Swift 6.x + + typealias MyValue = Optional + + @TaskLocal + static var number: MyValue // Swift 6: error, please specify default value explicitl + + // Solution 1: Specify the default value + @TaskLocal + static var number: MyValue = nil + + // Solution 2: Avoid the type-alias + @TaskLocal + static var number: Optional + ``` + + At the same time, task locals can now be declared as global properties, which wasn't possible before. + * Swift 5.10 missed a semantic check from [SE-0309][]. In type context, a reference to a protocol `P` that has associated types or `Self` requirements should use the `any` keyword, but this was not enforced in nested generic argument positions. diff --git a/lib/Macros/Sources/SwiftMacros/TaskLocalMacro.swift b/lib/Macros/Sources/SwiftMacros/TaskLocalMacro.swift index b2cc6580a257d..c42befa8e29a2 100644 --- a/lib/Macros/Sources/SwiftMacros/TaskLocalMacro.swift +++ b/lib/Macros/Sources/SwiftMacros/TaskLocalMacro.swift @@ -33,6 +33,11 @@ extension TaskLocalMacro: PeerMacro { return [] } + guard varDecl.bindings.count == 1 else { + throw DiagnosticsError( + syntax: declaration, + message: "'@TaskLocal' property must have exactly one binding", id: .incompatibleDecl) + } guard let firstBinding = varDecl.bindings.first else { throw DiagnosticsError( syntax: declaration, @@ -46,18 +51,18 @@ extension TaskLocalMacro: PeerMacro { } let type = firstBinding.typeAnnotation?.type - let explicitType: String + let explicitTypeAnnotation: TypeAnnotationSyntax? if let type { - explicitType = ": TaskLocal<\(type.trimmed)>" + explicitTypeAnnotation = TypeAnnotationSyntax(type: TypeSyntax("TaskLocal<\(type.trimmed)>")) } else { - explicitType = "" + explicitTypeAnnotation = nil } - let initialValue: Any + let initialValue: ExprSyntax if let initializerValue = firstBinding.initializer?.value { - initialValue = initializerValue + initialValue = ExprSyntax(initializerValue) } else if let type, type.isOptional { - initialValue = "nil" + initialValue = ExprSyntax(NilLiteralExprSyntax()) } else { throw DiagnosticsError( syntax: declaration, @@ -66,16 +71,16 @@ extension TaskLocalMacro: PeerMacro { // If the property is global, do not prefix the synthesised decl with 'static' let isGlobal = context.lexicalContext.isEmpty - let staticKeyword: String + let staticKeyword: TokenSyntax? if isGlobal { - staticKeyword = "" + staticKeyword = nil } else { - staticKeyword = "static " + staticKeyword = TokenSyntax.keyword(.static, trailingTrivia: .space) } return [ """ - \(raw: staticKeyword)let $\(name)\(raw: explicitType) = TaskLocal(wrappedValue: \(raw: initialValue)) + \(staticKeyword)let $\(name)\(explicitTypeAnnotation) = TaskLocal(wrappedValue: \(initialValue)) """ ] } @@ -96,11 +101,11 @@ extension TaskLocalMacro: AccessorMacro { try requireStaticContext(varDecl, in: context) guard let firstBinding = varDecl.bindings.first else { - return [] // TODO: make error + return [] } guard let name = firstBinding.pattern.as(IdentifierPatternSyntax.self)?.identifier else { - return [] // TODO: make error + return [] } return ["get { $\(name).get() }"] @@ -142,7 +147,7 @@ private func requireStaticContext(_ decl: VariableDeclSyntax, if diagnose { throw DiagnosticsError( syntax: decl, - message: "'@TaskLocal' can only be applied to 'static' property", id: .mustBeStatic) + message: "'@TaskLocal' can only be applied to 'static' property, or global variables", id: .mustBeStatic) } return false @@ -153,10 +158,19 @@ extension TypeSyntax { // has no type information, but at least for the common case for Optional // and T? we can detect the optional. fileprivate var isOptional: Bool { - let strRepr = "\(self)" - return strRepr.last == "?" || - strRepr.starts(with: "Optional<") || - strRepr.starts(with: "Swift.Optional<") + switch self.as(TypeSyntaxEnum.self) { + case .optionalType: + return true + case .identifierType(let identifierType): + return identifierType.name.text == "Optional" + case .memberType(let memberType): + guard let baseIdentifier = memberType.baseType.as(IdentifierTypeSyntax.self), + baseIdentifier.name.text == "Swift" else { + return false + } + return memberType.name.text == "Optional" + default: return false + } } } @@ -185,8 +199,8 @@ struct TaskLocalMacroDiagnostic: DiagnosticMessage { } extension DiagnosticsError { - init( - syntax: S, + init( + syntax: some SyntaxProtocol, message: String, domain: String = "Swift", id: TaskLocalMacroDiagnostic.ID, diff --git a/stdlib/public/Concurrency/TaskLocal.swift b/stdlib/public/Concurrency/TaskLocal.swift index 19bfb96197582..98721e71b1c36 100644 --- a/stdlib/public/Concurrency/TaskLocal.swift +++ b/stdlib/public/Concurrency/TaskLocal.swift @@ -17,7 +17,11 @@ import Swift // Macros are disabled when Swift is built without swift-syntax. #if $Macros && hasAttribute(attached) -// TODO: docs +/// Macro that introduces a ``TaskLocal-class`` binding. +/// +/// For information about task-local bindings, see ``TaskLocal-class``. +/// +/// - SeeAlso: ``TaskLocal-class`` @available(SwiftStdlib 5.1, *) @attached(accessor) @attached(peer, names: prefixed(`$`)) @@ -26,22 +30,25 @@ public macro TaskLocal() = #endif -/// Wrapper that defines a task-local value key. +/// Wrapper type that defines a task-local value key. /// /// A task-local value is a value that can be bound and read in the context of a -/// `Task`. It is implicitly carried with the task, and is accessible by any -/// child tasks the task creates (such as TaskGroup or `async let` created tasks). +/// ``Task``. It is implicitly carried with the task, and is accessible by any +/// child tasks it creates (such as TaskGroup or `async let` created tasks). /// /// ### Task-local declarations /// -/// Task locals must be declared as static properties (or global properties, -/// once property wrappers support these), like this: +/// Task locals must be declared as static properties or global properties, like this: /// /// enum Example { /// @TaskLocal /// static var traceID: TraceID? /// } /// +/// // Global task local properties are supported since Swift 6.0: +/// @TaskLocal +/// var contextualNumber: Int = 12 +/// /// ### Default values /// Reading a task local value when no value was bound to it results in returning /// its default value. For a task local declared as optional (such as e.g. `TraceID?`), @@ -150,6 +157,8 @@ public macro TaskLocal() = /// read() // traceID: nil /// } /// } +/// +/// - SeeAlso: ``TaskLocal-macro`` @available(SwiftStdlib 5.1, *) public final class TaskLocal: Sendable, CustomStringConvertible { let defaultValue: Value From 4b515edca4b422d75dfe0539a034953ac8063a6b Mon Sep 17 00:00:00 2001 From: Konrad `ktoso` Malawski Date: Tue, 30 Apr 2024 13:53:17 +0900 Subject: [PATCH 4/6] fix typos --- CHANGELOG.md | 2 +- lib/Macros/Sources/SwiftMacros/TaskLocalMacro.swift | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1b84df2d6911e..da8123b4c2624 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,7 +21,7 @@ typealias MyValue = Optional @TaskLocal - static var number: MyValue // Swift 6: error, please specify default value explicitl + static var number: MyValue // Swift 6: error, please specify default value explicitly // Solution 1: Specify the default value @TaskLocal diff --git a/lib/Macros/Sources/SwiftMacros/TaskLocalMacro.swift b/lib/Macros/Sources/SwiftMacros/TaskLocalMacro.swift index c42befa8e29a2..921a80bbdd9d1 100644 --- a/lib/Macros/Sources/SwiftMacros/TaskLocalMacro.swift +++ b/lib/Macros/Sources/SwiftMacros/TaskLocalMacro.swift @@ -17,7 +17,7 @@ import SwiftDiagnostics /// Macro implementing the TaskLocal functionality. /// /// It introduces a peer `static let $name: TaskLocal` as well as a getter -/// that assesses accesses the task local storage. +/// that accesses the task local storage. public enum TaskLocalMacro {} extension TaskLocalMacro: PeerMacro { From 95da1b529d5723e8c14b51644e38d6106a8d44ea Mon Sep 17 00:00:00 2001 From: Konrad `ktoso` Malawski Date: Tue, 30 Apr 2024 16:25:54 +0900 Subject: [PATCH 5/6] add missing swift-plugin-dir to test --- validation-test/Concurrency/rdar107275872.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/validation-test/Concurrency/rdar107275872.swift b/validation-test/Concurrency/rdar107275872.swift index 76213531a44dc..fd56f6970a30c 100644 --- a/validation-test/Concurrency/rdar107275872.swift +++ b/validation-test/Concurrency/rdar107275872.swift @@ -1,5 +1,5 @@ // RUN: %empty-directory(%t) -// RUN: %target-build-swift -O -Xfrontend -disable-availability-checking %s -parse-as-library -module-name main -o %t/main +// RUN: %target-build-swift -O -Xfrontend -disable-availability-checking %s -plugin-path %swift-plugin-dir -parse-as-library -module-name main -o %t/main // RUN: %target-codesign %t/main // RUN: %target-run %t/main | %FileCheck %s From 2ac9f705da379305a637047191074b72c629aeda Mon Sep 17 00:00:00 2001 From: Konrad `ktoso` Malawski Date: Tue, 30 Apr 2024 21:51:10 +0900 Subject: [PATCH 6/6] [Concurrency] Extra runtime test for global task local --- .../Runtime/async_task_locals_basic.swift | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/test/Concurrency/Runtime/async_task_locals_basic.swift b/test/Concurrency/Runtime/async_task_locals_basic.swift index 6ce6cada44a1e..eab78a0d51c63 100644 --- a/test/Concurrency/Runtime/async_task_locals_basic.swift +++ b/test/Concurrency/Runtime/async_task_locals_basic.swift @@ -8,11 +8,14 @@ // REQUIRES: concurrency_runtime // UNSUPPORTED: back_deployment_runtime -final class StringLike: Sendable, CustomStringConvertible { +final class StringLike: Sendable, ExpressibleByStringLiteral, CustomStringConvertible { let value: String init(_ value: String) { self.value = value } + init(stringLiteral value: StringLiteralType) { + self.value = value + } var description: String { value } } @@ -33,6 +36,9 @@ enum TL { static var clazz: ClassTaskLocal? } +@TaskLocal +var globalTaskLocal: StringLike = StringLike("") + @available(SwiftStdlib 5.1, *) final class ClassTaskLocal: Sendable { init() { @@ -217,6 +223,13 @@ func inside_actor() async { await Worker().setAndRead() } +@available(SwiftStdlib 5.1, *) +func global_task_local() async { + await $globalTaskLocal.withValue("value-1") { + await printTaskLocalAsync($globalTaskLocal) // CHECK-NEXT: TaskLocal(defaultValue: ) (value-1) + } +} + @available(SwiftStdlib 5.1, *) @main struct Main { static func main() async { @@ -229,5 +242,6 @@ func inside_actor() async { await nested_3_onlyTopContributesAsync() await nested_3_onlyTopContributesMixed() await inside_actor() + await global_task_local() } }