Skip to content

Conversation

stmontgomery
Copy link
Contributor

Acknowledge unsafe API usages from various testing library macros such as @Test, @Suite, and #expect(processExitsWith:) which are revealed in modules which enable the new opt-in strict memory safety feature in Swift 6.2.

Motivation:

This fix allows clients of the testing library to enable SE-0458: Opt-in Strict Memory Safety Checking if they wish and avoid diagnostics from the testing library macros in their modules. These warnings generally looked like this, before this fix:

⚠️ @__swiftmacro_22MemorySafeTestingTests19exampleTestFunction33_F2EA1AA3013574E5644E5A4339F05086LL0F0fMp_.swift:23:14: warning: expression uses unsafe constructs but is not marked with 'unsafe'
  Testing.Test.__store($s22MemorySafeTestingTests19exampleTestFunction33_F2EA1AA3013574E5644E5A4339F05086LL0F0fMp_25generator1e3470c498e8fe35fMu_, into: outValue, asTypeAt: type)
             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Modifications:

  • Add test file to reproduce the new diagnostics. It's in a new module which opts-in to strict memory safety, and marked as a dependency of TestingTests.
  • Add unsafe keyword in the appropriate places in our macros to acknowledge the existing unsafe usages.

Checklist:

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

Fixes: rdar://151238560

@stmontgomery stmontgomery added this to the Swift 6.x milestone Jun 2, 2025
@stmontgomery stmontgomery self-assigned this Jun 2, 2025
@stmontgomery stmontgomery added enhancement New feature or request exit-tests ☠️ Work related to exit tests macros 🔭 Related to Swift macros such as @Test or #expect discovery 🔎 test content discovery labels Jun 2, 2025
@stmontgomery
Copy link
Contributor Author

@swift-ci please test

@stmontgomery
Copy link
Contributor Author

This fails to build when using a pre-6.2 toolchain because those toolchains don't recognize the unsafe keyword. (Unlike the @unsafe attribute which 6.1 toolchains are aware of.) I'll need to figure out some way to conditionalize this — hopefully without resorting to #if checks in the macro expansion code itself.

@stmontgomery stmontgomery marked this pull request as draft June 2, 2025 14:37
@grynspan
Copy link
Contributor

grynspan commented Jun 2, 2025

Can we conditionally mark the __store functions @safe?

…ry-safety only on newer toolchains and only attempting to include the relevant test code in that situation
…tions by (conditionally) marking them `@safe` when using a new-enough compiler
… 'unsafe' expression keyword is included. This expands the scope of a similar fix made earlier.
@stmontgomery stmontgomery marked this pull request as ready for review June 2, 2025 16:08
@stmontgomery
Copy link
Contributor Author

stmontgomery commented Jun 2, 2025

Can we conditionally mark the __store functions @safe?

Yes, I've gone ahead and done that, and removed the relevant unsafe keywords afterward.

@stmontgomery
Copy link
Contributor Author

This fails to build when using a pre-6.2 toolchain because those toolchains don't recognize the unsafe keyword. (Unlike the @unsafe attribute which 6.1 toolchains are aware of.) I'll need to figure out some way to conditionalize this — hopefully without resorting to #if checks in the macro expansion code itself.

OK I've adjusted the PR to only include the unsafe expression keyword when the macro plugin was built using a ≥ 6.2 compiler. This approach expands the scope of a similar fix I made earlier in #1093.

With this and some other adjustments, I am now able to build the package locally using a 6.1 toolchain successfully.

@stmontgomery
Copy link
Contributor Author

@swift-ci please test

@stmontgomery
Copy link
Contributor Author

@swift-ci please test

@stmontgomery
Copy link
Contributor Author

@swift-ci please test

@stmontgomery
Copy link
Contributor Author

@swift-ci please test

@stmontgomery stmontgomery merged commit ca81dff into swiftlang:main Jun 2, 2025
3 checks passed
@stmontgomery stmontgomery deleted the unsafe-test-decls branch June 2, 2025 19:41
@stmontgomery stmontgomery modified the milestones: Swift 6.x, Swift 6.2 Jun 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discovery 🔎 test content discovery enhancement New feature or request exit-tests ☠️ Work related to exit tests macros 🔭 Related to Swift macros such as @Test or #expect
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants