Skip to content

Conversation

grynspan
Copy link
Contributor

@grynspan grynspan commented Mar 12, 2025

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:

  • Code and documentation should follow the style of the Style Guide.
  • If public symbols are renamed or modified, DocC references should be updated.

…ction.

Looks like # 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.
@grynspan grynspan added bug 🪲 Something isn't working workaround Workaround for an issue in another component (may need to revert later) labels Mar 12, 2025
@grynspan grynspan added this to the Swift 6.2 milestone Mar 12, 2025
@grynspan grynspan self-assigned this Mar 12, 2025
@grynspan
Copy link
Contributor Author

@swift-ci test

@grynspan
Copy link
Contributor Author

Windows CI is down right now. Self-reviewing due to potential impact on the toolchain.

@grynspan grynspan added the self-reviewed PR was self-reviewed by a code owner label Mar 12, 2025
@grynspan grynspan merged commit 87ceeb4 into main Mar 12, 2025
2 of 3 checks passed
@grynspan grynspan deleted the jgrynspan/146809312-crash-loading-test-content branch March 12, 2025 00:56
protocol TestContentRecordContainer: _TestDiscovery.TestContentRecordContainer {}

/// An abstract base class describing a type that contains tests.
/// A protocol base class describing a type that contains tests.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typo here fixed on main already.

/// An abstract base class describing a type that contains tests.
/// A protocol base class describing a type that contains tests.
///
/// - Warning: This class is used to implement the `@Test` macro. Do not use it
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typo here fixed on main already.

@grynspan grynspan added the discovery 🔎 test content discovery label Mar 12, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🪲 Something isn't working discovery 🔎 test content discovery self-reviewed PR was self-reviewed by a code owner workaround Workaround for an issue in another component (may need to revert later)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant