Skip to content
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

[6.1]Revert [ConformanceLookup] Don't allow skipping inherited unavailable conformances in favor of explicit available ones. #79596

Open
wants to merge 6 commits into
base: release/6.1
Choose a base branch
from
3 changes: 0 additions & 3 deletions include/swift/AST/DiagnosticsSema.def
Original file line number Diff line number Diff line change
Expand Up @@ -3176,9 +3176,6 @@ GROUPED_WARNING(
"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,
Expand Down
52 changes: 26 additions & 26 deletions lib/AST/ConformanceLookupTable.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -70,18 +70,9 @@ 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[dc].push_back(this);
table.AllSupersededDiagnostics[getDeclContext()].push_back(this);
}
}

Expand Down Expand Up @@ -269,6 +260,14 @@ 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<ExtensionDecl>(dc)) {
if (AvailableAttr::isUnavailable(ext))
return;
}
}

// Don't add redundant conformances here. This is merely an
// optimization; resolveConformances() would zap the duplicates
// anyway.
Expand Down Expand Up @@ -616,23 +615,30 @@ 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);
}

// 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() && !isReplaceable(rhs->getRankingKind())) ||
(rhs->isFixed() && !isReplaceable(lhs->getRankingKind()));

diagnoseSuperseded = (lhs->isFixed() && !isReplaceableOrMarker(rhs)) ||
(rhs->isFixed() && !isReplaceableOrMarker(lhs));
return lhs->isFixed() ? Ordering::Before : Ordering::After;
}

Expand Down Expand Up @@ -874,6 +880,8 @@ DeclContext *ConformanceLookupTable::getConformingContext(
return nullptr;
auto inheritedConformance = swift::lookupConformance(
superclassTy, protocol, /*allowMissing=*/false);
if (inheritedConformance.hasUnavailableConformance())
inheritedConformance = ProtocolConformanceRef::forInvalid();
if (inheritedConformance)
return superclassDecl;
} while ((superclassDecl = superclassDecl->getSuperclassDecl()));
Expand Down Expand Up @@ -1138,17 +1146,9 @@ void ConformanceLookupTable::lookupConformances(
if (diagnostics) {
auto knownDiags = AllSupersededDiagnostics.find(dc);
if (knownDiags != AllSupersededDiagnostics.end()) {
for (auto *entry : knownDiags->second) {
for (const 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(),
Expand Down
3 changes: 1 addition & 2 deletions lib/AST/ProtocolConformanceRef.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -333,8 +333,7 @@ bool ProtocolConformanceRef::hasUnavailableConformance() const {

// Check whether this conformance is on an unavailable extension.
auto concrete = getConcrete();
auto *dc = concrete->getRootConformance()->getDeclContext();
auto ext = dyn_cast<ExtensionDecl>(dc);
auto ext = dyn_cast<ExtensionDecl>(concrete->getDeclContext());
if (ext && AvailableAttr::isUnavailable(ext))
return true;

Expand Down
25 changes: 4 additions & 21 deletions lib/Sema/TypeCheckConcurrency.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6703,16 +6703,8 @@ ProtocolConformance *swift::deriveImplicitSendableConformance(

// Local function to form the implicit conformance.
auto formConformance = [&](const DeclAttribute *attrMakingUnavailable)
-> ProtocolConformance * {
-> NormalProtocolConformance * {
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.
Expand Down Expand Up @@ -6740,18 +6732,6 @@ 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<ProtocolConformance *, 2> conformances;
nominal->lookupConformance(proto, conformances);
for (auto conformance : conformances) {
if (conformance->getDeclContext() == conformanceDC) {
return conformance;
}
}
}

auto conformance = ctx.getNormalConformance(
Expand All @@ -6773,6 +6753,9 @@ ProtocolConformance *swift::deriveImplicitSendableConformance(
auto inheritedConformance = checkConformance(
classDecl->mapTypeIntoContext(superclass),
proto, /*allowMissing=*/false);
if (inheritedConformance.hasUnavailableConformance())
inheritedConformance = ProtocolConformanceRef::forInvalid();

if (inheritedConformance) {
inheritedConformance = inheritedConformance
.mapConformanceOutOfContext();
Expand Down
60 changes: 12 additions & 48 deletions lib/Sema/TypeCheckProtocol.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6370,56 +6370,20 @@ 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();
auto definingModule = extendedNominal->getParentModule()->getName();
bool conformanceInOrigModule =
(existingModule->getName() == definingModule ||
if (existingModule != dc->getParentModule() &&
(existingModule->getName() ==
extendedNominal->getParentModule()->getName() ||
existingModule == diag.Protocol->getParentModule() ||
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<ProtocolConformance *, 2> conformances;
nominal->lookupConformance(diag.Protocol, conformances);
for (auto conformance : conformances) {
if (isa<InheritedProtocolConformance>(conformance))
SendableConformance = conformance;
}
}

if ((existingModule != dc->getParentModule() && conformanceInOrigModule) ||
diag.Protocol->isMarkerProtocol()) {
existingModule->getName().is("CoreGraphics"))) {
// Warn about the conformance.
if (isSendable && SendableConformance &&
isa<InheritedProtocolConformance>(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());
}
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.
Expand Down
9 changes: 0 additions & 9 deletions test/Concurrency/Inputs/SendableConformances.swift

This file was deleted.

24 changes: 0 additions & 24 deletions test/Concurrency/redundant_sendable_conformance.swift

This file was deleted.

11 changes: 4 additions & 7 deletions test/Concurrency/sendable_checking.swift
Original file line number Diff line number Diff line change
Expand Up @@ -101,24 +101,21 @@ public actor MyActor: MyProto {
}

// Make sure the generic signature doesn't minimize away Sendable requirements.
class NSClass { }

@available(*, unavailable)
extension NSClass: @unchecked Sendable {} // expected-note {{conformance of 'NSClass' to 'Sendable' has been explicitly marked unavailable here}}
@_nonSendable class NSClass { }

struct WrapClass<T: NSClass> {
var t: T
}

extension WrapClass: Sendable where T: Sendable { }

// expected-warning@+2 {{conformance of 'SendableSubclass' to protocol 'Sendable' is already unavailable}}
// expected-note@+1 {{'SendableSubclass' inherits conformance to protocol 'Sendable' from superclass here}}
// Make sure we don't inherit the unavailable Sendable conformance from
// our superclass.
class SendableSubclass: NSClass, @unchecked Sendable { }

@available(SwiftStdlib 5.1, *)
func testSubclassing(obj: SendableSubclass) async {
acceptCV(obj) // expected-warning {{conformance of 'NSClass' to 'Sendable' is unavailable; this is an error in the Swift 6 language mode}}
acceptCV(obj) // okay!
}


Expand Down
2 changes: 1 addition & 1 deletion test/Concurrency/sendable_conformance_checking.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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-warning {{redundant conformance of 'MultiConformance' to protocol 'Sendable'}}
extension MultiConformance: @unchecked Sendable {} // expected-error {{redundant conformance of 'MultiConformance' to protocol 'Sendable'}}

@available(SwiftStdlib 5.1, *)
actor MyActor {
Expand Down
2 changes: 1 addition & 1 deletion test/Parse/inverses.swift
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ struct Burrito<Filling: ~Copyable>: ~Copyable {}
extension Burrito: Alias {} // expected-error {{conformance to 'Copyable' must be declared in a separate extension}}
// expected-note@-1 {{'Burrito<Filling>' declares conformance to protocol 'Copyable' here}}

extension Burrito: Copyable & Edible & P {} // expected-warning {{redundant conformance of 'Burrito<Filling>' to protocol 'Copyable'}}
extension Burrito: Copyable & Edible & P {} // expected-error {{redundant conformance of 'Burrito<Filling>' to protocol 'Copyable'}}

struct Blah<T: ~Copyable>: ~Copyable {}
extension Blah: P, Q, Copyable, R {} // expected-error {{generic struct 'Blah' required to be 'Copyable' but is marked with '~Copyable'}}
Expand Down
7 changes: 0 additions & 7 deletions test/decl/protocol/conforms/redundant_conformance.swift
Original file line number Diff line number Diff line change
Expand Up @@ -86,10 +86,3 @@ class Class3 {
class SomeMockClass: Class3.ProviderThree { // okay
var someInt: Int = 5
}


class ImplicitCopyable {}

class InheritImplicitCopyable: ImplicitCopyable, Copyable {}
// expected-warning@-1 {{redundant conformance of 'InheritImplicitCopyable' to protocol 'Copyable'}}
// expected-note@-2 {{'InheritImplicitCopyable' inherits conformance to protocol 'Copyable' from superclass here}}