diff --git a/include/swift/AST/DiagnosticsSema.def b/include/swift/AST/DiagnosticsSema.def index e953aacace25e..8a4a83c5fec4b 100644 --- a/include/swift/AST/DiagnosticsSema.def +++ b/include/swift/AST/DiagnosticsSema.def @@ -3147,6 +3147,9 @@ WARNING(witness_deprecated,none, "protocol %1%select{|: %2}2", (const ValueDecl *, Identifier, StringRef)) +WARNING(unavailable_conformance,none, + "conformance of %0 to protocol %1 is already unavailable", + (Type, Identifier)) ERROR(redundant_conformance,none, "redundant conformance of %0 to protocol %1", (Type, Identifier)) ERROR(redundant_conformance_conditional,none, @@ -5557,6 +5560,9 @@ ERROR(non_sendable_isolated_capture,none, ERROR(implicit_async_let_non_sendable_capture,none, "capture of %1 with non-sendable type %0 in 'async let' binding", (Type, DeclName)) +ERROR(self_capture_deinit_task,none, + "capture of 'self' in a closure that outlives deinit", + ()) ERROR(implicit_non_sendable_capture,none, "implicit capture of %1 requires that %0 conforms to `Sendable`", (Type, DeclName)) diff --git a/lib/AST/ConformanceLookupTable.cpp b/lib/AST/ConformanceLookupTable.cpp index 59f693365aa01..85639161717f1 100644 --- a/lib/AST/ConformanceLookupTable.cpp +++ b/lib/AST/ConformanceLookupTable.cpp @@ -69,9 +69,18 @@ void ConformanceLookupTable::ConformanceEntry::markSupersededBy( SupersededBy = entry; if (diagnose) { + // If an unavailable Sendable conformance is superseded by a + // retroactive one in the client, we need to record this error + // at the client decl context. + auto *dc = getDeclContext(); + if (getProtocol()->isMarkerProtocol() && isFixed() && + !entry->isFixed()) { + dc = entry->getDeclContext(); + } + // Record the problem in the conformance table. We'll // diagnose these in semantic analysis. - table.AllSupersededDiagnostics[getDeclContext()].push_back(this); + table.AllSupersededDiagnostics[dc].push_back(this); } } @@ -259,14 +268,6 @@ void ConformanceLookupTable::inheritConformances(ClassDecl *classDecl, auto addInheritedConformance = [&](ConformanceEntry *entry) { auto protocol = entry->getProtocol(); - // Don't add unavailable conformances. - if (auto dc = entry->Source.getDeclContext()) { - if (auto ext = dyn_cast(dc)) { - if (AvailableAttr::isUnavailable(ext)) - return; - } - } - // Don't add redundant conformances here. This is merely an // optimization; resolveConformances() would zap the duplicates // anyway. @@ -614,6 +615,10 @@ ConformanceLookupTable::Ordering ConformanceLookupTable::compareConformances( // same conformance, this silently takes the class and drops the extension. if (lhs->getDeclContext()->isAlwaysAvailableConformanceContext() != rhs->getDeclContext()->isAlwaysAvailableConformanceContext()) { + // Diagnose conflicting marker protocol conformances that differ in + // un-availability. + diagnoseSuperseded = lhs->getProtocol()->isMarkerProtocol(); + return (lhs->getDeclContext()->isAlwaysAvailableConformanceContext() ? Ordering::Before : Ordering::After); @@ -621,23 +626,12 @@ ConformanceLookupTable::Ordering ConformanceLookupTable::compareConformances( // If one entry is fixed and the other is not, we have our answer. if (lhs->isFixed() != rhs->isFixed()) { - auto isReplaceableOrMarker = [](ConformanceEntry *entry) -> bool { - ConformanceEntryKind kind = entry->getRankingKind(); - if (isReplaceable(kind)) - return true; - - // Allow replacement of an explicit conformance to a marker protocol. - // (This permits redundant explicit declarations of `Sendable`.) - return (kind == ConformanceEntryKind::Explicit - && entry->getProtocol()->isMarkerProtocol()); - }; - // If the non-fixed conformance is not replaceable, we have a failure to // diagnose. // FIXME: We should probably diagnose if they have different constraints. - diagnoseSuperseded = (lhs->isFixed() && !isReplaceableOrMarker(rhs)) || - (rhs->isFixed() && !isReplaceableOrMarker(lhs)); - + diagnoseSuperseded = (lhs->isFixed() && !isReplaceable(rhs->getRankingKind())) || + (rhs->isFixed() && !isReplaceable(lhs->getRankingKind())); + return lhs->isFixed() ? Ordering::Before : Ordering::After; } @@ -879,8 +873,6 @@ DeclContext *ConformanceLookupTable::getConformingContext( return nullptr; auto inheritedConformance = ModuleDecl::lookupConformance( superclassTy, protocol, /*allowMissing=*/false); - if (inheritedConformance.hasUnavailableConformance()) - inheritedConformance = ProtocolConformanceRef::forInvalid(); if (inheritedConformance) return superclassDecl; } while ((superclassDecl = superclassDecl->getSuperclassDecl())); @@ -1145,9 +1137,17 @@ void ConformanceLookupTable::lookupConformances( if (diagnostics) { auto knownDiags = AllSupersededDiagnostics.find(dc); if (knownDiags != AllSupersededDiagnostics.end()) { - for (const auto *entry : knownDiags->second) { + for (auto *entry : knownDiags->second) { ConformanceEntry *supersededBy = entry->getSupersededBy(); + // Diagnose the client conformance as superseded. + auto *definingModule = nominal->getParentModule(); + if (entry->getDeclContext()->getParentModule() == definingModule && + supersededBy->getDeclContext()->getParentModule() != definingModule) { + supersededBy = entry; + entry = entry->getSupersededBy(); + } + diagnostics->push_back({entry->getProtocol(), entry->getDeclaredLoc(), entry->getKind(), diff --git a/lib/AST/ProtocolConformanceRef.cpp b/lib/AST/ProtocolConformanceRef.cpp index 425522bc9b036..06f05ef44f3a7 100644 --- a/lib/AST/ProtocolConformanceRef.cpp +++ b/lib/AST/ProtocolConformanceRef.cpp @@ -288,7 +288,8 @@ bool ProtocolConformanceRef::hasUnavailableConformance() const { // Check whether this conformance is on an unavailable extension. auto concrete = getConcrete(); - auto ext = dyn_cast(concrete->getDeclContext()); + auto *dc = concrete->getRootConformance()->getDeclContext(); + auto ext = dyn_cast(dc); if (ext && AvailableAttr::isUnavailable(ext)) return true; diff --git a/lib/Sema/TypeCheckConcurrency.cpp b/lib/Sema/TypeCheckConcurrency.cpp index be7fc3a350663..15519cf62086b 100644 --- a/lib/Sema/TypeCheckConcurrency.cpp +++ b/lib/Sema/TypeCheckConcurrency.cpp @@ -2582,6 +2582,25 @@ namespace { if (capture.isOpaqueValue()) continue; + auto *closure = localFunc.getAbstractClosureExpr(); + + // Diagnose a `self` capture inside an escaping `sending` + // `@Sendable` closure in a deinit, which almost certainly + // means `self` would escape deinit at runtime. + auto *explicitClosure = dyn_cast_or_null(closure); + auto *dc = getDeclContext(); + if (explicitClosure && isa(dc) && + !explicitClosure->getType()->isNoEscape() && + (explicitClosure->isPassedToSendingParameter() || + explicitClosure->isSendable())) { + auto var = dyn_cast_or_null(capture.getDecl()); + if (var && var->isSelfParameter()) { + ctx.Diags.diagnose(explicitClosure->getLoc(), + diag::self_capture_deinit_task) + .warnUntilSwiftVersion(6); + } + } + // If the closure won't execute concurrently with the context in // which the declaration occurred, it's okay. auto decl = capture.getDecl(); @@ -2606,7 +2625,6 @@ namespace { if (type->hasError()) continue; - auto *closure = localFunc.getAbstractClosureExpr(); if (closure && closure->isImplicit()) { auto *patternBindingDecl = getTopPatternBindingDecl(); if (patternBindingDecl && patternBindingDecl->isAsyncLet()) { @@ -6179,8 +6197,16 @@ ProtocolConformance *swift::deriveImplicitSendableConformance( // Local function to form the implicit conformance. auto formConformance = [&](const DeclAttribute *attrMakingUnavailable) - -> NormalProtocolConformance * { + -> ProtocolConformance * { DeclContext *conformanceDC = nominal; + + // FIXME: @_nonSendable should be a builtin extension macro. This behavior + // of explanding the unavailable conformance during implicit Sendable + // derivation means that clients can unknowingly ignore unavailable Sendable + // Sendable conformances from the original module added via @_nonSendable + // because they are not expanded if an explicit conformance is found via + // conformance lookup. So, if a retroactive, unchecked Sendable conformance + // is written, no redundant conformance warning is emitted. if (attrMakingUnavailable) { // Conformance availability is currently tied to the declaring extension. // FIXME: This is a hack--we should give conformances real availability. @@ -6208,6 +6234,18 @@ ProtocolConformance *swift::deriveImplicitSendableConformance( file->getOrCreateSynthesizedFile().addTopLevelDecl(extension); conformanceDC = extension; + + // Let the conformance lookup table register the conformance + // from the extension. Otherwise, we'll end up with redundant + // conformances between the explicit conformance from the extension + // and the conformance synthesized below. + SmallVector conformances; + nominal->lookupConformance(proto, conformances); + for (auto conformance : conformances) { + if (conformance->getDeclContext() == conformanceDC) { + return conformance; + } + } } auto conformance = ctx.getNormalConformance( @@ -6229,9 +6267,6 @@ ProtocolConformance *swift::deriveImplicitSendableConformance( auto inheritedConformance = ModuleDecl::checkConformance( classDecl->mapTypeIntoContext(superclass), proto, /*allowMissing=*/false); - if (inheritedConformance.hasUnavailableConformance()) - inheritedConformance = ProtocolConformanceRef::forInvalid(); - if (inheritedConformance) { inheritedConformance = inheritedConformance .mapConformanceOutOfContext(); diff --git a/lib/Sema/TypeCheckProtocol.cpp b/lib/Sema/TypeCheckProtocol.cpp index b98587b0cde12..c11a3823e2af6 100644 --- a/lib/Sema/TypeCheckProtocol.cpp +++ b/lib/Sema/TypeCheckProtocol.cpp @@ -6256,20 +6256,56 @@ void TypeChecker::checkConformancesInContext(IterableDeclContext *idc) { // protocol, just warn; we'll pick up the original conformance. auto existingModule = diag.ExistingDC->getParentModule(); auto extendedNominal = diag.ExistingDC->getSelfNominalTypeDecl(); - if (existingModule != dc->getParentModule() && - (existingModule->getName() == - extendedNominal->getParentModule()->getName() || + auto definingModule = extendedNominal->getParentModule()->getName(); + bool conformanceInOrigModule = + (existingModule->getName() == definingModule || existingModule == diag.Protocol->getParentModule() || - existingModule->getName().is("CoreGraphics"))) { + existingModule->getName().is("CoreGraphics")); + + // Redundant Sendable conformances are always warnings. + auto knownProtocol = diag.Protocol->getKnownProtocolKind(); + bool isSendable = knownProtocol == KnownProtocolKind::Sendable; + // Try to find an inherited Sendable conformance if there is one. + if (isSendable && !SendableConformance) { + SmallVector conformances; + nominal->lookupConformance(diag.Protocol, conformances); + for (auto conformance : conformances) { + if (isa(conformance)) + SendableConformance = conformance; + } + } + + if ((existingModule != dc->getParentModule() && conformanceInOrigModule) || + isSendable) { // Warn about the conformance. - auto diagID = differentlyConditional - ? diag::redundant_conformance_adhoc_conditional - : diag::redundant_conformance_adhoc; - Context.Diags.diagnose(diag.Loc, diagID, dc->getDeclaredInterfaceType(), - diag.Protocol->getName(), - existingModule->getName() == - extendedNominal->getParentModule()->getName(), - existingModule->getName()); + if (isSendable && SendableConformance && + isa(SendableConformance)) { + // Allow re-stated unchecked conformances to Sendable in subclasses + // as long as the inherited conformance isn't unavailable. + auto *conformance = SendableConformance->getRootConformance(); + auto *decl = conformance->getDeclContext()->getAsDecl(); + if (!AvailableAttr::isUnavailable(decl)) { + continue; + } + + Context.Diags.diagnose(diag.Loc, diag::unavailable_conformance, + nominal->getDeclaredInterfaceType(), + diag.Protocol->getName()); + } else if (existingModule == dc->getParentModule()) { + Context.Diags.diagnose(diag.Loc, diag::redundant_conformance, + nominal->getDeclaredInterfaceType(), + diag.Protocol->getName()) + .limitBehavior(DiagnosticBehavior::Warning); + } else { + auto diagID = differentlyConditional + ? diag::redundant_conformance_adhoc_conditional + : diag::redundant_conformance_adhoc; + Context.Diags.diagnose(diag.Loc, diagID, dc->getDeclaredInterfaceType(), + diag.Protocol->getName(), + existingModule->getName() == + extendedNominal->getParentModule()->getName(), + existingModule->getName()); + } // Complain about any declarations in this extension whose names match // a requirement in that protocol. diff --git a/test/Concurrency/Inputs/SendableConformances.swift b/test/Concurrency/Inputs/SendableConformances.swift new file mode 100644 index 0000000000000..33a4b3995158e --- /dev/null +++ b/test/Concurrency/Inputs/SendableConformances.swift @@ -0,0 +1,9 @@ + +public class NonSendableClass {} + +@available(*, unavailable) +extension NonSendableClass: @unchecked Sendable {} + +public struct SendableStruct: Sendable {} + +public struct AnotherSendableStruct: Sendable {} diff --git a/test/Concurrency/redundant_sendable_conformance.swift b/test/Concurrency/redundant_sendable_conformance.swift new file mode 100644 index 0000000000000..956636e695ec4 --- /dev/null +++ b/test/Concurrency/redundant_sendable_conformance.swift @@ -0,0 +1,24 @@ +// RUN: %empty-directory(%t) +// RUN: %target-swift-frontend -emit-module -emit-module-path %t/SendableConformances.swiftmodule -module-name SendableConformances %S/Inputs/SendableConformances.swift + +// RUN: %target-swift-frontend -typecheck %s -verify -swift-version 6 -I %t + +// REQUIRES: concurrency + +import SendableConformances + +typealias RequireSendable = T + +extension NonSendableClass: @retroactive @unchecked Sendable {} +// expected-warning@-1 {{conformance of 'NonSendableClass' to protocol 'Sendable' was already stated in the type's module 'SendableConformances'}} + +typealias CheckNonSendableClass = RequireSendable + +extension SendableStruct: @retroactive @unchecked Sendable {} +// expected-warning@-1 {{conformance of 'SendableStruct' to protocol 'Sendable' was already stated in the type's module 'SendableConformances'}} + +@available(*, unavailable) +extension AnotherSendableStruct: @retroactive @unchecked Sendable {} +// expected-warning@-1 {{conformance of 'AnotherSendableStruct' to protocol 'Sendable' was already stated in the type's module 'SendableConformances'}} + +typealias CheckAnotherSendableStruct = RequireSendable diff --git a/test/Concurrency/self_escapes_deinit.swift b/test/Concurrency/self_escapes_deinit.swift new file mode 100644 index 0000000000000..352256dfdb861 --- /dev/null +++ b/test/Concurrency/self_escapes_deinit.swift @@ -0,0 +1,37 @@ +// RUN: %target-typecheck-verify-swift -strict-concurrency=complete -disable-availability-checking + +@MainActor +class C { + let x: Int = 0 + + deinit { + // expected-warning@+1 {{capture of 'self' in a closure that outlives deinit; this is an error in the Swift 6 language mode}} + Task { @MainActor in + _ = self + } + + // expected-warning@+1 {{capture of 'self' in a closure that outlives deinit; this is an error in the Swift 6 language mode}} + Task { + _ = x + } + } +} + +func enqueueSomewhereElse(_ closure: @escaping @Sendable () -> Void) {} + +@MainActor +class C2 { + let x: Int = 0 + + deinit { + // expected-warning@+1 {{capture of 'self' in a closure that outlives deinit; this is an error in the Swift 6 language mode}} + enqueueSomewhereElse { + _ = self + } + + // expected-warning@+1 {{capture of 'self' in a closure that outlives deinit; this is an error in the Swift 6 language mode}} + enqueueSomewhereElse { + _ = self.x + } + } +} diff --git a/test/Concurrency/sendable_checking.swift b/test/Concurrency/sendable_checking.swift index aa2fefde2f7e1..9a51673d76679 100644 --- a/test/Concurrency/sendable_checking.swift +++ b/test/Concurrency/sendable_checking.swift @@ -102,7 +102,10 @@ public actor MyActor: MyProto { } // Make sure the generic signature doesn't minimize away Sendable requirements. -@_nonSendable class NSClass { } +class NSClass { } + +@available(*, unavailable) +extension NSClass: @unchecked Sendable {} // expected-note {{conformance of 'NSClass' to 'Sendable' has been explicitly marked unavailable here}} struct WrapClass { var t: T @@ -110,13 +113,13 @@ struct WrapClass { extension WrapClass: Sendable where T: Sendable { } -// Make sure we don't inherit the unavailable Sendable conformance from -// our superclass. +// expected-warning@+2 {{conformance of 'SendableSubclass' to protocol 'Sendable' is already unavailable}} +// expected-note@+1 {{'SendableSubclass' inherits conformance to protocol 'Sendable' from superclass here}} class SendableSubclass: NSClass, @unchecked Sendable { } @available(SwiftStdlib 5.1, *) func testSubclassing(obj: SendableSubclass) async { - acceptCV(obj) // okay! + acceptCV(obj) // expected-warning {{conformance of 'NSClass' to 'Sendable' is unavailable; this is an error in the Swift 6 language mode}} } diff --git a/test/Concurrency/sendable_conformance_checking.swift b/test/Concurrency/sendable_conformance_checking.swift index 2a7624d09163b..a28c03ab85ad3 100644 --- a/test/Concurrency/sendable_conformance_checking.swift +++ b/test/Concurrency/sendable_conformance_checking.swift @@ -180,7 +180,7 @@ extension SendableExtSub: @unchecked Sendable {} // Still want to know about same-class redundancy class MultiConformance: @unchecked Sendable {} // expected-note {{'MultiConformance' declares conformance to protocol 'Sendable' here}} -extension MultiConformance: @unchecked Sendable {} // expected-error {{redundant conformance of 'MultiConformance' to protocol 'Sendable'}} +extension MultiConformance: @unchecked Sendable {} // expected-warning {{redundant conformance of 'MultiConformance' to protocol 'Sendable'}} @available(SwiftStdlib 5.1, *) actor MyActor {