From dfe70181aee7f4b2dfe67ac7a4ee75f947faf99a Mon Sep 17 00:00:00 2001 From: Jonathan Grynspan Date: Fri, 27 Sep 2024 11:54:11 -0400 Subject: [PATCH] Inherit isolation in `#expect(exitsWith:)`. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This PR ensures the body of _the implementation of_ `#expect(exitsWith:)` inherits isolation from the caller. The actual body closure passed to the macro cannot inherit isolation as it runs in a separate process, but the glue code we emit needs to inherit so that if the caller is actor-isolated, the code compiles cleanly. Without this change, the following test fails to compile: ```swift @MainActor @Test func f() async { await #expect(exitsWith: .failure) { /* ... */ } // 🛑 ^ ^ ^ sending main actor-isolated value of type '() -> [Comment]' with later accesses to nonisolated context risks causing data races } ``` --- Sources/Testing/ExitTests/ExitTest.swift | 3 +- .../ExpectationChecking+Macro.swift | 4 +- Sources/TestingMacros/ConditionMacro.swift | 56 ++++++++++++------- Tests/TestingTests/ExitTestTests.swift | 14 +++++ 4 files changed, 53 insertions(+), 24 deletions(-) diff --git a/Sources/Testing/ExitTests/ExitTest.swift b/Sources/Testing/ExitTests/ExitTest.swift index 81352ec15..7db151125 100644 --- a/Sources/Testing/ExitTests/ExitTest.swift +++ b/Sources/Testing/ExitTests/ExitTest.swift @@ -153,6 +153,7 @@ extension ExitTest { /// - isRequired: Whether or not the expectation is required. The value of /// this argument does not affect whether or not an error is thrown on /// failure. +/// - isolation: The actor to which the exit test is isolated, if any. /// - sourceLocation: The source location of the expectation. /// /// This function contains the common implementation for all @@ -160,10 +161,10 @@ extension ExitTest { /// convention. func callExitTest( exitsWith expectedExitCondition: ExitCondition, - performing _: @escaping @Sendable () async throws -> Void, expression: __Expression, comments: @autoclosure () -> [Comment], isRequired: Bool, + isolation: isolated (any Actor)? = #isolation, sourceLocation: SourceLocation ) async -> Result { guard let configuration = Configuration.current ?? Configuration.all.first else { diff --git a/Sources/Testing/Expectations/ExpectationChecking+Macro.swift b/Sources/Testing/Expectations/ExpectationChecking+Macro.swift index 60556a39c..0c65dee1a 100644 --- a/Sources/Testing/Expectations/ExpectationChecking+Macro.swift +++ b/Sources/Testing/Expectations/ExpectationChecking+Macro.swift @@ -1142,15 +1142,15 @@ public func __checkClosureCall( @_spi(Experimental) public func __checkClosureCall( exitsWith expectedExitCondition: ExitCondition, - performing body: @convention(thin) () async throws -> Void, + performing body: @convention(thin) () -> Void, expression: __Expression, comments: @autoclosure () -> [Comment], isRequired: Bool, + isolation: isolated (any Actor)? = #isolation, sourceLocation: SourceLocation ) async -> Result { await callExitTest( exitsWith: expectedExitCondition, - performing: { try await body() }, expression: expression, comments: comments(), isRequired: isRequired, diff --git a/Sources/TestingMacros/ConditionMacro.swift b/Sources/TestingMacros/ConditionMacro.swift index e761dcd00..7bacf0a31 100644 --- a/Sources/TestingMacros/ConditionMacro.swift +++ b/Sources/TestingMacros/ConditionMacro.swift @@ -376,32 +376,46 @@ extension ExitTestConditionMacro { let bodyArgumentExpr = arguments[trailingClosureIndex].expression + var decls = [DeclSyntax]() + + // Implement the body of the exit test outside the enum we're declaring so + // that `Self` resolves to the type containing the exit test, not the enum. + let bodyThunkName = context.makeUniqueName("") + decls.append( + """ + @Sendable func \(bodyThunkName)() async throws -> Void { + return try await Testing.__requiringTry(Testing.__requiringAwait(\(bodyArgumentExpr.trimmed)))() + } + """ + ) + // Create a local type that can be discovered at runtime and which contains // the exit test body. let enumName = context.makeUniqueName("__🟠$exit_test_body__") - let enumDecl: DeclSyntax = """ - @available(*, deprecated, message: "This type is an implementation detail of the testing library. Do not use it directly.") - enum \(enumName): Testing.__ExitTestContainer { - static var __sourceLocation: Testing.SourceLocation { - \(createSourceLocationExpr(of: macro, context: context)) - } - static var __body: @Sendable () async throws -> Void { - \(bodyArgumentExpr.trimmed) + decls.append( + """ + @available(*, deprecated, message: "This type is an implementation detail of the testing library. Do not use it directly.") + enum \(enumName): Testing.__ExitTestContainer, Sendable { + static var __sourceLocation: Testing.SourceLocation { + \(createSourceLocationExpr(of: macro, context: context)) + } + static var __body: @Sendable () async throws -> Void { + \(bodyThunkName) + } + static var __expectedExitCondition: Testing.ExitCondition { + \(arguments[expectedExitConditionIndex].expression.trimmed) + } } - static var __expectedExitCondition: Testing.ExitCondition { - \(arguments[expectedExitConditionIndex].expression.trimmed) + """ + ) + + arguments[trailingClosureIndex].expression = ExprSyntax( + ClosureExprSyntax { + for decl in decls { + CodeBlockItemSyntax(item: .decl(decl)) + } } - } - """ - - // Explicitly include a closure signature to work around a compiler bug - // type-checking thin throwing functions after macro expansion. - // SEE: rdar://133979438 - arguments[trailingClosureIndex].expression = """ - { () async throws in - \(enumDecl) - } - """ + ) // Replace the exit test body (as an argument to the macro) with a stub // closure that hosts the type we created above. diff --git a/Tests/TestingTests/ExitTestTests.swift b/Tests/TestingTests/ExitTestTests.swift index 7dd9b7d06..60f708957 100644 --- a/Tests/TestingTests/ExitTestTests.swift +++ b/Tests/TestingTests/ExitTestTests.swift @@ -280,6 +280,20 @@ private import _TestingInternals #expect(ExitCondition.signal(SIGTERM) !== .signal(SIGINT)) #endif } + + @MainActor static func someMainActorFunction() { + MainActor.assertIsolated() + } + + @Test("Exit test can be main-actor-isolated") + @MainActor + func mainActorIsolation() async { + await #expect(exitsWith: .success) { + await Self.someMainActorFunction() + _ = 0 + exit(EXIT_SUCCESS) + } + } } // MARK: - Fixtures