Skip to content

Acknowledge unsafe API usages in code expanded from testing library macros #1134

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Jun 2, 2025

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