From 82f04e5b78b1cf35383feb9a89ba95d88b07ccd6 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Tue, 3 Jun 2025 14:33:19 -0700 Subject: [PATCH 1/4] [Concurrency] Clean-up duplicate code in `checkSendableInstanceStorage` (cherry picked from commit e24196e365b161f0f50c77f5b87c33aa7574fea2) --- lib/Sema/TypeCheckConcurrency.cpp | 61 +++++-------------- .../concurrent_value_checking.swift | 2 +- .../sendable_metatype_typecheck.swift | 2 +- 3 files changed, 16 insertions(+), 49 deletions(-) diff --git a/lib/Sema/TypeCheckConcurrency.cpp b/lib/Sema/TypeCheckConcurrency.cpp index b809d6196bfdf..361629689bbc5 100644 --- a/lib/Sema/TypeCheckConcurrency.cpp +++ b/lib/Sema/TypeCheckConcurrency.cpp @@ -6783,55 +6783,20 @@ static bool checkSendableInstanceStorage( } } - // Check that the property type is Sendable. - SendableCheckContext context(dc, check); - diagnoseNonSendableTypes( - propertyType, context, - /*inDerivedConformance*/Type(), property->getLoc(), - [&](Type type, DiagnosticBehavior behavior) { - auto preconcurrency = context.preconcurrencyBehavior(type); - if (isImplicitSendableCheck(check)) { - // If this is for an externally-visible conformance, fail. - if (check == SendableCheck::ImplicitForExternallyVisible) { - invalid = true; - return true; - } - - // If we are to ignore this diagnostic, just continue. - if (behavior == DiagnosticBehavior::Ignore || - preconcurrency == DiagnosticBehavior::Ignore) - return true; - - invalid = true; - return true; - } - - if (preconcurrency) - behavior = preconcurrency.value(); - - property - ->diagnose(diag::non_concurrent_type_member, propertyType, - false, property->getName(), nominal) - .limitBehaviorWithPreconcurrency(behavior, - preconcurrency.has_value()); - return false; - }); - - if (invalid) { - // For implicit checks, bail out early if anything failed. - if (isImplicitSendableCheck(check)) - return true; - } - - return false; + return checkSendabilityOfMemberType(property, propertyType); } /// Handle an enum associated value. bool operator()(EnumElementDecl *element, Type elementType) override { - SendableCheckContext context (dc, check); + return checkSendabilityOfMemberType(element, elementType); + } + + private: + bool checkSendabilityOfMemberType(ValueDecl *member, Type memberType) { + SendableCheckContext context(dc, check); diagnoseNonSendableTypes( - elementType, context, - /*inDerivedConformance*/Type(), element->getLoc(), + memberType, context, + /*inDerivedConformance*/ Type(), member->getLoc(), [&](Type type, DiagnosticBehavior behavior) { auto preconcurrency = context.preconcurrencyBehavior(type); if (isImplicitSendableCheck(check)) { @@ -6853,9 +6818,10 @@ static bool checkSendableInstanceStorage( if (preconcurrency) behavior = preconcurrency.value(); - element - ->diagnose(diag::non_concurrent_type_member, type, true, - element->getName(), nominal) + member + ->diagnose(diag::non_concurrent_type_member, type, + isa(member), member->getName(), + nominal) .limitBehaviorWithPreconcurrency(behavior, preconcurrency.has_value()); return false; @@ -6869,6 +6835,7 @@ static bool checkSendableInstanceStorage( return false; } + } visitor(nominal, dc, check); return visitor.visit(nominal, dc) || visitor.invalid; diff --git a/test/Concurrency/concurrent_value_checking.swift b/test/Concurrency/concurrent_value_checking.swift index f7466a5b98319..8c6c32d6c361d 100644 --- a/test/Concurrency/concurrent_value_checking.swift +++ b/test/Concurrency/concurrent_value_checking.swift @@ -339,7 +339,7 @@ enum E2 { extension E2: Sendable where T: Sendable { } final class C1: Sendable { - let nc: NotConcurrent? = nil // expected-warning{{stored property 'nc' of 'Sendable'-conforming class 'C1' has non-Sendable type 'NotConcurrent?'}} + let nc: NotConcurrent? = nil // expected-warning{{stored property 'nc' of 'Sendable'-conforming class 'C1' has non-Sendable type 'NotConcurrent'}} var x: Int = 0 // expected-warning{{stored property 'x' of 'Sendable'-conforming class 'C1' is mutable}} let i: Int = 0 } diff --git a/test/Concurrency/sendable_metatype_typecheck.swift b/test/Concurrency/sendable_metatype_typecheck.swift index d0231d11fe07d..707adf27a35dd 100644 --- a/test/Concurrency/sendable_metatype_typecheck.swift +++ b/test/Concurrency/sendable_metatype_typecheck.swift @@ -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'}} } extension Q { From a947450a4dacf1da78a241d0ccb2a2ac5e039220 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Tue, 3 Jun 2025 14:37:03 -0700 Subject: [PATCH 2/4] [Concurrency] NFC: Rename `ImpliedByStandardProtocol` to `ImpliedByPreconcurrencyProtocol` The new name better reflects the intention for this Sendable check kind. (cherry picked from commit 7cca7225a1fd51c770ef2536d6e836eb501dc726) --- lib/Sema/TypeCheckConcurrency.cpp | 2 +- lib/Sema/TypeCheckConcurrency.h | 4 ++-- lib/Sema/TypeCheckProtocol.cpp | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/Sema/TypeCheckConcurrency.cpp b/lib/Sema/TypeCheckConcurrency.cpp index 361629689bbc5..7778eeb7d9d53 100644 --- a/lib/Sema/TypeCheckConcurrency.cpp +++ b/lib/Sema/TypeCheckConcurrency.cpp @@ -798,7 +798,7 @@ bool SendableCheckContext::warnInMinimalChecking() const { case SendableCheck::Explicit: return true; - case SendableCheck::ImpliedByStandardProtocol: + case SendableCheck::ImpliedByPreconcurrencyProtocol: case SendableCheck::Implicit: case SendableCheck::ImplicitForExternallyVisible: return false; diff --git a/lib/Sema/TypeCheckConcurrency.h b/lib/Sema/TypeCheckConcurrency.h index 7d74ba17d26f9..40cce44878d51 100644 --- a/lib/Sema/TypeCheckConcurrency.h +++ b/lib/Sema/TypeCheckConcurrency.h @@ -352,7 +352,7 @@ enum class SendableCheck { /// Sendable conformance was implied by a protocol that inherits from /// Sendable and also predates concurrency. - ImpliedByStandardProtocol, + ImpliedByPreconcurrencyProtocol, /// Implicit conformance to Sendable. Implicit, @@ -366,7 +366,7 @@ enum class SendableCheck { static inline bool isImplicitSendableCheck(SendableCheck check) { switch (check) { case SendableCheck::Explicit: - case SendableCheck::ImpliedByStandardProtocol: + case SendableCheck::ImpliedByPreconcurrencyProtocol: return false; case SendableCheck::Implicit: diff --git a/lib/Sema/TypeCheckProtocol.cpp b/lib/Sema/TypeCheckProtocol.cpp index 4eb2ffd32c594..d17757100882d 100644 --- a/lib/Sema/TypeCheckProtocol.cpp +++ b/lib/Sema/TypeCheckProtocol.cpp @@ -6681,7 +6681,7 @@ void TypeChecker::checkConformancesInContext(IterableDeclContext *idc) { if (!hasDeprecatedUnsafeSendable && SendableConformance) { SendableCheck check = SendableCheck::Explicit; if (sendableConformancePreconcurrency) - check = SendableCheck::ImpliedByStandardProtocol; + check = SendableCheck::ImpliedByPreconcurrencyProtocol; else if (SendableConformance->getSourceKind() == ConformanceEntryKind::Synthesized) check = SendableCheck::Implicit; From ace1e527d1a5e367f3e7a9476340a5e9b21294f7 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Tue, 3 Jun 2025 16:25:12 -0700 Subject: [PATCH 3/4] [Concurrency] Downgrade errors to warnings when `Sendable` requirement 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 (cherry picked from commit 6d452293671bb87ee3e59dc76dbdbe75b8aeace5) --- lib/Sema/TypeCheckConcurrency.cpp | 26 ++++++++++++++----- .../predates_concurrency_swift6.swift | 20 +++++++++++--- test/type/explicit_existential.swift | 2 +- 3 files changed, 37 insertions(+), 11 deletions(-) diff --git a/lib/Sema/TypeCheckConcurrency.cpp b/lib/Sema/TypeCheckConcurrency.cpp index 7778eeb7d9d53..1330587984c31 100644 --- a/lib/Sema/TypeCheckConcurrency.cpp +++ b/lib/Sema/TypeCheckConcurrency.cpp @@ -6768,11 +6768,16 @@ static bool checkSendableInstanceStorage( if (property->supportsMutation() && isolation.isUnspecified()) { auto behavior = SendableCheckContext(dc, check).defaultDiagnosticBehavior(); + // If Sendable came from a `@preconcurrency` protocol the error + // should be downgraded even with strict concurrency checking to + // allow clients time to address the new requirement. + auto preconcurrency = + check == SendableCheck::ImpliedByPreconcurrencyProtocol; if (behavior != DiagnosticBehavior::Ignore) { property ->diagnose(diag::concurrent_value_class_mutable_property, property->getName(), nominal) - .limitBehaviorUntilSwiftVersion(behavior, 6); + .limitBehaviorWithPreconcurrency(behavior, preconcurrency); } invalid = invalid || (behavior == DiagnosticBehavior::Unspecified); return true; @@ -6798,7 +6803,7 @@ static bool checkSendableInstanceStorage( memberType, context, /*inDerivedConformance*/ Type(), member->getLoc(), [&](Type type, DiagnosticBehavior behavior) { - auto preconcurrency = context.preconcurrencyBehavior(type); + auto preconcurrencyBehavior = context.preconcurrencyBehavior(type); if (isImplicitSendableCheck(check)) { // If this is for an externally-visible conformance, fail. if (check == SendableCheck::ImplicitForExternallyVisible) { @@ -6808,22 +6813,29 @@ static bool checkSendableInstanceStorage( // If we are to ignore this diagnostic, just continue. if (behavior == DiagnosticBehavior::Ignore || - preconcurrency == DiagnosticBehavior::Ignore) + preconcurrencyBehavior == DiagnosticBehavior::Ignore) return true; invalid = true; return true; } - if (preconcurrency) - behavior = preconcurrency.value(); + // If Sendable came from a `@preconcurrency` protocol the error + // should be downgraded even with strict concurrency checking to + // allow clients time to address the new requirement. + bool fromPreconcurrencyConformance = + check == SendableCheck::ImpliedByPreconcurrencyProtocol; + + if (preconcurrencyBehavior) + behavior = preconcurrencyBehavior.value(); member ->diagnose(diag::non_concurrent_type_member, type, isa(member), member->getName(), nominal) - .limitBehaviorWithPreconcurrency(behavior, - preconcurrency.has_value()); + .limitBehaviorWithPreconcurrency( + behavior, fromPreconcurrencyConformance || + preconcurrencyBehavior.has_value()); return false; }); diff --git a/test/Concurrency/predates_concurrency_swift6.swift b/test/Concurrency/predates_concurrency_swift6.swift index 9b4cf8d137501..9ce556e9ba91b 100644 --- a/test/Concurrency/predates_concurrency_swift6.swift +++ b/test/Concurrency/predates_concurrency_swift6.swift @@ -88,14 +88,14 @@ func testCallsWithAsync() async { @preconcurrency protocol P: Sendable { } protocol Q: P { } -class NS { } // expected-note 3{{class 'NS' does not conform to the 'Sendable' protocol}} +class NS { } // expected-note 5{{class 'NS' does not conform to the 'Sendable' protocol}} struct S1: P { - var ns: NS // expected-error{{stored property 'ns' of 'Sendable'-conforming struct 'S1' has non-Sendable type 'NS'}} + var ns: NS // expected-warning{{stored property 'ns' of 'Sendable'-conforming struct 'S1' has non-Sendable type 'NS'}} } struct S2: Q { - var ns: NS // expected-error{{stored property 'ns' of 'Sendable'-conforming struct 'S2' has non-Sendable type 'NS'}} + var ns: NS // expected-warning{{stored property 'ns' of 'Sendable'-conforming struct 'S2' has non-Sendable type 'NS'}} } struct S3: Q, Sendable { @@ -309,3 +309,17 @@ do { // If destination is @preconcurrency the Sendable conformance error should be downgraded d = data // expected-warning {{type 'Any' does not conform to the 'Sendable' protocol}} } + +do { + final class Mutating: P { + var state: Int = 0 // expected-warning {{stored property 'state' of 'Sendable'-conforming class 'Mutating' is mutable}} + } + + struct StructWithInit: P { + let prop = NS() // expected-warning {{stored property 'prop' of 'Sendable'-conforming struct 'StructWithInit' has non-Sendable type 'NS'}} + } + + enum E : P { + case test(NS) // expected-warning {{associated value 'test' of 'Sendable'-conforming enum 'E' has non-Sendable type 'NS'}} + } +} diff --git a/test/type/explicit_existential.swift b/test/type/explicit_existential.swift index 43aaa989c6f9f..ed629adbd7cde 100644 --- a/test/type/explicit_existential.swift +++ b/test/type/explicit_existential.swift @@ -90,7 +90,7 @@ protocol HasAssoc { do { enum MyError: Error { - case bad(Any) // expected-swift-6-error {{associated value 'bad' of 'Sendable'-conforming enum 'MyError' has non-Sendable type 'Any'}} + case bad(Any) // expected-swift-6-warning {{associated value 'bad' of 'Sendable'-conforming enum 'MyError' has non-Sendable type 'Any'}} } func checkIt(_ js: Any) throws { From fe2c8ef00af565c2e2c88c0d780b1e4e72e5d034 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Thu, 5 Jun 2025 10:02:22 -0700 Subject: [PATCH 4/4] [Diagnostics] Use `contains` instead of `has` when non-Sendable type appears inside of a member type (cherry picked from commit 3495c6184085c80a364de274419df219ba9e9954) --- include/swift/AST/DiagnosticsSema.def | 4 ++-- lib/Sema/TypeCheckConcurrency.cpp | 2 +- test/Concurrency/concurrent_value_checking.swift | 2 +- test/Concurrency/sendable_metatype_typecheck.swift | 4 ++-- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/include/swift/AST/DiagnosticsSema.def b/include/swift/AST/DiagnosticsSema.def index 8f0d4308f03df..fb9b31802c32c 100644 --- a/include/swift/AST/DiagnosticsSema.def +++ b/include/swift/AST/DiagnosticsSema.def @@ -5869,8 +5869,8 @@ ERROR(non_sendable_keypath_capture,none, (Type)) ERROR(non_concurrent_type_member,none, "%select{stored property %2|associated value %2}1 of " - "'Sendable'-conforming %kind3 has non-Sendable type %0", - (Type, bool, DeclName, const ValueDecl *)) + "'Sendable'-conforming %kind3 %select{contains|has}4 non-Sendable type %0", + (Type, bool, DeclName, const ValueDecl *, bool)) ERROR(concurrent_value_class_mutable_property,none, "stored property %0 of 'Sendable'-conforming %kind1 is mutable", (DeclName, const ValueDecl *)) diff --git a/lib/Sema/TypeCheckConcurrency.cpp b/lib/Sema/TypeCheckConcurrency.cpp index 1330587984c31..a46c1db2f5848 100644 --- a/lib/Sema/TypeCheckConcurrency.cpp +++ b/lib/Sema/TypeCheckConcurrency.cpp @@ -6832,7 +6832,7 @@ static bool checkSendableInstanceStorage( member ->diagnose(diag::non_concurrent_type_member, type, isa(member), member->getName(), - nominal) + nominal, type->isEqual(memberType)) .limitBehaviorWithPreconcurrency( behavior, fromPreconcurrencyConformance || preconcurrencyBehavior.has_value()); diff --git a/test/Concurrency/concurrent_value_checking.swift b/test/Concurrency/concurrent_value_checking.swift index 8c6c32d6c361d..0738d4ea0dd0a 100644 --- a/test/Concurrency/concurrent_value_checking.swift +++ b/test/Concurrency/concurrent_value_checking.swift @@ -339,7 +339,7 @@ enum E2 { extension E2: Sendable where T: Sendable { } final class C1: Sendable { - let nc: NotConcurrent? = nil // expected-warning{{stored property 'nc' of 'Sendable'-conforming class 'C1' has non-Sendable type 'NotConcurrent'}} + let nc: NotConcurrent? = nil // expected-warning{{stored property 'nc' of 'Sendable'-conforming class 'C1' contains non-Sendable type 'NotConcurrent'}} var x: Int = 0 // expected-warning{{stored property 'x' of 'Sendable'-conforming class 'C1' is mutable}} let i: Int = 0 } diff --git a/test/Concurrency/sendable_metatype_typecheck.swift b/test/Concurrency/sendable_metatype_typecheck.swift index 707adf27a35dd..2a090215518b6 100644 --- a/test/Concurrency/sendable_metatype_typecheck.swift +++ b/test/Concurrency/sendable_metatype_typecheck.swift @@ -119,11 +119,11 @@ class Holder: @unchecked Sendable { } enum E: Sendable { -case q(Q.Type, Int) // expected-warning{{associated value 'q' of 'Sendable'-conforming enum 'E' has non-Sendable type 'any Q.Type'}} +case q(Q.Type, Int) // expected-warning{{associated value 'q' of 'Sendable'-conforming enum 'E' contains non-Sendable type 'any Q.Type'}} } 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'}} + var tuple: ([Q.Type], Int) // expected-warning{{stored property 'tuple' of 'Sendable'-conforming struct 'S' contains non-Sendable type 'any Q.Type'}} } extension Q {