Skip to content

Commit 7097c2b

Browse files
authored
Add unsafe keyword handling to macro expansions. (#1069)
This PR changes the `@Test` and `#expect()` macros so they handle `unsafe` expressions the same way `try` and `await` expressions are handled. The propagation rules for `unsafe` aren't the same as for `try` and `await` (i.e. it doesn't colour the calling function), but the general way we handle the keyword is the same. I haven't attempted to avoid inserting `unsafe` if a function is not marked `@unsafe` as it complicates the necessary logic but has no effects at runtime. ### Checklist: - [x] Code and documentation should follow the style of the [Style Guide](https://github.com/apple/swift-testing/blob/main/Documentation/StyleGuide.md). - [x] If public symbols are renamed or modified, DocC references should be updated.
1 parent 72bed50 commit 7097c2b

File tree

7 files changed

+184
-18
lines changed

7 files changed

+184
-18
lines changed

Package.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,7 @@ let package = Package(
9696
}(),
9797

9898
dependencies: [
99-
.package(url: "https://github.com/swiftlang/swift-syntax.git", from: "601.0.0-latest"),
99+
.package(url: "https://github.com/swiftlang/swift-syntax.git", from: "602.0.0-latest"),
100100
],
101101

102102
targets: [

Sources/Testing/Test+Macro.swift

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -537,6 +537,15 @@ extension Test {
537537
value
538538
}
539539

540+
/// A function that abstracts away whether or not the `unsafe` keyword is needed
541+
/// on an expression.
542+
///
543+
/// - Warning: This function is used to implement the `@Test` macro. Do not use
544+
/// it directly.
545+
@unsafe @inlinable public func __requiringUnsafe<T>(_ value: consuming T) throws -> T where T: ~Copyable {
546+
value
547+
}
548+
540549
/// The current default isolation context.
541550
///
542551
/// - Warning: This property is used to implement the `@Test` macro. Do not call

Sources/TestingMacros/CMakeLists.txt

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ if(SwiftTesting_BuildMacrosAsExecutables)
3131
set(FETCHCONTENT_BASE_DIR ${CMAKE_BINARY_DIR}/_d)
3232
FetchContent_Declare(SwiftSyntax
3333
GIT_REPOSITORY https://github.com/swiftlang/swift-syntax
34-
GIT_TAG 1cd35348b089ff8966588742c69727205d99f8ed) # 601.0.0-prerelease-2024-11-18
34+
GIT_TAG 340f8400262d494c7c659cd838223990195d7fed) # 602.0.0-prerelease-2025-04-10
3535
FetchContent_MakeAvailable(SwiftSyntax)
3636
endif()
3737

@@ -101,6 +101,7 @@ target_sources(TestingMacros PRIVATE
101101
Support/ConditionArgumentParsing.swift
102102
Support/DiagnosticMessage.swift
103103
Support/DiagnosticMessage+Diagnosing.swift
104+
Support/EffectfulExpressionHandling.swift
104105
Support/SHA256.swift
105106
Support/SourceCodeCapturing.swift
106107
Support/SourceLocationGeneration.swift

Sources/TestingMacros/ConditionMacro.swift

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,6 @@ extension ConditionMacro {
117117
var checkArguments = [Argument]()
118118
do {
119119
if let trailingClosureIndex {
120-
121120
// Include all arguments other than the "comment" and "sourceLocation"
122121
// arguments here.
123122
checkArguments += macroArguments.indices.lazy
@@ -458,7 +457,7 @@ extension ExitTestConditionMacro {
458457
decls.append(
459458
"""
460459
@Sendable func \(bodyThunkName)() async throws -> Swift.Void {
461-
return try await Testing.__requiringTry(Testing.__requiringAwait(\(bodyArgumentExpr.trimmed)))()
460+
return \(applyEffectfulKeywords([.try, .await, .unsafe], to: bodyArgumentExpr))()
462461
}
463462
"""
464463
)

Sources/TestingMacros/Support/ConditionArgumentParsing.swift

Lines changed: 9 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -472,17 +472,6 @@ private func _parseCondition(from expr: ExprSyntax, for macro: some Freestanding
472472
return _parseCondition(from: closureExpr, for: macro, in: context)
473473
}
474474

475-
// If the condition involves the `try` or `await` keywords, assume we cannot
476-
// expand it. This check cannot handle expressions like
477-
// `try #expect(a.b(c))` where `b()` is throwing because the `try` keyword is
478-
// outside the macro expansion. SEE: rdar://109470248
479-
let containsTryOrAwait = expr.tokens(viewMode: .sourceAccurate).lazy
480-
.map(\.tokenKind)
481-
.contains { $0 == .keyword(.try) || $0 == .keyword(.await) }
482-
if containsTryOrAwait {
483-
return Condition(expression: expr)
484-
}
485-
486475
if let infixOperator = expr.as(InfixOperatorExprSyntax.self),
487476
let op = infixOperator.operator.as(BinaryOperatorExprSyntax.self) {
488477
return _parseCondition(from: expr, leftOperand: infixOperator.leftOperand, operator: op, rightOperand: infixOperator.rightOperand, for: macro, in: context)
@@ -527,6 +516,15 @@ private func _parseCondition(from expr: ExprSyntax, for macro: some Freestanding
527516
///
528517
/// - Returns: An instance of ``Condition`` describing `expr`.
529518
func parseCondition(from expr: ExprSyntax, for macro: some FreestandingMacroExpansionSyntax, in context: some MacroExpansionContext) -> Condition {
519+
// If the condition involves the `unsafe`, `try`, or `await` keywords, assume
520+
// we cannot expand it. This check cannot handle expressions like
521+
// `try #expect(a.b(c))` where `b()` is throwing because the `try` keyword is
522+
// outside the macro expansion. SEE: rdar://109470248
523+
let effectKeywordsToApply = findEffectKeywords(in: expr, context: context)
524+
guard effectKeywordsToApply.intersection([.unsafe, .try, .await]).isEmpty else {
525+
return Condition(expression: expr)
526+
}
527+
530528
_diagnoseTrivialBooleanValue(from: expr, for: macro, in: context)
531529
let result = _parseCondition(from: expr, for: macro, in: context)
532530
return result
Lines changed: 159 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,159 @@
1+
//
2+
// This source file is part of the Swift.org open source project
3+
//
4+
// Copyright (c) 2024 Apple Inc. and the Swift project authors
5+
// Licensed under Apache License v2.0 with Runtime Library Exception
6+
//
7+
// See https://swift.org/LICENSE.txt for license information
8+
// See https://swift.org/CONTRIBUTORS.txt for Swift project authors
9+
//
10+
11+
import SwiftSyntax
12+
import SwiftSyntaxBuilder
13+
import SwiftSyntaxMacros
14+
15+
// MARK: - Finding effect keywords
16+
17+
/// A syntax visitor class that looks for effectful keywords in a given
18+
/// expression.
19+
private final class _EffectFinder: SyntaxAnyVisitor {
20+
/// The effect keywords discovered so far.
21+
var effectKeywords: Set<Keyword> = []
22+
23+
override func visitAny(_ node: Syntax) -> SyntaxVisitorContinueKind {
24+
switch node.kind {
25+
case .tryExpr:
26+
effectKeywords.insert(.try)
27+
case .awaitExpr:
28+
effectKeywords.insert(.await)
29+
case .consumeExpr:
30+
effectKeywords.insert(.consume)
31+
case .borrowExpr:
32+
effectKeywords.insert(.borrow)
33+
case .unsafeExpr:
34+
effectKeywords.insert(.unsafe)
35+
case .closureExpr, .functionDecl:
36+
// Do not delve into closures or function declarations.
37+
return .skipChildren
38+
case .variableDecl:
39+
// Delve into variable declarations.
40+
return .visitChildren
41+
default:
42+
// Do not delve into declarations other than variables.
43+
if node.isProtocol((any DeclSyntaxProtocol).self) {
44+
return .skipChildren
45+
}
46+
}
47+
48+
// Recurse into everything else.
49+
return .visitChildren
50+
}
51+
}
52+
53+
/// Find effectful keywords in a syntax node.
54+
///
55+
/// - Parameters:
56+
/// - node: The node to inspect.
57+
/// - context: The macro context in which the expression is being parsed.
58+
///
59+
/// - Returns: A set of effectful keywords such as `await` that are present in
60+
/// `node`.
61+
///
62+
/// This function does not descend into function declarations or closure
63+
/// expressions because they represent distinct lexical contexts and their
64+
/// effects are uninteresting in the context of `node` unless they are called.
65+
func findEffectKeywords(in node: some SyntaxProtocol, context: some MacroExpansionContext) -> Set<Keyword> {
66+
// TODO: gather any effects from the lexical context once swift-syntax-#3037 and related PRs land
67+
let effectFinder = _EffectFinder(viewMode: .sourceAccurate)
68+
effectFinder.walk(node)
69+
return effectFinder.effectKeywords
70+
}
71+
72+
// MARK: - Inserting effect keywords/thunks
73+
74+
/// Make a function call expression to an effectful thunk function provided by
75+
/// the testing library.
76+
///
77+
/// - Parameters:
78+
/// - thunkName: The unqualified name of the thunk function to call. This
79+
/// token must be the name of a function in the `Testing` module.
80+
/// - expr: The expression to thunk.
81+
///
82+
/// - Returns: An expression representing a call to the function named
83+
/// `thunkName`, passing `expr`.
84+
private func _makeCallToEffectfulThunk(_ thunkName: TokenSyntax, passing expr: some ExprSyntaxProtocol) -> ExprSyntax {
85+
ExprSyntax(
86+
FunctionCallExprSyntax(
87+
calledExpression: MemberAccessExprSyntax(
88+
base: DeclReferenceExprSyntax(baseName: .identifier("Testing")),
89+
declName: DeclReferenceExprSyntax(baseName: thunkName)
90+
),
91+
leftParen: .leftParenToken(),
92+
rightParen: .rightParenToken()
93+
) {
94+
LabeledExprSyntax(expression: expr.trimmed)
95+
}
96+
)
97+
}
98+
99+
/// Apply the given effectful keywords (i.e. `try` and `await`) to an expression
100+
/// using thunk functions provided by the testing library.
101+
///
102+
/// - Parameters:
103+
/// - effectfulKeywords: The effectful keywords to apply.
104+
/// - expr: The expression to apply the keywords and thunk functions to.
105+
///
106+
/// - Returns: A copy of `expr` if no changes are needed, or an expression that
107+
/// adds the keywords in `effectfulKeywords` to `expr`.
108+
func applyEffectfulKeywords(_ effectfulKeywords: Set<Keyword>, to expr: some ExprSyntaxProtocol) -> ExprSyntax {
109+
let originalExpr = expr
110+
var expr = ExprSyntax(expr)
111+
112+
let needAwait = effectfulKeywords.contains(.await) && !expr.is(AwaitExprSyntax.self)
113+
let needTry = effectfulKeywords.contains(.try) && !expr.is(TryExprSyntax.self)
114+
let needUnsafe = effectfulKeywords.contains(.unsafe) && !expr.is(UnsafeExprSyntax.self)
115+
116+
// First, add thunk function calls.
117+
if needAwait {
118+
expr = _makeCallToEffectfulThunk(.identifier("__requiringAwait"), passing: expr)
119+
}
120+
if needTry {
121+
expr = _makeCallToEffectfulThunk(.identifier("__requiringTry"), passing: expr)
122+
}
123+
if needUnsafe {
124+
expr = _makeCallToEffectfulThunk(.identifier("__requiringUnsafe"), passing: expr)
125+
}
126+
127+
// Then add keyword expressions. (We do this separately so we end up writing
128+
// `try await __r(__r(self))` instead of `try __r(await __r(self))` which is
129+
// less accepted by the compiler.)
130+
if needAwait {
131+
expr = ExprSyntax(
132+
AwaitExprSyntax(
133+
awaitKeyword: .keyword(.await).with(\.trailingTrivia, .space),
134+
expression: expr
135+
)
136+
)
137+
}
138+
if needTry {
139+
expr = ExprSyntax(
140+
TryExprSyntax(
141+
tryKeyword: .keyword(.try).with(\.trailingTrivia, .space),
142+
expression: expr
143+
)
144+
)
145+
}
146+
if needUnsafe {
147+
expr = ExprSyntax(
148+
UnsafeExprSyntax(
149+
unsafeKeyword: .keyword(.unsafe).with(\.trailingTrivia, .space),
150+
expression: expr
151+
)
152+
)
153+
}
154+
155+
expr.leadingTrivia = originalExpr.leadingTrivia
156+
expr.trailingTrivia = originalExpr.trailingTrivia
157+
158+
return expr
159+
}

Sources/TestingMacros/TestDeclarationMacro.swift

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -246,17 +246,17 @@ public struct TestDeclarationMacro: PeerMacro, Sendable {
246246
// detecting isolation to other global actors.
247247
lazy var isMainActorIsolated = !functionDecl.attributes(named: "MainActor", inModuleNamed: "_Concurrency").isEmpty
248248
var forwardCall: (ExprSyntax) -> ExprSyntax = {
249-
"try await Testing.__requiringTry(Testing.__requiringAwait(\($0)))"
249+
applyEffectfulKeywords([.try, .await, .unsafe], to: $0)
250250
}
251251
let forwardInit = forwardCall
252252
if functionDecl.noasyncAttribute != nil {
253253
if isMainActorIsolated {
254254
forwardCall = {
255-
"try await MainActor.run { try Testing.__requiringTry(\($0)) }"
255+
"try await MainActor.run { \(applyEffectfulKeywords([.try, .unsafe], to: $0)) }"
256256
}
257257
} else {
258258
forwardCall = {
259-
"try { try Testing.__requiringTry(\($0)) }()"
259+
"try { \(applyEffectfulKeywords([.try, .unsafe], to: $0)) }()"
260260
}
261261
}
262262
}

0 commit comments

Comments
 (0)