Skip to content

[6.2] AST, Sema: Cherry-pick 4 NonisolatedNonsendingByDefault bug fixes #81573

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 4 commits into from
May 17, 2025

Conversation

AnthonyLatsis
Copy link
Collaborator

  • Explanation:
    • 2 fixes:
      • Attributes come before a closure capture list. InFlightDiagnostic::fixItAddAttribute was not accounting for this.
      • Do not emit migration mode diags if nonisolated(nonsending) is explicit. This broke when we split @execution(...) into @concurrent and nonisolated(nonsending) because the latter became its own TypeRepr, whereas the condition for whether to attempt migration diagnostics inside resolveASTFunctionType was still based on the function type's attributes alone.
    • Prevent @concurrent on closures from skipping throws inference. Introduction of @concurrent attribute caused an unintended side-effect in ClosureEffectsRequest since the attribute could only be used on async types setting async too early prevented body analysis for throws from running.
    • Fix UB in NonisolatedNonsendingByDefault migration diagnosis. The diagnostic can outlive the locally constructed attribute, which was passed by pointer, if there is an active DiagnosticTransaction.
  • Scope:
    • Diagnostics QoI for NonisolatedNonsendingByDefault migration. This code is currently the only client of InFlightDiagnostic::fixItAddAttribute.
    • throws inference for @concurrent closures.
    • NonisolatedNonsendingByDefault migration diagnosis.
  • Issues:
    • rdar://151421590
  • Original PRs:
  • Risk: Overall low.
  • Testing:
    • Added regression tests, ran source compatibility suite.
    • Added regression tests.
    • No regression test successfully reduced, issue discovered as a crash while running swift package migrate against swift-build.
  • Reviewers:

xedin and others added 4 commits May 16, 2025 21:24
…rence

Introduction of `@concurrent` attribute caused an unintended
side-effect in `ClosureEffectsRequest` since the attribute
could only be used on `async` types setting `async` too early
prevented body analysis for `throws` from running.

Resolves: rdar://151421590
(cherry picked from commit cda9866)
…nding)` is explicit

This broke when we split `@execution(...)` into `@concurrent` and
`nonisolated(nonsending)` because the latter became its own `TypeRepr`,
whereas the condition for whether to attempt migration diagnostics
inside `resolveASTFunctionType` is still based on the function type's
attributes alone.

(cherry picked from commit bee2b67)
The diagnostic can outlive the locally constructed attribute, which was
passed by pointer, if there is an active `DiagnosticTransaction`.

(cherry picked from commit f4e49d5)
@AnthonyLatsis AnthonyLatsis requested a review from a team as a code owner May 16, 2025 20:53
@xedin
Copy link
Contributor

xedin commented May 16, 2025

@swift-ci please test

@AnthonyLatsis AnthonyLatsis merged commit 810fbce into swiftlang:release/6.2 May 17, 2025
5 checks passed
@AnthonyLatsis AnthonyLatsis deleted the pinus-sibirica-6.2 branch May 17, 2025 06:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants