Skip to content

[6.2] Sema: Make @concurrent not imply async on closures #83110

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 1 commit into from
Jul 17, 2025

Conversation

AnthonyLatsis
Copy link
Collaborator

@AnthonyLatsis AnthonyLatsis commented Jul 16, 2025

  • Explanation:
    The present approach is not prudent because @concurrent synchronous functions, a natural extension, are a likely-to-happen future direction, whereas the current inference rule is entirely grounded on @concurrent being exclusive to async functions.

    If we were to ship this rule, we would have to keep the promise for backwards compatibility when implementing the aforementioned future direction, replacing one inconsistency with another, and possibly introducing new bug-prone expression checking code.

    func foo(_: () -> Void) {}
    func foo(_: () async -> Void) {}
    
    // In a future without this change and  `@concurrent` synchronous
    // functions accepted, the first call resolves to the first overload,
    // and the second call resolves to the second, despite `@concurrent` no
    // longer implying `async`.
    foo { }
    foo { @concurrent in }

    This change also drops the fix-it for removing @concurrent when used on a synchronous closure. With the inference rule gone, and the diagnosis delayed until after solution application, this error raises a fairly balanced choice between removing the attribute and being explicit about the effect, where a unilateral suggestion is quite possibly more harmful than useful.

  • Scope: Type-checking for explicitly @concurrent closures.

  • Issues: —

  • Original PRs: Sema: Make @concurrent not imply async on closures #83094.

  • Risk: Overall low, medium for potentially existing adopters because closures that were previously inferred as async due to the attribute alone will now error.

  • Testing: Regression tests added, source compatibility suite run.

  • Reviewers: @xedin.

The present approach is not prudent because `@concurrent` synchronous
functions, a natural extension, are a likely-to-happen future direction,
whereas the current inference rule is entirely grounded on `@concurrent`
being exclusive to async functions.

If we were to ship this rule, we would have to keep the promise for
backwards compatibility when implementing the aforementioned future
direction, replacing one inconsistency with another, and possibly
introducing new bug-prone expression checking code.

```swift
func foo(_: () -> Void) {}
func foo(_: () async -> Void) {}

// In a future without this change and  `@concurrent` synchronous
// functions accepted, the first call resolves to the first overload,
// and the second call resolves to the second, despite `@concurrent` no
// longer implying `async`.
foo { }
foo { @Concurrent in }
```

This change also drops the fix-it for removing `@concurrent` when used
on a synchronous closure. With the inference rule gone, and the
diagnosis delayed until after solution application, this error raises a
fairly balanced choice between removing the attribute and being
explicit about the effect, where a unilateral suggestion is quite
possibly more harmful than useful.

(cherry picked from commit 58d5059)
@AnthonyLatsis AnthonyLatsis requested a review from a team as a code owner July 16, 2025 19:58
@AnthonyLatsis AnthonyLatsis changed the title Sema: Make @concurrent not imply async on closures [6.2] Sema: Make @concurrent not imply async on closures Jul 16, 2025
@AnthonyLatsis AnthonyLatsis enabled auto-merge July 17, 2025 03:16
@AnthonyLatsis
Copy link
Collaborator Author

@swift-ci please test

@AnthonyLatsis
Copy link
Collaborator Author

@swift-ci please test Windows

@AnthonyLatsis AnthonyLatsis merged commit ffa408d into release/6.2 Jul 17, 2025
5 checks passed
@AnthonyLatsis AnthonyLatsis deleted the jepa-release branch July 17, 2025 15:44
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.

2 participants