From e69878a2a2b1d747844f21eb2a028cc422b63b57 Mon Sep 17 00:00:00 2001 From: Jonathan Grynspan Date: Mon, 20 Jan 2025 12:07:47 -0500 Subject: [PATCH 1/5] Make all test content types directly conform to `TestContent`. This PR eliminates the `TestContentAccessorResult` associated type from the (currently internal, potentially eventually API) `TestContent` protocol. This associated type needed to be `~Copyable` so `ExitTest` could be used with it, but that appears to pose some _problems_ for the compiler (rdar://143049814&143080508). Instead, we remove the associated type and just say "the test content record is the type that conforms to `TestContent`". `ExitTest` is happy with this, but `Test`'s produced type is a non-nominal function type, so we wrap that function in a small private type with identical layout and have that type conform. The ultimate purpose of this PR is to get us a bit closer to turning `TestContent` into a public or tools-SPI protocol that other components can use for test discovery. --- Sources/Testing/Discovery.swift | 24 ++++++--------------- Sources/Testing/ExitTests/ExitTest.swift | 1 - Sources/Testing/Test+Discovery.swift | 21 +++++++++++++++--- Tests/TestingTests/MiscellaneousTests.swift | 13 +++++------ 4 files changed, 31 insertions(+), 28 deletions(-) diff --git a/Sources/Testing/Discovery.swift b/Sources/Testing/Discovery.swift index 2ad44b324..a5f64b994 100644 --- a/Sources/Testing/Discovery.swift +++ b/Sources/Testing/Discovery.swift @@ -48,14 +48,6 @@ protocol TestContent: ~Copyable { /// `ABI/TestContent.md` for a list of values and corresponding types. static var testContentKind: UInt32 { get } - /// The type of value returned by the test content accessor for this type. - /// - /// This type may or may not equal `Self` depending on the type's compile-time - /// and runtime requirements. If it does not equal `Self`, it should equal a - /// type whose instances can be converted to instances of `Self` (e.g. by - /// calling them if they are functions.) - associatedtype TestContentAccessorResult: ~Copyable - /// A type of "hint" passed to ``discover(withHint:)`` to help the testing /// library find the correct result. /// @@ -75,7 +67,7 @@ protocol TestContent: ~Copyable { /// This type is not part of the public interface of the testing library. In the /// future, we could make it public if we want to support runtime discovery of /// test content by second- or third-party code. -struct TestContentRecord: Sendable where T: ~Copyable { +struct TestContentRecord: Sendable where T: TestContent & ~Copyable { /// The base address of the image containing this instance, if known. /// /// On platforms such as WASI that statically link to the testing library, the @@ -93,11 +85,7 @@ struct TestContentRecord: Sendable where T: ~Copyable { self.imageAddress = imageAddress self._record = record } -} -// This `T: TestContent` constraint is in an extension in order to work around a -// compiler crash. SEE: rdar://143049814 -extension TestContentRecord where T: TestContent & ~Copyable { /// The context value for this test content record. var context: UInt { _record.context @@ -109,18 +97,18 @@ extension TestContentRecord where T: TestContent & ~Copyable { /// - hint: An optional hint value. If not `nil`, this value is passed to /// the accessor function of the underlying test content record. /// - /// - Returns: An instance of the associated ``TestContentAccessorResult`` - /// type, or `nil` if the underlying test content record did not match - /// `hint` or otherwise did not produce a value. + /// - Returns: An instance of the test content type `T`, or `nil` if the + /// underlying test content record did not match `hint` or otherwise did not + /// produce a value. /// /// If this function is called more than once on the same instance, a new /// value is created on each call. - func load(withHint hint: T.TestContentAccessorHint? = nil) -> T.TestContentAccessorResult? { + func load(withHint hint: T.TestContentAccessorHint? = nil) -> T? { guard let accessor = _record.accessor else { return nil } - return withUnsafeTemporaryAllocation(of: T.TestContentAccessorResult.self, capacity: 1) { buffer in + return withUnsafeTemporaryAllocation(of: T.self, capacity: 1) { buffer in let initialized = if let hint { withUnsafePointer(to: hint) { hint in accessor(buffer.baseAddress!, hint) diff --git a/Sources/Testing/ExitTests/ExitTest.swift b/Sources/Testing/ExitTests/ExitTest.swift index a3a92c036..f6ea88d22 100644 --- a/Sources/Testing/ExitTests/ExitTest.swift +++ b/Sources/Testing/ExitTests/ExitTest.swift @@ -228,7 +228,6 @@ extension ExitTest: TestContent { 0x65786974 } - typealias TestContentAccessorResult = Self typealias TestContentAccessorHint = ID } diff --git a/Sources/Testing/Test+Discovery.swift b/Sources/Testing/Test+Discovery.swift index 939e85947..068ce23a1 100644 --- a/Sources/Testing/Test+Discovery.swift +++ b/Sources/Testing/Test+Discovery.swift @@ -10,13 +10,28 @@ private import _TestingInternals -extension Test: TestContent { +/// A type that encapsulates test content records that produce instances of +/// ``Test``. +/// +/// This type is necessary because such test content records produce an indirect +/// `async` accessor function rather than directly producing instances of +/// ``Test``, but functions are non-nominal types and cannot directly conform to +/// protocols. +/// +/// - Note: This helper type must have the exact in-memory layout of the `async` +/// accessor function. Do not add any additional stored properties. The layout +/// of this type is _de facto_ [guaranteed](https://github.com/swiftlang/swift/blob/main/docs/ABI/TypeLayout.rst) +/// by the Swift ABI. +/* @frozen */ private struct _TestRecord: TestContent { static var testContentKind: UInt32 { 0x74657374 } - typealias TestContentAccessorResult = @Sendable () async -> Self + /// This instance's actual (asynchronous) accessor function. + var asyncAccessor: @Sendable () async -> Test +} +extension Test { /// All available ``Test`` instances in the process, according to the runtime. /// /// The order of values in this sequence is unspecified. @@ -43,7 +58,7 @@ extension Test: TestContent { // Walk all test content and gather generator functions, then call them in // a task group and collate their results. if useNewMode { - let generators = Self.allTestContentRecords().lazy.compactMap { $0.load() } + let generators = _TestRecord.allTestContentRecords().lazy.compactMap { $0.load()?.asyncAccessor } await withTaskGroup(of: Self.self) { taskGroup in for generator in generators { taskGroup.addTask(operation: generator) diff --git a/Tests/TestingTests/MiscellaneousTests.swift b/Tests/TestingTests/MiscellaneousTests.swift index 9cc3b7910..41956cded 100644 --- a/Tests/TestingTests/MiscellaneousTests.swift +++ b/Tests/TestingTests/MiscellaneousTests.swift @@ -583,7 +583,8 @@ struct MiscellaneousTests { #if !SWT_NO_DYNAMIC_LINKING && hasFeature(SymbolLinkageMarkers) struct DiscoverableTestContent: TestContent { typealias TestContentAccessorHint = UInt32 - typealias TestContentAccessorResult = UInt32 + + var value: UInt32 static var testContentKind: UInt32 { record.kind @@ -593,7 +594,7 @@ struct MiscellaneousTests { 0x01020304 } - static var expectedResult: TestContentAccessorResult { + static var expectedValue: UInt32 { 0xCAFEF00D } @@ -618,7 +619,7 @@ struct MiscellaneousTests { if let hint, hint.load(as: TestContentAccessorHint.self) != expectedHint { return false } - _ = outValue.initializeMemory(as: TestContentAccessorResult.self, to: expectedResult) + _ = outValue.initializeMemory(as: Self.self, to: .init(value: expectedValue)) return true }, UInt(truncatingIfNeeded: UInt64(0x0204060801030507)), @@ -639,21 +640,21 @@ struct MiscellaneousTests { // Can find a single test record #expect(allRecords.contains { record in - record.load() == DiscoverableTestContent.expectedResult + record.load()?.value == DiscoverableTestContent.expectedValue && record.context == DiscoverableTestContent.expectedContext }) // Can find a test record with matching hint #expect(allRecords.contains { record in let hint = DiscoverableTestContent.expectedHint - return record.load(withHint: hint) == DiscoverableTestContent.expectedResult + return record.load(withHint: hint)?.value == DiscoverableTestContent.expectedValue && record.context == DiscoverableTestContent.expectedContext }) // Doesn't find a test record with a mismatched hint #expect(!allRecords.contains { record in let hint = ~DiscoverableTestContent.expectedHint - return record.load(withHint: hint) == DiscoverableTestContent.expectedResult + return record.load(withHint: hint)?.value == DiscoverableTestContent.expectedValue && record.context == DiscoverableTestContent.expectedContext }) } From 293cc5c1f21d3ffd8efe5ac37d612f7f27961f6a Mon Sep 17 00:00:00 2001 From: Jonathan Grynspan Date: Mon, 20 Jan 2025 12:29:28 -0500 Subject: [PATCH 2/5] Fix typo in a nearby comment --- Sources/Testing/Discovery.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Sources/Testing/Discovery.swift b/Sources/Testing/Discovery.swift index a5f64b994..647da522f 100644 --- a/Sources/Testing/Discovery.swift +++ b/Sources/Testing/Discovery.swift @@ -48,7 +48,7 @@ protocol TestContent: ~Copyable { /// `ABI/TestContent.md` for a list of values and corresponding types. static var testContentKind: UInt32 { get } - /// A type of "hint" passed to ``discover(withHint:)`` to help the testing + /// A type of "hint" passed to ``allTestContentRecords()`` to help the testing /// library find the correct result. /// /// By default, this type equals `Never`, indicating that this type of test From f97a168d08f8533b07fbc8155e59585902cd8855 Mon Sep 17 00:00:00 2001 From: Jonathan Grynspan Date: Mon, 20 Jan 2025 12:33:06 -0500 Subject: [PATCH 3/5] Fix incorrect return type of _sectionBounds on Windows --- Sources/Testing/Discovery+Platform.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Sources/Testing/Discovery+Platform.swift b/Sources/Testing/Discovery+Platform.swift index 9c352bb41..a42800577 100644 --- a/Sources/Testing/Discovery+Platform.swift +++ b/Sources/Testing/Discovery+Platform.swift @@ -230,7 +230,7 @@ private func _findSection(named sectionName: String, in hModule: HMODULE) -> Sec /// /// - Returns: An array of structures describing the bounds of all known test /// content sections in the current process. -private func _sectionBounds(_ kind: SectionBounds.Kind) -> [SectionBounds] { +private func _sectionBounds(_ kind: SectionBounds.Kind) -> some Sequence { let sectionName = switch kind { case .testContent: ".sw5test" From a15fca2b659524a45030ecffd864fc36a5bec2ac Mon Sep 17 00:00:00 2001 From: Jonathan Grynspan Date: Tue, 21 Jan 2025 09:31:04 -0500 Subject: [PATCH 4/5] Move wrapper type into Test, make it an enum for ABIness, update documentation to tell devs not to use our typealias because it'll almost certainly break them in the future --- Documentation/ABI/TestContent.md | 14 +++++++++ Sources/Testing/Test+Discovery.swift | 47 +++++++++++++++------------- 2 files changed, 40 insertions(+), 21 deletions(-) diff --git a/Documentation/ABI/TestContent.md b/Documentation/ABI/TestContent.md index c1a90aff1..4f1346b95 100644 --- a/Documentation/ABI/TestContent.md +++ b/Documentation/ABI/TestContent.md @@ -63,6 +63,17 @@ struct SWTTestContentRecord { }; ``` +Do not use the `__TestContentRecord` typealias defined in the testing library. +This type exists to support the testing library's macros and may change in the +future (e.g. to accomodate a generic argument or to make use of one of the +reserved fields.) + +Instead, define your own copy of this type where needed—you can copy the +definition above _verbatim_. If your test record type's `context` field (as +described below) is a pointer type, make sure to change its type in your version +of `TestContentRecord` accordingly so that, on systems with pointer +authentication enabled, the pointer is correctly resigned at load time. + ### Record content #### The kind field @@ -79,6 +90,9 @@ record's kind is a 32-bit unsigned value. The following kinds are defined: +If a test content record's `kind` field equals `0x00000000`, the values of all +other fields in that record are undefined. + #### The accessor field The function `accessor` is a C function. When called, it initializes the memory diff --git a/Sources/Testing/Test+Discovery.swift b/Sources/Testing/Test+Discovery.swift index 068ce23a1..015a0b1c8 100644 --- a/Sources/Testing/Test+Discovery.swift +++ b/Sources/Testing/Test+Discovery.swift @@ -10,28 +10,28 @@ private import _TestingInternals -/// A type that encapsulates test content records that produce instances of -/// ``Test``. -/// -/// This type is necessary because such test content records produce an indirect -/// `async` accessor function rather than directly producing instances of -/// ``Test``, but functions are non-nominal types and cannot directly conform to -/// protocols. -/// -/// - Note: This helper type must have the exact in-memory layout of the `async` -/// accessor function. Do not add any additional stored properties. The layout -/// of this type is _de facto_ [guaranteed](https://github.com/swiftlang/swift/blob/main/docs/ABI/TypeLayout.rst) -/// by the Swift ABI. -/* @frozen */ private struct _TestRecord: TestContent { - static var testContentKind: UInt32 { - 0x74657374 - } +extension Test { + /// A type that encapsulates test content records that produce instances of + /// ``Test``. + /// + /// This type is necessary because such test content records produce an + /// indirect `async` accessor function rather than directly producing + /// instances of ``Test``, but functions are non-nominal types and cannot + /// directly conform to protocols. + /// + /// - Note: This helper type must have the exact in-memory layout of the + /// `async` accessor function. Do not add any additional cases or associated + /// values. The layout of this type is [guaranteed](https://github.com/swiftlang/swift/blob/main/docs/ABI/TypeLayout.rst#fragile-enum-layout) + /// by the Swift ABI. + /* @frozen */ private enum _Record: TestContent { + static var testContentKind: UInt32 { + 0x74657374 + } - /// This instance's actual (asynchronous) accessor function. - var asyncAccessor: @Sendable () async -> Test -} + /// The actual (asynchronous) accessor function. + case generator(@Sendable () async -> Test) + } -extension Test { /// All available ``Test`` instances in the process, according to the runtime. /// /// The order of values in this sequence is unspecified. @@ -58,7 +58,12 @@ extension Test { // Walk all test content and gather generator functions, then call them in // a task group and collate their results. if useNewMode { - let generators = _TestRecord.allTestContentRecords().lazy.compactMap { $0.load()?.asyncAccessor } + let generators = _Record.allTestContentRecords().lazy.compactMap { record in + if case let .generator(generator) = record.load() { + return generator + } + return nil // currently unreachable, but not provably so + } await withTaskGroup(of: Self.self) { taskGroup in for generator in generators { taskGroup.addTask(operation: generator) From 266405c3cb524fbc853c37cd9dbdf4838b7b3651 Mon Sep 17 00:00:00 2001 From: Jonathan Grynspan Date: Tue, 21 Jan 2025 10:01:17 -0500 Subject: [PATCH 5/5] testDiscovery() needs to enumerate only once because it's a sequence, not a collection, and may not be enumerable twice--cast to array for the bulk of the test --- Tests/TestingTests/MiscellaneousTests.swift | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/Tests/TestingTests/MiscellaneousTests.swift b/Tests/TestingTests/MiscellaneousTests.swift index 41956cded..a172f7d5a 100644 --- a/Tests/TestingTests/MiscellaneousTests.swift +++ b/Tests/TestingTests/MiscellaneousTests.swift @@ -629,14 +629,15 @@ struct MiscellaneousTests { @Test func testDiscovery() async { // Check the type of the test record sequence (it should be lazy.) - let allRecords = DiscoverableTestContent.allTestContentRecords() + let allRecordsSeq = DiscoverableTestContent.allTestContentRecords() #if SWT_FIXED_143080508 - #expect(allRecords is any LazySequenceProtocol) - #expect(!(allRecords is [TestContentRecord])) + #expect(allRecordsSeq is any LazySequenceProtocol) + #expect(!(allRecordsSeq is [TestContentRecord])) #endif // It should have exactly one matching record (because we only emitted one.) - #expect(Array(allRecords).count == 1) + let allRecords = Array(allRecordsSeq) + #expect(allRecords.count == 1) // Can find a single test record #expect(allRecords.contains { record in