Skip to content

Commit 46afc15

Browse files
authored
Don't capture arguments to #expect(exitsWith:) during macro expansion. (#912)
Other than the body closure, the arguments to `#expect(exitsWith:)` are not guaranteed to be literals or otherwise context-free, but we capture them during macro expansion. This can result in obscure errors if the developer writes an exit test with an argument that is computed: ```swift let foo = ... await #expect(exitsWith: self.exitCondition(for: foo)) { // 🛑 error: closure captures 'foo' before it is declared // 🛑 error: enum declaration cannot close over value 'self' defined in outer scope // (and other possible errors) ... } ``` This PR removes the macro's compile-time dependency on its `exitCondition` and `sourceLocation` arguments and introduces an `ID` type to identify exit tests (instead of using their source locations as unique IDs.) The `ID` type is meant to be a UUID, but for the moment I've just strung together two 64-bit integers instead to avoid a dependency on Foundation or platform API. ### 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 5494982 commit 46afc15

File tree

7 files changed

+84
-58
lines changed

7 files changed

+84
-58
lines changed

Documentation/ABI/TestContent.md

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -118,9 +118,17 @@ to by `hint` depend on the kind of record:
118118
- For exit test declarations (kind `0x65786974`), the accessor produces a
119119
structure describing the exit test (of type `__ExitTest`.)
120120

121-
Test content records of this kind accept a `hint` of type `SourceLocation`.
122-
They only produce a result if they represent an exit test declared at the same
123-
source location (or if the hint is `nil`.)
121+
Test content records of this kind accept a `hint` of type `__ExitTest.ID`.
122+
They only produce a result if they represent an exit test declared with the
123+
same ID (or if `hint` is `nil`.)
124+
125+
> [!WARNING]
126+
> Calling code should use [`withUnsafeTemporaryAllocation(of:capacity:_:)`](https://developer.apple.com/documentation/swift/withunsafetemporaryallocation(of:capacity:_:))
127+
> and [`withUnsafePointer(to:_:)`](https://developer.apple.com/documentation/swift/withunsafepointer(to:_:)-35wrn),
128+
> respectively, to ensure the pointers passed to `accessor` are large enough and
129+
> are well-aligned. If they are not large enough to contain values of the
130+
> appropriate types (per above), or if `hint` points to uninitialized or
131+
> incorrectly-typed memory, the result is undefined.
124132
125133
#### The context field
126134

Sources/Testing/ExitTests/ExitTest.swift

Lines changed: 40 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -52,16 +52,25 @@ public typealias ExitTest = __ExitTest
5252
@available(*, unavailable, message: "Exit tests are not available on this platform.")
5353
#endif
5454
public struct __ExitTest: Sendable, ~Copyable {
55-
/// The expected exit condition of the exit test.
56-
@_spi(ForToolsIntegrationOnly)
57-
public var expectedExitCondition: ExitCondition
55+
/// A type whose instances uniquely identify instances of `__ExitTest`.
56+
public struct ID: Sendable, Equatable, Codable {
57+
/// An underlying UUID (stored as two `UInt64` values to avoid relying on
58+
/// `UUID` from Foundation or any platform-specific interfaces.)
59+
private var _lo: UInt64
60+
private var _hi: UInt64
61+
62+
/// Initialize an instance of this type.
63+
///
64+
/// - Warning: This member is used to implement the `#expect(exitsWith:)`
65+
/// macro. Do not use it directly.
66+
public init(__uuid uuid: (UInt64, UInt64)) {
67+
self._lo = uuid.0
68+
self._hi = uuid.1
69+
}
70+
}
5871

59-
/// The source location of the exit test.
60-
///
61-
/// The source location is unique to each exit test and is consistent between
62-
/// processes, so it can be used to uniquely identify an exit test at runtime.
63-
@_spi(ForToolsIntegrationOnly)
64-
public var sourceLocation: SourceLocation
72+
/// A value that uniquely identifies this instance.
73+
public var id: ID
6574

6675
/// The body closure of the exit test.
6776
///
@@ -110,12 +119,10 @@ public struct __ExitTest: Sendable, ~Copyable {
110119
/// - Warning: This initializer is used to implement the `#expect(exitsWith:)`
111120
/// macro. Do not use it directly.
112121
public init(
113-
__expectedExitCondition expectedExitCondition: ExitCondition,
114-
sourceLocation: SourceLocation,
122+
__identifiedBy id: ID,
115123
body: @escaping @Sendable () async throws -> Void = {}
116124
) {
117-
self.expectedExitCondition = expectedExitCondition
118-
self.sourceLocation = sourceLocation
125+
self.id = id
119126
self.body = body
120127
}
121128
}
@@ -222,28 +229,24 @@ extension ExitTest: TestContent {
222229
}
223230

224231
typealias TestContentAccessorResult = Self
225-
typealias TestContentAccessorHint = SourceLocation
232+
typealias TestContentAccessorHint = ID
226233
}
227234

228235
@_spi(Experimental) @_spi(ForToolsIntegrationOnly)
229236
extension ExitTest {
230237
/// Find the exit test function at the given source location.
231238
///
232239
/// - Parameters:
233-
/// - sourceLocation: The source location of the exit test to find.
240+
/// - id: The unique identifier of the exit test to find.
234241
///
235242
/// - Returns: The specified exit test function, or `nil` if no such exit test
236243
/// could be found.
237-
public static func find(at sourceLocation: SourceLocation) -> Self? {
244+
public static func find(identifiedBy id: ExitTest.ID) -> Self? {
238245
var result: Self?
239246

240-
enumerateTestContent(withHint: sourceLocation) { _, exitTest, _, stop in
241-
if exitTest.sourceLocation == sourceLocation {
242-
result = ExitTest(
243-
__expectedExitCondition: exitTest.expectedExitCondition,
244-
sourceLocation: exitTest.sourceLocation,
245-
body: exitTest.body
246-
)
247+
enumerateTestContent(withHint: id) { _, exitTest, _, stop in
248+
if exitTest.id == id {
249+
result = ExitTest(__identifiedBy: id, body: exitTest.body)
247250
stop = true
248251
}
249252
}
@@ -252,14 +255,8 @@ extension ExitTest {
252255
// Call the legacy lookup function that discovers tests embedded in types.
253256
result = types(withNamesContaining: exitTestContainerTypeNameMagic).lazy
254257
.compactMap { $0 as? any __ExitTestContainer.Type }
255-
.first { $0.__sourceLocation == sourceLocation }
256-
.map { type in
257-
ExitTest(
258-
__expectedExitCondition: type.__expectedExitCondition,
259-
sourceLocation: type.__sourceLocation,
260-
body: type.__body
261-
)
262-
}
258+
.first { $0.__id == id }
259+
.map { ExitTest(__identifiedBy: $0.__id, body: $0.__body) }
263260
}
264261

265262
return result
@@ -272,6 +269,7 @@ extension ExitTest {
272269
/// a given status.
273270
///
274271
/// - Parameters:
272+
/// - exitTestID: The unique identifier of the exit test.
275273
/// - expectedExitCondition: The expected exit condition.
276274
/// - observedValues: An array of key paths representing results from within
277275
/// the exit test that should be observed and returned by this macro. The
@@ -290,6 +288,7 @@ extension ExitTest {
290288
/// `await #expect(exitsWith:) { }` invocations regardless of calling
291289
/// convention.
292290
func callExitTest(
291+
identifiedBy exitTestID: ExitTest.ID,
293292
exitsWith expectedExitCondition: ExitCondition,
294293
observing observedValues: [any PartialKeyPath<ExitTestArtifacts> & Sendable],
295294
expression: __Expression,
@@ -304,7 +303,7 @@ func callExitTest(
304303

305304
var result: ExitTestArtifacts
306305
do {
307-
var exitTest = ExitTest(__expectedExitCondition: expectedExitCondition, sourceLocation: sourceLocation)
306+
var exitTest = ExitTest(__identifiedBy: exitTestID)
308307
exitTest.observedValues = observedValues
309308
result = try await configuration.exitTestHandler(exitTest)
310309

@@ -424,23 +423,21 @@ extension ExitTest {
424423
/// `__swiftPMEntryPoint()` function. The effect of using it under other
425424
/// configurations is undefined.
426425
static func findInEnvironmentForEntryPoint() -> Self? {
427-
// Find the source location of the exit test to run, if any, in the
428-
// environment block.
429-
var sourceLocation: SourceLocation?
430-
if var sourceLocationString = Environment.variable(named: "SWT_EXPERIMENTAL_EXIT_TEST_SOURCE_LOCATION") {
431-
sourceLocation = try? sourceLocationString.withUTF8 { sourceLocationBuffer in
432-
let sourceLocationBuffer = UnsafeRawBufferPointer(sourceLocationBuffer)
433-
return try JSON.decode(SourceLocation.self, from: sourceLocationBuffer)
426+
// Find the ID of the exit test to run, if any, in the environment block.
427+
var id: __ExitTest.ID?
428+
if var idString = Environment.variable(named: "SWT_EXPERIMENTAL_EXIT_TEST_ID") {
429+
id = try? idString.withUTF8 { idBuffer in
430+
try JSON.decode(__ExitTest.ID.self, from: UnsafeRawBufferPointer(idBuffer))
434431
}
435432
}
436-
guard let sourceLocation else {
433+
guard let id else {
437434
return nil
438435
}
439436

440437
// If an exit test was found, inject back channel handling into its body.
441438
// External tools authors should set up their own back channel mechanisms
442439
// and ensure they're installed before calling ExitTest.callAsFunction().
443-
guard var result = find(at: sourceLocation) else {
440+
guard var result = find(identifiedBy: id) else {
444441
return nil
445442
}
446443

@@ -560,8 +557,8 @@ extension ExitTest {
560557

561558
// Insert a specific variable that tells the child process which exit test
562559
// to run.
563-
try JSON.withEncoding(of: exitTest.sourceLocation) { json in
564-
childEnvironment["SWT_EXPERIMENTAL_EXIT_TEST_SOURCE_LOCATION"] = String(decoding: json, as: UTF8.self)
560+
try JSON.withEncoding(of: exitTest.id) { json in
561+
childEnvironment["SWT_EXPERIMENTAL_EXIT_TEST_ID"] = String(decoding: json, as: UTF8.self)
565562
}
566563

567564
typealias ResultUpdater = @Sendable (inout ExitTestArtifacts) -> Void

Sources/Testing/Expectations/Expectation+Macro.swift

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -546,7 +546,7 @@ public macro require<R>(
546546
observing observedValues: [any PartialKeyPath<ExitTestArtifacts> & Sendable] = [],
547547
_ comment: @autoclosure () -> Comment? = nil,
548548
sourceLocation: SourceLocation = #_sourceLocation,
549-
performing expression: @convention(thin) () async throws -> Void
549+
performing expression: @escaping @Sendable @convention(thin) () async throws -> Void
550550
) -> ExitTestArtifacts? = #externalMacro(module: "TestingMacros", type: "ExitTestExpectMacro")
551551

552552
/// Check that an expression causes the process to terminate in a given fashion
@@ -658,5 +658,5 @@ public macro require<R>(
658658
observing observedValues: [any PartialKeyPath<ExitTestArtifacts> & Sendable] = [],
659659
_ comment: @autoclosure () -> Comment? = nil,
660660
sourceLocation: SourceLocation = #_sourceLocation,
661-
performing expression: @convention(thin) () async throws -> Void
661+
performing expression: @escaping @Sendable @convention(thin) () async throws -> Void
662662
) -> ExitTestArtifacts = #externalMacro(module: "TestingMacros", type: "ExitTestRequireMacro")

Sources/Testing/Expectations/ExpectationChecking+Macro.swift

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1147,6 +1147,7 @@ public func __checkClosureCall<R>(
11471147
/// `#require()` macros. Do not call it directly.
11481148
@_spi(Experimental)
11491149
public func __checkClosureCall(
1150+
identifiedBy exitTestID: __ExitTest.ID,
11501151
exitsWith expectedExitCondition: ExitCondition,
11511152
observing observedValues: [any PartialKeyPath<ExitTestArtifacts> & Sendable],
11521153
performing body: @convention(thin) () -> Void,
@@ -1157,6 +1158,7 @@ public func __checkClosureCall(
11571158
sourceLocation: SourceLocation
11581159
) async -> Result<ExitTestArtifacts?, any Error> {
11591160
await callExitTest(
1161+
identifiedBy: exitTestID,
11601162
exitsWith: expectedExitCondition,
11611163
observing: observedValues,
11621164
expression: expression,

Sources/Testing/Test+Discovery+Legacy.swift

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -32,11 +32,8 @@ let testContainerTypeNameMagic = "__🟠$test_container__"
3232
@_alwaysEmitConformanceMetadata
3333
@_spi(Experimental)
3434
public protocol __ExitTestContainer {
35-
/// The expected exit condition of the exit test.
36-
static var __expectedExitCondition: ExitCondition { get }
37-
38-
/// The source location of the exit test.
39-
static var __sourceLocation: SourceLocation { get }
35+
/// The unique identifier of the exit test.
36+
static var __id: __ExitTest.ID { get }
4037

4138
/// The body function of the exit test.
4239
static var __body: @Sendable () async throws -> Void { get }

Sources/TestingMacros/ConditionMacro.swift

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -416,6 +416,10 @@ extension ExitTestConditionMacro {
416416

417417
let bodyArgumentExpr = arguments[trailingClosureIndex].expression
418418

419+
// TODO: use UUID() here if we can link to Foundation
420+
let exitTestID = (UInt64.random(in: 0 ... .max), UInt64.random(in: 0 ... .max))
421+
let exitTestIDExpr: ExprSyntax = "Testing.__ExitTest.ID(__uuid: (\(literal: exitTestID.0), \(literal: exitTestID.1)))"
422+
419423
var decls = [DeclSyntax]()
420424

421425
// Implement the body of the exit test outside the enum we're declaring so
@@ -436,15 +440,12 @@ extension ExitTestConditionMacro {
436440
"""
437441
@available(*, deprecated, message: "This type is an implementation detail of the testing library. Do not use it directly.")
438442
enum \(enumName): Testing.__ExitTestContainer, Sendable {
439-
static var __sourceLocation: Testing.SourceLocation {
440-
\(createSourceLocationExpr(of: macro, context: context))
443+
static var __id: Testing.__ExitTest.ID {
444+
\(exitTestIDExpr)
441445
}
442446
static var __body: @Sendable () async throws -> Void {
443447
\(bodyThunkName)
444448
}
445-
static var __expectedExitCondition: Testing.ExitCondition {
446-
\(arguments[expectedExitConditionIndex].expression.trimmed)
447-
}
448449
}
449450
"""
450451
)
@@ -458,6 +459,16 @@ extension ExitTestConditionMacro {
458459
}
459460
)
460461

462+
// Insert the exit test's ID as the first argument. Note that this will
463+
// invalidate all indices into `arguments`!
464+
arguments.insert(
465+
Argument(
466+
label: "identifiedBy",
467+
expression: exitTestIDExpr
468+
),
469+
at: arguments.startIndex
470+
)
471+
461472
// Replace the exit test body (as an argument to the macro) with a stub
462473
// closure that hosts the type we created above.
463474
var macro = macro

Tests/TestingTests/ExitTestTests.swift

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -405,6 +405,17 @@ private import _TestingInternals
405405
#expect(result.standardOutputContent.isEmpty)
406406
#expect(result.standardErrorContent.contains("STANDARD ERROR".utf8.reversed()))
407407
}
408+
409+
@Test("Arguments to the macro are not captured during expansion (do not need to be literals/const)")
410+
func argumentsAreNotCapturedDuringMacroExpansion() async throws {
411+
let unrelatedSourceLocation = #_sourceLocation
412+
func nonConstExitCondition() async throws -> ExitCondition {
413+
.failure
414+
}
415+
await #expect(exitsWith: try await nonConstExitCondition(), sourceLocation: unrelatedSourceLocation) {
416+
fatalError()
417+
}
418+
}
408419
}
409420

410421
// MARK: - Fixtures

0 commit comments

Comments
 (0)