Implement thread safety macros#436
Conversation
There was a problem hiding this comment.
Pull request overview
Introduces Swift macros to automate generation of thread-safe stored properties/initializers (via an internal mutex-protected state), and wires the new macro targets + swift-syntax dependency into the Swift package.
Changes:
- Add new macro plugin + macro interface targets and
swift-syntaxdependency inPackage.swift. - Implement
@ThreadSafe,@ThreadSafeProperty, and@ThreadSafeInitializermacros plus supporting SwiftSyntax parsing extensions. - Add a reusable
Mutexconcurrency utility toArgmaxCore.
Reviewed changes
Copilot reviewed 10 out of 11 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| Sources/ArgmaxCoreMacros/Macros.swift | Declares public macro interfaces (ThreadSafe, ThreadSafeProperty, ThreadSafeInitializer) and TypeErased. |
| Sources/ArgmaxCoreMacroPlugin/ArgmaxCoreMacroPlugin.swift | Registers the macro implementations with the compiler plugin entry point. |
| Sources/ArgmaxCoreMacroPlugin/Macros/ThreadSafeMacro.swift | Generates _InternalState/_internalState + inLock, and auto-attaches property/initializer macros. |
| Sources/ArgmaxCoreMacroPlugin/Macros/ThreadSafePropertyMacro.swift | Generates accessors that redirect reads/writes through _internalState. |
| Sources/ArgmaxCoreMacroPlugin/Macros/ThreadSafeInitializerMacro.swift | Rewrites initializer bodies to stage property values then initialize _internalState. |
| Sources/ArgmaxCoreMacroPlugin/Extensions/ClassDeclSyntax+Extensions.swift | Discovers stored vars and (heuristically) infers types/defaults for macro expansion. |
| Sources/ArgmaxCoreMacroPlugin/Extensions/VariableDeclSyntax+Extensions.swift | Adds helper to identify eligible mutable stored properties. |
| Sources/ArgmaxCoreMacroPlugin/Extensions/TypeSyntax+Extensions.swift | Adds optional-type default value helper. |
| Sources/ArgmaxCore/ConcurrencyUtilities.swift | Adds Mutex wrapper around OSAllocatedUnfairLock and retroactive Sendable for KeyPath. |
| Package.swift | Adds macro targets/test target and swift-syntax package dependency. |
| Package.resolved | Pins swift-syntax and updates resolved format metadata. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } else if let defaultValue { | ||
| // Heuristically tries to infer the type from the default value | ||
| let value = stripTrailingCallSuffix(from: defaultValue) | ||
| .trimmingCharacters(in: .whitespacesAndNewlines) | ||
|
|
||
| let type: String = | ||
| if value == "true" || value == "false" { | ||
| "Bool" | ||
| } else if value.wholeMatch(of: Self.integerLiteralRegex) != nil { | ||
| "Int" | ||
| } else if value.wholeMatch(of: Self.doubleLiteralRegex) != nil { | ||
| "Double" | ||
| } else if value.wholeMatch(of: Self.quotedStringRegex) != nil { | ||
| "String" | ||
| } else { | ||
| value | ||
| } | ||
| storedVars.append((name, type, defaultValue)) |
There was a problem hiding this comment.
The type inference fallback uses the default value text as the inferred type when it doesn’t match a simple literal (e.g. .init(), .someCase(), [], [:]). This can produce invalid types like .init in generated code when the property has no explicit type annotation. Consider detecting these patterns and emitting a diagnostic requiring an explicit type annotation (or expanding inference to handle common cases).
| /// Regex to match "self.key = " with flexible whitespace | ||
| /// Handles standard spacing, multiple spaces, tabs, etc. | ||
| private static func selfDotPropertyEqual( | ||
| _ propertyName: String, | ||
| isAtStart: Bool = false | ||
| ) -> Regex<Regex<Substring>.RegexOutput> | ||
| { | ||
| if isAtStart { | ||
| Regex { | ||
| Anchor.startOfLine | ||
| "self." | ||
| propertyName | ||
| OneOrMore(.whitespace) | ||
| "=" | ||
| } | ||
| } else { | ||
| Regex { | ||
| "self." | ||
| propertyName | ||
| OneOrMore(.whitespace) | ||
| "=" | ||
| } | ||
| } | ||
| } | ||
|
|
||
| /// Regex to match "key = " with flexible whitespace | ||
| /// Handles standard spacing, multiple spaces, tabs, etc. | ||
| private static func propertyEqual(_ propertyName: String, isAtStart: Bool = false) -> Regex<Regex<Substring>.RegexOutput> { | ||
| if isAtStart { | ||
| Regex { | ||
| Anchor.startOfLine | ||
| propertyName | ||
| OneOrMore(.whitespace) | ||
| "=" | ||
| } | ||
| } else { | ||
| Regex { | ||
| propertyName | ||
| OneOrMore(.whitespace) | ||
| "=" | ||
| } |
There was a problem hiding this comment.
The regexes used to detect assignments don’t enforce identifier boundaries (e.g. property foo will match foobar = ... / self.foobar = ...) and also require at least one whitespace before =. This can cause incorrect rewrites of unrelated assignments or miss valid ones. Consider matching full identifiers (word boundary or negative lookahead for identifier characters) and allowing optional whitespace around =.
| var isMutable: Bool { | ||
| guard | ||
| bindingSpecifier.text == "var", | ||
| attributes.isEmpty, |
There was a problem hiding this comment.
isMutable currently returns false whenever a var has any attributes (attributes.isEmpty). This prevents @ThreadSafe from tracking properties that already have attributes (including @ThreadSafeProperty itself, property wrappers, etc.) and makes the later “already tracked” attribute check in ThreadSafeMacro unreachable. Consider relaxing this to allow attributes and instead filter only unsupported cases (e.g. computed/accessor vars, multiple bindings, etc.).
| attributes.isEmpty, |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 14 out of 15 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| let hasInitializer = classDecl.memberBlock.members.contains(where: { $0.decl.as(InitializerDeclSyntax.self) != nil }) | ||
| if !hasInitializer { | ||
| // Generate and initialize _internalState property | ||
| let variables = try storedVariables.map { name, _, defaultValue in | ||
| guard let defaultValue else { | ||
| throw DiagnosticsError( | ||
| syntax: classDecl, | ||
| message: "Property '\(name)' must have a default value or the class must define an initializer.") | ||
| } | ||
| return "\(name): \(defaultValue)" | ||
| } | ||
| let decl = "private let \(Constants.internalStateName) = Mutex<_InternalState>(_InternalState(\(variables.joined(separator: ", "))))" | ||
| let internalStateProperty = DeclSyntax(""" | ||
| \(raw: decl) | ||
| """) | ||
| members.append(internalStateProperty) | ||
| } else { | ||
| // Generate _internalState property | ||
| let internalStateProperty = DeclSyntax(""" | ||
| private let \(raw: Constants.internalStateName): Mutex<_InternalState> | ||
| """) | ||
| members.append(internalStateProperty) | ||
| } |
There was a problem hiding this comment.
hasInitializer is true if any initializer exists, but _internalState initialization is only injected via @ThreadSafeInitializer onto non-convenience initializers. A class (especially a subclass) can legally declare only convenience initializers and rely on an inherited designated initializer; in that case this macro will emit an uninitialized private let _internalState: Mutex<_InternalState> with no initializer rewriting to assign it, causing a new compile error. Consider basing this on whether the class declares a designated initializer (non-convenience), or fall back to inline _internalState initialization from defaults when there is no designated initializer in the declaration.
| var isMutable: Bool { | ||
| guard | ||
| bindingSpecifier.text == "var", | ||
| attributes.isEmpty, | ||
| bindings.count == 1, |
There was a problem hiding this comment.
VariableDeclSyntax.isMutable requires attributes.isEmpty, which means a stored property already annotated with @ThreadSafeProperty will be excluded from ClassDeclSyntax.storedVariables. As a result, @ThreadSafe can generate an _InternalState that omits those fields, and the accessor expansion will then reference missing members (compile-time error). Consider splitting this into (a) “is mutable stored identifier binding” and (b) “eligible for auto-annotation”, and ensure storedVariables includes vars that already have @ThreadSafeProperty even if they have attributes.
| private let _internalState = Mutex<_InternalState>(_InternalState()) | ||
|
|
||
| private struct _InternalState: Sendable { |
There was a problem hiding this comment.
This test’s expected expansion generates an empty _InternalState even though count is still expanded by @ThreadSafeProperty into accessors that read/write _internalState.value.count. That combination won’t compile because _InternalState lacks a count field. Once storedVariables includes pre-annotated @ThreadSafeProperty vars, the expected expansion here should include count in _InternalState and initialize _internalState with count: 0.
| private let _internalState = Mutex<_InternalState>(_InternalState()) | |
| private struct _InternalState: Sendable { | |
| private let _internalState = Mutex<_InternalState>(_InternalState(count: 0)) | |
| private struct _InternalState: Sendable { | |
| var count: Int |
This pull request introduces a new macro system to the project, enabling thread-safe class properties and initializers using Swift macros. The changes include new macro targets and dependencies in the package manifest, the implementation of thread-safety macros, and supporting extensions for syntax parsing and code generation. Additionally, a reusable
Mutexclass is added for concurrency utilities. These updates lay the groundwork for automating thread-safety in classes with minimal manual intervention.Macro system integration
ArgmaxCoreMacroPlugin,ArgmaxCoreMacros, andArgmaxCoreMacrosTests) and theswift-syntaxpackage dependency toPackage.swiftfor macro support.ArgmaxCoreMacroPlugin.swift, registering macros for thread-safe properties and initializers.Thread-safety macros
ThreadSafeMacroto generate an internal state struct and a mutex for class properties, and auto-apply thread-safe attributes to class members and initializers.ThreadSafeInitializerMacroto rewrite initializers for thread-safe state setup, including property mutation and default value handling.Syntax extensions for macro logic
ClassDeclSyntax,VariableDeclSyntax, andTypeSyntaxto facilitate parsing, type inference, and property analysis for macro expansion.Concurrency utilities
Mutexclass toConcurrencyUtilities.swiftfor locking and mutation of internal state, plus a retroactiveSendableconformance for key paths.