Skip to content

Commit 87ceeb4

Browse files
authored
Fix crash when loading test content records from the type metadata section. (#1015)
Looks like #880 and/or #1010 caused a regression: the compiler appears to be dead-code-stripping the classes we emit to contain test content records. This PR changes the design back to using a protocol so that the members we need are always covered by `@_alwaysEmitConformanceMetadata`. This makes `_TestDiscovery` a little harder to use with legacy lookup, but it's all experimental and eventually going to be removed anyway. Resolves rdar://146809312. ### 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 1e55aa7 commit 87ceeb4

6 files changed

+70
-103
lines changed

Sources/Testing/Test+Discovery+Legacy.swift

+22-20
Original file line numberDiff line numberDiff line change
@@ -11,35 +11,37 @@
1111
#if !SWT_NO_LEGACY_TEST_DISCOVERY
1212
@_spi(Experimental) @_spi(ForToolsIntegrationOnly) internal import _TestDiscovery
1313

14-
/// A shadow declaration of `_TestDiscovery.TestContentRecordContainer` that
15-
/// allows us to add public conformances to it without causing the
16-
/// `_TestDiscovery` module to appear in `Testing.private.swiftinterface`.
17-
///
18-
/// This protocol is not part of the public interface of the testing library.
19-
@_alwaysEmitConformanceMetadata
20-
protocol TestContentRecordContainer: _TestDiscovery.TestContentRecordContainer {}
21-
22-
/// An abstract base class describing a type that contains tests.
14+
/// A protocol base class describing a type that contains tests.
2315
///
2416
/// - Warning: This class is used to implement the `@Test` macro. Do not use it
2517
/// directly.
26-
open class __TestContentRecordContainer: TestContentRecordContainer {
27-
/// The corresponding test content record.
18+
@_alwaysEmitConformanceMetadata
19+
public protocol __TestContentRecordContainer {
20+
/// The test content record associated with this container.
2821
///
2922
/// - Warning: This property is used to implement the `@Test` macro. Do not
3023
/// use it directly.
31-
open nonisolated class var __testContentRecord: __TestContentRecord {
32-
(0, 0, nil, 0, 0)
33-
}
24+
nonisolated static var __testContentRecord: __TestContentRecord { get }
25+
}
26+
27+
extension DiscoverableAsTestContent where Self: ~Copyable {
28+
/// Get all test content of this type known to Swift and found in the current
29+
/// process using the legacy discovery mechanism.
30+
///
31+
/// - Returns: A sequence of instances of ``TestContentRecord``. Only test
32+
/// content records matching this ``TestContent`` type's requirements are
33+
/// included in the sequence.
34+
static func allTypeMetadataBasedTestContentRecords() -> AnySequence<TestContentRecord<Self>> {
35+
return allTypeMetadataBasedTestContentRecords { type, buffer in
36+
guard let type = type as? any __TestContentRecordContainer.Type else {
37+
return false
38+
}
3439

35-
static func storeTestContentRecord(to outTestContentRecord: UnsafeMutableRawPointer) -> Bool {
36-
outTestContentRecord.withMemoryRebound(to: __TestContentRecord.self, capacity: 1) { outTestContentRecord in
37-
outTestContentRecord.initialize(to: __testContentRecord)
40+
buffer.withMemoryRebound(to: __TestContentRecord.self) { buffer in
41+
buffer.baseAddress!.initialize(to: type.__testContentRecord)
42+
}
3843
return true
3944
}
4045
}
4146
}
42-
43-
@available(*, unavailable)
44-
extension __TestContentRecordContainer: Sendable {}
4547
#endif

Sources/TestingMacros/ConditionMacro.swift

+3-3
Original file line numberDiff line numberDiff line change
@@ -469,10 +469,10 @@ extension ExitTestConditionMacro {
469469
// Create another local type for legacy test discovery.
470470
var recordDecl: DeclSyntax?
471471
#if !SWT_NO_LEGACY_TEST_DISCOVERY
472-
let className = context.makeUniqueName("__🟡$")
472+
let legacyEnumName = context.makeUniqueName("__🟡$")
473473
recordDecl = """
474-
private final class \(className): Testing.__TestContentRecordContainer {
475-
override nonisolated class var __testContentRecord: Testing.__TestContentRecord {
474+
enum \(legacyEnumName): Testing.__TestContentRecordContainer {
475+
nonisolated static var __testContentRecord: Testing.__TestContentRecord {
476476
\(enumName).testContentRecord
477477
}
478478
}

Sources/TestingMacros/SuiteDeclarationMacro.swift

+3-3
Original file line numberDiff line numberDiff line change
@@ -166,12 +166,12 @@ public struct SuiteDeclarationMacro: MemberMacro, PeerMacro, Sendable {
166166

167167
#if !SWT_NO_LEGACY_TEST_DISCOVERY
168168
// Emit a type that contains a reference to the test content record.
169-
let className = context.makeUniqueName("__🟡$")
169+
let enumName = context.makeUniqueName("__🟡$")
170170
result.append(
171171
"""
172172
@available(*, deprecated, message: "This type is an implementation detail of the testing library. Do not use it directly.")
173-
final class \(className): Testing.__TestContentRecordContainer {
174-
override nonisolated class var __testContentRecord: Testing.__TestContentRecord {
173+
enum \(enumName): Testing.__TestContentRecordContainer {
174+
nonisolated static var __testContentRecord: Testing.__TestContentRecord {
175175
\(testContentRecordName)
176176
}
177177
}

Sources/TestingMacros/TestDeclarationMacro.swift

+3-3
Original file line numberDiff line numberDiff line change
@@ -491,12 +491,12 @@ public struct TestDeclarationMacro: PeerMacro, Sendable {
491491

492492
#if !SWT_NO_LEGACY_TEST_DISCOVERY
493493
// Emit a type that contains a reference to the test content record.
494-
let className = context.makeUniqueName(thunking: functionDecl, withPrefix: "__🟡$")
494+
let enumName = context.makeUniqueName(thunking: functionDecl, withPrefix: "__🟡$")
495495
result.append(
496496
"""
497497
@available(*, deprecated, message: "This type is an implementation detail of the testing library. Do not use it directly.")
498-
final class \(className): Testing.__TestContentRecordContainer {
499-
override nonisolated class var __testContentRecord: Testing.__TestContentRecord {
498+
enum \(enumName): Testing.__TestContentRecordContainer {
499+
nonisolated static var __testContentRecord: Testing.__TestContentRecord {
500500
\(testContentRecordName)
501501
}
502502
}

Sources/_TestDiscovery/DiscoverableAsTestContent.swift

+13
Original file line numberDiff line numberDiff line change
@@ -39,4 +39,17 @@ public protocol DiscoverableAsTestContent: Sendable, ~Copyable {
3939
/// By default, this type equals `Never`, indicating that this type of test
4040
/// content does not support hinting during discovery.
4141
associatedtype TestContentAccessorHint = Never
42+
43+
#if !SWT_NO_LEGACY_TEST_DISCOVERY
44+
/// A string present in the names of types containing test content records
45+
/// associated with this type.
46+
@available(swift, deprecated: 100000.0, message: "Do not adopt this functionality in new code. It will be removed in a future release.")
47+
static var _testContentTypeNameHint: String { get }
48+
#endif
49+
}
50+
51+
extension DiscoverableAsTestContent where Self: ~Copyable {
52+
public static var _testContentTypeNameHint: String {
53+
"__🟡$"
54+
}
4255
}

Sources/_TestDiscovery/TestContentRecord.swift

+26-74
Original file line numberDiff line numberDiff line change
@@ -249,80 +249,17 @@ extension DiscoverableAsTestContent where Self: ~Copyable {
249249

250250
private import _TestingInternals
251251

252-
/// A protocol describing a type, emitted at compile time or macro expansion
253-
/// time, that represents a single test content record.
254-
///
255-
/// Use this protocol to make discoverable any test content records contained in
256-
/// the type metadata section (the "legacy discovery mechanism"). For example,
257-
/// if you have creasted a test content record named `myRecord` and your test
258-
/// content record typealias is named `MyRecordType`:
259-
///
260-
/// ```swift
261-
/// private enum MyRecordContainer: TestContentRecordContainer {
262-
/// nonisolated static func storeTestContentRecord(to outTestContentRecord: UnsafeMutableRawPointer) -> Bool {
263-
/// outTestContentRecord.initializeMemory(as: MyRecordType.self, to: myRecord)
264-
/// return true
265-
/// }
266-
/// }
267-
/// ```
268-
///
269-
/// Then, at discovery time, call ``DiscoverableAsTestContent/allTypeMetadataBasedTestContentRecords()``
270-
/// to look up `myRecord`.
271-
///
272-
/// Types that represent test content and that should be discoverable at runtime
273-
/// should not conform to this protocol. Instead, they should conform to
274-
/// ``DiscoverableAsTestContent``.
275-
@_spi(Experimental) @_spi(ForToolsIntegrationOnly)
276-
@_alwaysEmitConformanceMetadata
277-
@available(swift, deprecated: 100000.0, message: "Do not adopt this functionality in new code. It will be removed in a future release.")
278-
public protocol TestContentRecordContainer {
279-
/// Store this container's corresponding test content record to memory.
280-
///
281-
/// - Parameters:
282-
/// - outTestContentRecord: A pointer to uninitialized memory large enough
283-
/// to hold a test content record. The memory is untyped so that client
284-
/// code can use a custom definition of the test content record tuple
285-
/// type.
286-
///
287-
/// - Returns: Whether or not `outTestContentRecord` was initialized. If this
288-
/// function returns `true`, the caller is responsible for deinitializing
289-
/// said memory after it is done using it.
290-
nonisolated static func storeTestContentRecord(to outTestContentRecord: UnsafeMutableRawPointer) -> Bool
291-
}
292-
293252
extension DiscoverableAsTestContent where Self: ~Copyable {
294-
/// Make a test content record of this type from the given test content record
295-
/// container type if it matches this type's requirements.
296-
///
297-
/// - Parameters:
298-
/// - containerType: The test content record container type.
299-
/// - sb: The section bounds containing `containerType` and, thus, the test
300-
/// content record.
301-
///
302-
/// - Returns: A new test content record value, or `nil` if `containerType`
303-
/// failed to store a record or if the record's kind did not match this
304-
/// type's ``testContentKind`` property.
305-
private static func _makeTestContentRecord(from containerType: (some TestContentRecordContainer).Type, in sb: SectionBounds) -> TestContentRecord<Self>? {
306-
withUnsafeTemporaryAllocation(of: _TestContentRecord.self, capacity: 1) { buffer in
307-
// Load the record from the container type.
308-
guard containerType.storeTestContentRecord(to: buffer.baseAddress!) else {
309-
return nil
310-
}
311-
let record = buffer.baseAddress!.move()
312-
313-
// Make sure that the record's kind matches.
314-
guard record.kind == Self.testContentKind else {
315-
return nil
316-
}
317-
318-
// Construct the TestContentRecord instance from the record.
319-
return TestContentRecord(imageAddress: sb.imageAddress, record: record)
320-
}
321-
}
322-
323253
/// Get all test content of this type known to Swift and found in the current
324254
/// process using the legacy discovery mechanism.
325255
///
256+
/// - Parameters:
257+
/// - baseType: The type which all discovered container types must
258+
/// conform to or subclass.
259+
/// - loader: A function that is called once per type conforming to or
260+
/// subclassing `baseType`. This function should load the corresponding
261+
/// test content record into the buffer passed to it.
262+
///
326263
/// - Returns: A sequence of instances of ``TestContentRecord``. Only test
327264
/// content records matching this ``TestContent`` type's requirements are
328265
/// included in the sequence.
@@ -332,15 +269,30 @@ extension DiscoverableAsTestContent where Self: ~Copyable {
332269
/// opaque type due to a compiler crash. ([143080508](rdar://143080508))
333270
/// }
334271
@available(swift, deprecated: 100000.0, message: "Do not adopt this functionality in new code. It will be removed in a future release.")
335-
public static func allTypeMetadataBasedTestContentRecords() -> AnySequence<TestContentRecord<Self>> {
272+
public static func allTypeMetadataBasedTestContentRecords(
273+
loadingWith loader: @escaping @Sendable (Any.Type, UnsafeMutableRawBufferPointer) -> Bool
274+
) -> AnySequence<TestContentRecord<Self>> {
336275
validateMemoryLayout()
337276

277+
let typeNameHint = _testContentTypeNameHint
278+
let kind = testContentKind
279+
let loader: @Sendable (Any.Type) -> _TestContentRecord? = { type in
280+
withUnsafeTemporaryAllocation(of: _TestContentRecord.self, capacity: 1) { buffer in
281+
// Load the record from the container type.
282+
guard loader(type, .init(buffer)) else {
283+
return nil
284+
}
285+
return buffer.baseAddress!.move()
286+
}
287+
}
288+
338289
let result = SectionBounds.all(.typeMetadata).lazy.flatMap { sb in
339290
stride(from: sb.buffer.baseAddress!, to: sb.buffer.baseAddress! + sb.buffer.count, by: SWTTypeMetadataRecordByteCount).lazy
340-
.compactMap { swt_getType(fromTypeMetadataRecord: $0, ifNameContains: "__🟡$") }
291+
.compactMap { swt_getType(fromTypeMetadataRecord: $0, ifNameContains: typeNameHint) }
341292
.map { unsafeBitCast($0, to: Any.Type.self) }
342-
.compactMap { $0 as? any TestContentRecordContainer.Type }
343-
.compactMap { _makeTestContentRecord(from: $0, in: sb) }
293+
.compactMap(loader)
294+
.filter { $0.kind == kind }
295+
.map { TestContentRecord<Self>(imageAddress: sb.imageAddress, record: $0) }
344296
}
345297
return AnySequence(result)
346298
}

0 commit comments

Comments
 (0)