Skip to content

Fixes for unavailable superclass conformances #59132

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
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion include/swift/AST/Module.h
Original file line number Diff line number Diff line change
Expand Up @@ -631,11 +631,18 @@ class ModuleDecl
/// might include "missing" conformances, which are synthesized for some
/// protocols as an error recovery mechanism.
///
/// \param allowUnavailable When \c true, the resulting conformance reference
/// might include "unavailable" conformances, meaning that the conformance
/// cannot actually be used and will be diagnosed if used later. Pass
/// \c false here for queries that want to determine whether the conformance
/// is likely to be usable.
///
/// \returns The result of the conformance search, which will be
/// None if the type does not conform to the protocol or contain a
/// ProtocolConformanceRef if it does conform.
ProtocolConformanceRef lookupConformance(Type type, ProtocolDecl *protocol,
bool allowMissing = false);
bool allowMissing = false,
bool allowUnavailable = true);

/// Look for the conformance of the given existential type to the given
/// protocol.
Expand Down
4 changes: 4 additions & 0 deletions include/swift/AST/ProtocolConformanceRef.h
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,10 @@ class ProtocolConformanceRef {
return Union.get<ProtocolDecl*>();
}

/// Determine whether this conformance (or a conformance it depends on)
/// involves an always-unavailable conformance.
bool hasUnavailableConformance() const;

/// Determine whether this conformance (or a conformance it depends on)
/// involves a "missing" conformance anywhere. Such conformances
/// cannot be depended on to always exist.
Expand Down
22 changes: 16 additions & 6 deletions lib/AST/ConformanceLookupTable.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ DeclContext *ConformanceLookupTable::ConformanceSource::getDeclContext() const {
return getImpliedSource()->Source.getDeclContext();

case ConformanceEntryKind::Synthesized:
return getSynthesizedDecl();
return getSynthesizedDeclContext();
}

llvm_unreachable("Unhandled ConformanceEntryKind in switch.");
Expand Down Expand Up @@ -241,6 +241,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 @@ -812,7 +820,8 @@ DeclContext *ConformanceLookupTable::getConformingContext(
if (superclassTy->is<ErrorType>())
return nullptr;
auto inheritedConformance = module->lookupConformance(
superclassTy, protocol);
superclassTy, protocol, /*allowMissing=*/false,
/*allowUnavailable=*/false);
if (inheritedConformance)
return superclassDecl;
} while ((superclassDecl = superclassDecl->getSuperclassDecl()));
Expand Down Expand Up @@ -927,10 +936,11 @@ ConformanceLookupTable::getConformance(NominalTypeDecl *nominal,
return entry->Conformance.get<ProtocolConformance *>();
}

void ConformanceLookupTable::addSynthesizedConformance(NominalTypeDecl *nominal,
ProtocolDecl *protocol) {
void ConformanceLookupTable::addSynthesizedConformance(
NominalTypeDecl *nominal, ProtocolDecl *protocol,
DeclContext *conformanceDC) {
addProtocol(protocol, nominal->getLoc(),
ConformanceSource::forSynthesized(nominal));
ConformanceSource::forSynthesized(conformanceDC));
}

void ConformanceLookupTable::registerProtocolConformance(
Expand All @@ -956,7 +966,7 @@ void ConformanceLookupTable::registerProtocolConformance(
auto inherited = dyn_cast<InheritedProtocolConformance>(conformance);
ConformanceSource source
= inherited ? ConformanceSource::forInherited(cast<ClassDecl>(nominal)) :
synthesized ? ConformanceSource::forSynthesized(nominal) :
synthesized ? ConformanceSource::forSynthesized(dc) :
ConformanceSource::forExplicit(dc);

ASTContext &ctx = nominal->getASTContext();
Expand Down
13 changes: 7 additions & 6 deletions lib/AST/ConformanceLookupTable.h
Original file line number Diff line number Diff line change
Expand Up @@ -120,10 +120,10 @@ class ConformanceLookupTable : public ASTAllocated<ConformanceLookupTable> {

/// Create a synthesized conformance.
///
/// The given nominal type declaration will get a synthesized
/// The given declaration context (for a type) will get a synthesized
/// conformance to the requested protocol.
static ConformanceSource forSynthesized(NominalTypeDecl *typeDecl) {
return ConformanceSource(typeDecl, ConformanceEntryKind::Synthesized);
static ConformanceSource forSynthesized(DeclContext *dc) {
return ConformanceSource(dc, ConformanceEntryKind::Synthesized);
}

/// Return a new conformance source with the given location of "@unchecked".
Expand Down Expand Up @@ -188,9 +188,9 @@ class ConformanceLookupTable : public ASTAllocated<ConformanceLookupTable> {

/// For a synthesized conformance, retrieve the nominal type decl
/// that will receive the conformance.
NominalTypeDecl *getSynthesizedDecl() const {
DeclContext *getSynthesizedDeclContext() const {
assert(getKind() == ConformanceEntryKind::Synthesized);
return static_cast<NominalTypeDecl *>(Storage.getPointer());
return static_cast<DeclContext *>(Storage.getPointer());
}

/// Get the declaration context that this conformance will be
Expand Down Expand Up @@ -428,7 +428,8 @@ class ConformanceLookupTable : public ASTAllocated<ConformanceLookupTable> {

/// Add a synthesized conformance to the lookup table.
void addSynthesizedConformance(NominalTypeDecl *nominal,
ProtocolDecl *protocol);
ProtocolDecl *protocol,
DeclContext *conformanceDC);

/// Register an externally-supplied protocol conformance.
void registerProtocolConformance(ProtocolConformance *conformance,
Expand Down
20 changes: 15 additions & 5 deletions lib/AST/Module.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1015,7 +1015,9 @@ ModuleDecl::lookupExistentialConformance(Type type, ProtocolDecl *protocol) {
// If the existential is class-constrained, the class might conform
// concretely.
if (auto superclass = layout.explicitSuperclass) {
if (auto result = lookupConformance(superclass, protocol))
if (auto result = lookupConformance(
superclass, protocol, /*allowMissing=*/false,
/*allowUnavailable=*/false))
return result;
}

Expand Down Expand Up @@ -1071,7 +1073,8 @@ ProtocolConformanceRef ProtocolConformanceRef::forMissingOrInvalid(

ProtocolConformanceRef ModuleDecl::lookupConformance(Type type,
ProtocolDecl *protocol,
bool allowMissing) {
bool allowMissing,
bool allowUnavailable) {
// If we are recursively checking for implicit conformance of a nominal
// type to Sendable, fail without evaluating this request. This
// squashes cycles.
Expand All @@ -1088,13 +1091,18 @@ ProtocolConformanceRef ModuleDecl::lookupConformance(Type type,
auto result = evaluateOrDefault(
getASTContext().evaluator, request, ProtocolConformanceRef::forInvalid());

// If we aren't supposed to allow missing conformances through for this
// protocol, replace the result with an "invalid" result.
// If we aren't supposed to allow missing conformances but we have one,
// replace the result with an "invalid" result.
if (!allowMissing &&
shouldCreateMissingConformances(type, protocol) &&
result.hasMissingConformance(this))
return ProtocolConformanceRef::forInvalid();

// If we aren't supposed to allow unavailable conformances but we have one,
// replace the result with an "invalid" result.
if (!allowUnavailable && result.hasUnavailableConformance())
return ProtocolConformanceRef::forInvalid();

return result;
}

Expand Down Expand Up @@ -1248,7 +1256,9 @@ LookupConformanceInModuleRequest::evaluate(
// able to be resolved by a substitution that makes the archetype
// concrete.
if (auto super = archetype->getSuperclass()) {
if (auto inheritedConformance = mod->lookupConformance(super, protocol)) {
if (auto inheritedConformance = mod->lookupConformance(
super, protocol, /*allowMissing=*/false,
/*allowUnavailable=*/false)) {
return ProtocolConformanceRef(ctx.getInheritedConformance(
type, inheritedConformance.getConcrete()));
}
Expand Down
25 changes: 24 additions & 1 deletion lib/AST/ProtocolConformance.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1270,7 +1270,8 @@ void NominalTypeDecl::prepareConformanceTable() const {
auto addSynthesized = [&](KnownProtocolKind kind) {
if (auto *proto = getASTContext().getProtocol(kind)) {
if (protocols.count(proto) == 0) {
ConformanceTable->addSynthesizedConformance(mutableThis, proto);
ConformanceTable->addSynthesizedConformance(
mutableThis, proto, mutableThis);
protocols.insert(proto);
}
}
Expand Down Expand Up @@ -1743,6 +1744,28 @@ SourceLoc swift::extractNearestSourceLoc(const ProtocolConformanceRef conformanc
return SourceLoc();
}

bool ProtocolConformanceRef::hasUnavailableConformance() const {
// Abstract conformances are never unavailable.
if (!isConcrete())
return false;

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

// Check the conformances in the substitution map.
auto module = concrete->getDeclContext()->getParentModule();
auto subMap = concrete->getSubstitutions(module);
for (auto subConformance : subMap.getConformances()) {
if (subConformance.hasUnavailableConformance())
return true;
}

return false;
}

bool ProtocolConformanceRef::hasMissingConformance(ModuleDecl *module) const {
return forEachMissingConformance(module,
[](BuiltinProtocolConformance *builtin) {
Expand Down
8 changes: 5 additions & 3 deletions lib/AST/RequirementMachine/ConcreteContraction.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -275,8 +275,10 @@ Optional<Type> ConcreteContraction::substTypeParameterRec(
// 'allowMissing' value here is actually irrelevant.
auto conformance = ((*substBaseType)->isTypeParameter()
? ProtocolConformanceRef(proto)
: module->lookupConformance(*substBaseType, proto,
/*allowMissing=*/false));
: module->lookupConformance(
*substBaseType, proto,
/*allowMissing=*/false,
/*allowUnavailable=*/false));

// The base type doesn't conform, in which case the requirement remains
// unsubstituted.
Expand Down Expand Up @@ -391,7 +393,7 @@ ConcreteContraction::substRequirement(const Requirement &req) const {

if (!substFirstType->isTypeParameter() &&
!module->lookupConformance(substFirstType, proto,
allowMissing)) {
allowMissing, /*allowUnavailable=*/false)) {
// Handle the case of <T where T : P, T : C> where C is a class and
// C does not conform to P by leaving the conformance requirement
// unsubstituted.
Expand Down
2 changes: 1 addition & 1 deletion lib/AST/RequirementMachine/ConcreteTypeWitness.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ void PropertyMap::concretizeNestedTypesFromConcreteParent(
auto conformance = module->lookupConformance(concreteType,
const_cast<ProtocolDecl *>(proto),
allowMissing);
if (conformance.isInvalid()) {
if (conformance.isInvalid() || conformance.hasUnavailableConformance()) {
// For superclass rules, it is totally fine to have a signature like:
//
// protocol P {}
Expand Down
30 changes: 4 additions & 26 deletions lib/Sema/TypeCheckConcurrency.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -584,29 +584,6 @@ static void addSendableFixIt(
}
}

/// Determine whether there is an unavailable conformance here.
static bool hasUnavailableConformance(ProtocolConformanceRef conformance) {
// Abstract conformances are never unavailable.
if (!conformance.isConcrete())
return false;

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

// Check the conformances in the substitution map.
auto module = concrete->getDeclContext()->getParentModule();
auto subMap = concrete->getSubstitutions(module);
for (auto subConformance : subMap.getConformances()) {
if (hasUnavailableConformance(subConformance))
return true;
}

return false;
}

static bool shouldDiagnoseExistingDataRaces(const DeclContext *dc) {
return contextRequiresStrictConcurrencyChecking(dc, [](const AbstractClosureExpr *) {
return Type();
Expand Down Expand Up @@ -892,7 +869,7 @@ bool swift::diagnoseNonSendableTypes(

// FIXME: More detail for unavailable conformances.
auto conformance = TypeChecker::conformsToProtocol(type, proto, module);
if (conformance.isInvalid() || hasUnavailableConformance(conformance)) {
if (conformance.isInvalid() || conformance.hasUnavailableConformance()) {
return diagnoseSingleNonSendableType(type, fromContext, loc, diagnose);
}

Expand Down Expand Up @@ -1041,7 +1018,7 @@ namespace {
return true;

// If there is an unavailable conformance here, fail.
if (hasUnavailableConformance(conformance))
if (conformance.hasUnavailableConformance())
return true;

// Look for missing Sendable conformances.
Expand Down Expand Up @@ -4382,7 +4359,8 @@ ProtocolConformance *GetImplicitSendableRequest::evaluate(
auto classModule = classDecl->getParentModule();
if (auto inheritedConformance = TypeChecker::conformsToProtocol(
classDecl->mapTypeIntoContext(superclass),
proto, classModule, /*allowMissing=*/false)) {
proto, classModule, /*allowMissing=*/false,
/*allowUnavailable=*/false)) {
inheritedConformance = inheritedConformance
.mapConformanceOutOfContext();
if (inheritedConformance.isConcrete()) {
Expand Down
8 changes: 5 additions & 3 deletions lib/Sema/TypeCheckProtocol.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3388,7 +3388,8 @@ void ConformanceChecker::recordTypeWitness(AssociatedTypeDecl *assocType,
auto overriddenConformance =
DC->getParentModule()->lookupConformance(Adoptee,
overridden->getProtocol(),
/*allowMissing=*/true);
/*allowMissing=*/true,
/*allowUnavailable=*/false);
if (overriddenConformance.isInvalid() ||
!overriddenConformance.isConcrete())
continue;
Expand Down Expand Up @@ -5613,9 +5614,10 @@ TypeChecker::containsProtocol(Type T, ProtocolDecl *Proto, ModuleDecl *M,

ProtocolConformanceRef
TypeChecker::conformsToProtocol(Type T, ProtocolDecl *Proto, ModuleDecl *M,
bool allowMissing) {
bool allowMissing, bool allowUnavailable) {
// Look up conformance in the module.
auto lookupResult = M->lookupConformance(T, Proto, allowMissing);
auto lookupResult = M->lookupConformance(
T, Proto, allowMissing, allowUnavailable);
if (lookupResult.isInvalid()) {
return ProtocolConformanceRef::forInvalid();
}
Expand Down
3 changes: 2 additions & 1 deletion lib/Sema/TypeChecker.h
Original file line number Diff line number Diff line change
Expand Up @@ -803,7 +803,8 @@ ProtocolConformanceRef containsProtocol(Type T, ProtocolDecl *Proto,
/// protocol \c Proto, or \c None.
ProtocolConformanceRef conformsToProtocol(Type T, ProtocolDecl *Proto,
ModuleDecl *M,
bool allowMissing = true);
bool allowMissing = true,
bool allowUnavailable = true);

/// Check whether the type conforms to a given known protocol.
bool conformsToKnownProtocol(Type type, KnownProtocolKind protocol,
Expand Down
7 changes: 7 additions & 0 deletions test/ClangImporter/objc_async.swift
Original file line number Diff line number Diff line change
Expand Up @@ -374,3 +374,10 @@ func testSender(
sender.sendPtr(ptr)
sender.sendStringArray(stringArray)
}

// Sendable checking
public struct SomeWrapper<T: AuditedNonSendable> {
public let unit: T
}

extension SomeWrapper: Sendable where T: Sendable {}
18 changes: 18 additions & 0 deletions test/Concurrency/sendable_checking.swift
Original file line number Diff line number Diff line change
Expand Up @@ -95,3 +95,21 @@ func f() async {
n.pointee += 1
}
}

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

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

extension WrapClass: Sendable where T: Sendable { }

// 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) // okay!
}
11 changes: 11 additions & 0 deletions test/Generics/superclass_constraint.swift
Original file line number Diff line number Diff line change
Expand Up @@ -221,3 +221,14 @@ public struct Barn<T: Teddy> {
// CHECK: Generic signature: <T, S where T : Teddy>
public func foo<S>(_: S, _: Barn<T>, _: Paddock<T>) {}
}


public class Animal { }

@available(*, unavailable, message: "Not a pony")
extension Animal: Pony { }

public struct AnimalWrapper<Friend: Animal> { }

// CHECK: Generic signature: <Friend where Friend : Animal, Friend : Pony>
extension AnimalWrapper: Pony where Friend: Pony { }
Loading