Skip to content

[Concurrency] Downgrade errors to warnings when Sendable requirement is inferred from a preconcurrency protocol #81960

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
Jun 6, 2025

Conversation

xedin
Copy link
Contributor

@xedin xedin commented Jun 3, 2025

If a type gets Sendable conformance requirement through another
@preconcurrency protocol the error should be downgraded even
with strict concurrency checking to allow clients time to address
the new requirement.

Resolves: rdar://146027395

@xedin xedin force-pushed the rdar-146027395 branch from 9b97ba5 to 584988c Compare June 3, 2025 23:41
@xedin
Copy link
Contributor Author

xedin commented Jun 3, 2025

@swift-ci please test

@@ -123,7 +123,7 @@ case q(Q.Type, Int) // expected-warning{{associated value 'q' of 'Sendable'-conf
}

struct S: Sendable {
var tuple: ([Q.Type], Int) // expected-warning{{stored property 'tuple' of 'Sendable'-conforming struct 'S' has non-Sendable type '([any Q.Type], Int)'}}
var tuple: ([Q.Type], Int) // expected-warning{{stored property 'tuple' of 'Sendable'-conforming struct 'S' has non-Sendable type 'any Q.Type'}}
Copy link
Member

Choose a reason for hiding this comment

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

I like that the diagnostic structurally looks through types with conditional conformances and points out the type that is actually non-Sendable. It might be nice as a follow up to somehow tailor the diagnostic in the case where the property type contains the non-Sendable type.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was the inconsistency between enum elements and properties, the former printed the mismatched type and the latter the whole type. Maybe for inner type we should use contains or uses instead of has something like:
stored property 'tuple' of 'Sendable'-conforming struct 'S' contains a non-Sendable type 'any Q.Type'?

Copy link
Member

Choose a reason for hiding this comment

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

Yeah I think contains sounds good!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll change it in this PR since CI is broken anyway due to stdlib changes.

@xedin
Copy link
Contributor Author

xedin commented Jun 4, 2025

@swift-ci please test macOS platform

@xedin
Copy link
Contributor Author

xedin commented Jun 5, 2025

@swift-ci please clean test macOS platform

@xedin
Copy link
Contributor Author

xedin commented Jun 5, 2025

@swift-ci please test Linux

@xedin
Copy link
Contributor Author

xedin commented Jun 5, 2025

@swift-ci please test Windows platform

xedin added 4 commits June 5, 2025 11:13
…econcurrencyProtocol`

The new name better reflects the intention for this Sendable check kind.
…t is inferred from a preconcurrency protocol

If a type gets `Sendable` conformace requirement through another
`@preconcurrency` protocol the error should be downgraded even
with strict concurrency checking to allow clients time to address
the new requirement.

Resolves: rdar://146027395
@xedin xedin force-pushed the rdar-146027395 branch from b8e154e to 3495c61 Compare June 5, 2025 18:16
@xedin
Copy link
Contributor Author

xedin commented Jun 5, 2025

@swift-ci please test Linux platform

@xedin
Copy link
Contributor Author

xedin commented Jun 5, 2025

@swift-ci please test Windows platform

@xedin
Copy link
Contributor Author

xedin commented Jun 6, 2025

@swift-ci please test Linux platform

@xedin
Copy link
Contributor Author

xedin commented Jun 6, 2025

@swift-ci please test macOS platform

@xedin xedin merged commit 86d036d into swiftlang:main Jun 6, 2025
5 checks passed
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