Skip to content

Commit 610812a

Browse files
committed
Suggest conditional conformance to Sendable derived from instance storage.
When proposing to annotate a public type as `Sendable` due to the `-require-explicit-sendable` command-line parameter, suggest a conditional conformance when it can be determined that a generic type would be `Sendable` when certain type parameters are `Sendable`. For example, given this type: public struct DictionaryHolder<T: Hashable, U, V> { var member: [T: (U, V?)] } We will now produce a Fix-It that suggests that one add: extension DictionaryHolder: Sendable where T: Sendable, U: Sendable, V: Sendable { }
1 parent 2e81274 commit 610812a

File tree

2 files changed

+234
-74
lines changed

2 files changed

+234
-74
lines changed

lib/Sema/TypeCheckConcurrency.cpp

Lines changed: 215 additions & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -729,6 +729,83 @@ void swift::diagnoseMissingSendableConformance(
729729
type, module, loc, diag::non_sendable_type);
730730
}
731731

732+
namespace {
733+
template<typename Visitor>
734+
bool visitInstanceStorage(
735+
NominalTypeDecl *nominal, DeclContext *dc, Visitor &visitor);
736+
737+
/// Infer Sendable from the instance storage of the given nominal type.
738+
/// \returns \c None if there is no way to make the type \c Sendable,
739+
/// \c true if \c Sendable needs to be @unchecked, \c false if it can be
740+
/// \c Sendable without the @unchecked.
741+
Optional<bool> inferSendableFromInstanceStorage(
742+
NominalTypeDecl *nominal, SmallVectorImpl<Requirement> &requirements) {
743+
struct Visitor {
744+
NominalTypeDecl *nominal;
745+
SmallVectorImpl<Requirement> &requirements;
746+
bool isUnchecked = false;
747+
ProtocolDecl *sendableProto = nullptr;
748+
749+
Visitor(
750+
NominalTypeDecl *nominal, SmallVectorImpl<Requirement> &requirements
751+
) : nominal(nominal), requirements(requirements) {
752+
ASTContext &ctx = nominal->getASTContext();
753+
sendableProto = ctx.getProtocol(KnownProtocolKind::Sendable);
754+
}
755+
756+
bool operator()(VarDecl *var, Type propertyType) {
757+
// If we have a class with mutable state, only an @unchecked
758+
// conformance will work.
759+
if (isa<ClassDecl>(nominal) && var->supportsMutation())
760+
isUnchecked = true;
761+
762+
return checkType(propertyType);
763+
}
764+
765+
bool operator()(EnumElementDecl *element, Type elementType) {
766+
return checkType(elementType);
767+
}
768+
769+
/// Check sendability of the given type, recording any requirements.
770+
bool checkType(Type type) {
771+
if (!sendableProto)
772+
return true;
773+
774+
auto module = nominal->getParentModule();
775+
auto conformance = TypeChecker::conformsToProtocol(
776+
type, sendableProto, module);
777+
if (conformance.isInvalid())
778+
return true;
779+
780+
// FIXME: Look for unavailable conformances, too!
781+
782+
// Look for missing Sendable conformances.
783+
return conformance.forEachMissingConformance(module,
784+
[&](BuiltinProtocolConformance *missing) {
785+
// For anything other than Sendable, fail.
786+
if (missing->getProtocol() != sendableProto)
787+
return true;
788+
789+
// If we have an archetype, capture the requirement
790+
// to make this type Sendable.
791+
if (missing->getType()->is<ArchetypeType>()) {
792+
requirements.push_back(
793+
Requirement(
794+
RequirementKind::Conformance,
795+
missing->getType()->mapTypeOutOfContext(),
796+
sendableProto->getDeclaredType()));
797+
return false;
798+
}
799+
800+
return true;
801+
});
802+
}
803+
} visitor(nominal, requirements);
804+
805+
return visitInstanceStorage(nominal, nominal, visitor);
806+
}
807+
}
808+
732809
static bool checkSendableInstanceStorage(
733810
NominalTypeDecl *nominal, DeclContext *dc, SendableCheck check);
734811

@@ -772,26 +849,47 @@ void swift::diagnoseMissingExplicitSendable(NominalTypeDecl *nominal) {
772849

773850
// Note to add a Sendable conformance, possibly an unchecked one.
774851
{
775-
bool isUnchecked = false;
852+
llvm::SmallVector<Requirement, 2> requirements;
853+
auto canMakeSendable = inferSendableFromInstanceStorage(
854+
nominal, requirements);
776855

777856
// Non-final classes can only have @unchecked.
857+
bool isUnchecked = !canMakeSendable || *canMakeSendable;
778858
if (auto classDecl = dyn_cast<ClassDecl>(nominal)) {
779859
if (!classDecl->isFinal())
780860
isUnchecked = true;
781861
}
782862

783-
// NOTE: We might need to introduce a conditional conformance here.
784-
if (!isUnchecked &&
785-
checkSendableInstanceStorage(
786-
nominal, nominal, SendableCheck::Implicit)) {
787-
isUnchecked = true;
788-
}
789-
790863
auto note = nominal->diagnose(
791864
isUnchecked ? diag::explicit_unchecked_sendable
792865
: diag::add_nominal_sendable_conformance,
793866
nominal->getDescriptiveKind(), nominal->getName());
794-
addSendableFixIt(nominal, note, isUnchecked);
867+
if (canMakeSendable && !requirements.empty()) {
868+
// Produce a Fix-It containing a conditional conformance to Sendable,
869+
// based on the requirements harvested from instance storage.
870+
871+
// Form the where clause containing all of the requirements.
872+
std::string whereClause;
873+
{
874+
llvm::raw_string_ostream out(whereClause);
875+
llvm::interleaveComma(
876+
requirements, out,
877+
[&](const Requirement &req) {
878+
out << req.getFirstType().getString() << ": "
879+
<< req.getSecondType().getString();
880+
});
881+
}
882+
883+
// Add a Fix-It containing the conditional extension text itself.
884+
auto insertionLoc = nominal->getBraces().End;
885+
note.fixItInsertAfter(
886+
insertionLoc,
887+
("\n\nextension " + nominal->getName().str() + ": "
888+
+ (isUnchecked? "@unchecked " : "") + "Sendable where " +
889+
whereClause + " { }\n").str());
890+
} else {
891+
addSendableFixIt(nominal, note, isUnchecked);
892+
}
795893
}
796894

797895
// Note to disable the warning.
@@ -3476,18 +3574,74 @@ static std::pair<DiagnosticBehavior, bool> limitSendableInstanceBehavior(
34763574
}
34773575
}
34783576

3577+
namespace {
3578+
/// Visit the instance storage of the given nominal type as seen through
3579+
/// the given declaration context.
3580+
///
3581+
/// \param visitor Called with each (stored property, property type) pair
3582+
/// for classes/structs and with each (enum element, associated value type)
3583+
/// pair for enums.
3584+
///
3585+
/// \returns \c true if any call to the \c visitor returns \c true, and
3586+
/// \c false otherwise.
3587+
template<typename Visitor>
3588+
bool visitInstanceStorage(
3589+
NominalTypeDecl *nominal, DeclContext *dc, Visitor &visitor) {
3590+
// Walk the stored properties of classes and structs.
3591+
if (isa<StructDecl>(nominal) || isa<ClassDecl>(nominal)) {
3592+
for (auto property : nominal->getStoredProperties()) {
3593+
auto propertyType = dc->mapTypeIntoContext(property->getInterfaceType())
3594+
->getRValueType()->getReferenceStorageReferent();
3595+
if (visitor(property, propertyType))
3596+
return true;
3597+
}
3598+
3599+
return false;
3600+
}
3601+
3602+
// Walk the enum elements that have associated values.
3603+
if (auto enumDecl = dyn_cast<EnumDecl>(nominal)) {
3604+
for (auto caseDecl : enumDecl->getAllCases()) {
3605+
for (auto element : caseDecl->getElements()) {
3606+
if (!element->hasAssociatedValues())
3607+
continue;
3608+
3609+
// Check that the associated value type is Sendable.
3610+
auto elementType = dc->mapTypeIntoContext(
3611+
element->getArgumentInterfaceType());
3612+
if (visitor(element, elementType))
3613+
return true;
3614+
}
3615+
}
3616+
3617+
return false;
3618+
}
3619+
3620+
return false;
3621+
}
3622+
}
3623+
34793624
/// Check the instance storage of the given nominal type to verify whether
34803625
/// it is comprised only of Sendable instance storage.
34813626
static bool checkSendableInstanceStorage(
34823627
NominalTypeDecl *nominal, DeclContext *dc, SendableCheck check) {
34833628
// Stored properties of structs and classes must have
34843629
// Sendable-conforming types.
3485-
const LangOptions &langOpts = dc->getASTContext().LangOpts;
3486-
bool invalid = false;
3487-
if (isa<StructDecl>(nominal) || isa<ClassDecl>(nominal)) {
3488-
auto classDecl = dyn_cast<ClassDecl>(nominal);
3489-
for (auto property : nominal->getStoredProperties()) {
3490-
if (classDecl && property->supportsMutation()) {
3630+
struct Visitor {
3631+
bool invalid = false;
3632+
NominalTypeDecl *nominal;
3633+
DeclContext *dc;
3634+
SendableCheck check;
3635+
const LangOptions &langOpts;
3636+
3637+
Visitor(NominalTypeDecl *nominal, DeclContext *dc, SendableCheck check)
3638+
: nominal(nominal), dc(dc), check(check),
3639+
langOpts(nominal->getASTContext().LangOpts) { }
3640+
3641+
/// Handle a stored property.
3642+
bool operator()(VarDecl *property, Type propertyType) {
3643+
// Classes with mutable properties are not Sendable.
3644+
if (property->supportsMutation() && isa<ClassDecl>(nominal)) {
34913645
if (check == SendableCheck::Implicit)
34923646
return true;
34933647

@@ -3499,28 +3653,26 @@ static bool checkSendableInstanceStorage(
34993653
nominal->getName())
35003654
.limitBehavior(action.first);
35013655
invalid = invalid || action.second;
3502-
continue;
3656+
return true;
35033657
}
35043658

3505-
// Check that the property is Sendable.
3506-
auto propertyType = dc->mapTypeIntoContext(property->getInterfaceType())
3507-
->getRValueType()->getReferenceStorageReferent();
3659+
// Check that the property type is Sendable.
35083660
bool diagnosedProperty = diagnoseNonSendableTypes(
3509-
propertyType, dc->getParentModule(), property->getLoc(),
3510-
[&](Type type, DiagnosticBehavior suggestedBehavior) {
3511-
auto action = limitSendableInstanceBehavior(
3512-
langOpts, check, suggestedBehavior);
3513-
if (check == SendableCheck::Implicit)
3514-
return action;
3515-
3516-
property->diagnose(diag::non_concurrent_type_member,
3517-
false, property->getName(),
3518-
nominal->getDescriptiveKind(),
3519-
nominal->getName(),
3520-
propertyType)
3521-
.limitBehavior(action.first);
3522-
return action;
3523-
});
3661+
propertyType, dc->getParentModule(), property->getLoc(),
3662+
[&](Type type, DiagnosticBehavior suggestedBehavior) {
3663+
auto action = limitSendableInstanceBehavior(
3664+
langOpts, check, suggestedBehavior);
3665+
if (check == SendableCheck::Implicit)
3666+
return action;
3667+
3668+
property->diagnose(diag::non_concurrent_type_member,
3669+
false, property->getName(),
3670+
nominal->getDescriptiveKind(),
3671+
nominal->getName(),
3672+
propertyType)
3673+
.limitBehavior(action.first);
3674+
return action;
3675+
});
35243676

35253677
if (diagnosedProperty) {
35263678
invalid = true;
@@ -3529,51 +3681,42 @@ static bool checkSendableInstanceStorage(
35293681
if (check == SendableCheck::Implicit)
35303682
return true;
35313683
}
3684+
3685+
return false;
35323686
}
35333687

3534-
return invalid;
3535-
}
3688+
/// Handle an enum associated value.
3689+
bool operator()(EnumElementDecl *element, Type elementType) {
3690+
bool diagnosedElement = diagnoseNonSendableTypes(
3691+
elementType, dc->getParentModule(), element->getLoc(),
3692+
[&](Type type, DiagnosticBehavior suggestedBehavior) {
3693+
auto action = limitSendableInstanceBehavior(
3694+
langOpts, check, suggestedBehavior);
3695+
if (check == SendableCheck::Implicit)
3696+
return action;
35363697

3537-
// Associated values of enum cases must have Sendable-conforming
3538-
// types.
3539-
if (auto enumDecl = dyn_cast<EnumDecl>(nominal)) {
3540-
for (auto caseDecl : enumDecl->getAllCases()) {
3541-
for (auto element : caseDecl->getElements()) {
3542-
if (!element->hasAssociatedValues())
3543-
continue;
3698+
element->diagnose(diag::non_concurrent_type_member,
3699+
true, element->getName(),
3700+
nominal->getDescriptiveKind(),
3701+
nominal->getName(),
3702+
type)
3703+
.limitBehavior(action.first);
3704+
return action;
3705+
});
35443706

3545-
// Check that the associated value type is Sendable.
3546-
auto elementType = dc->mapTypeIntoContext(
3547-
element->getArgumentInterfaceType());
3548-
bool diagnosedElement = diagnoseNonSendableTypes(
3549-
elementType, dc->getParentModule(), element->getLoc(),
3550-
[&](Type type, DiagnosticBehavior suggestedBehavior) {
3551-
auto action = limitSendableInstanceBehavior(
3552-
langOpts, check, suggestedBehavior);
3553-
if (check == SendableCheck::Implicit)
3554-
return action;
3555-
3556-
element->diagnose(diag::non_concurrent_type_member,
3557-
true, element->getName(),
3558-
nominal->getDescriptiveKind(),
3559-
nominal->getName(),
3560-
type)
3561-
.limitBehavior(action.first);
3562-
return action;
3563-
});
3564-
3565-
if (diagnosedElement) {
3566-
invalid = true;
3567-
3568-
// For implicit checks, bail out early if anything failed.
3569-
if (check == SendableCheck::Implicit)
3570-
return true;
3571-
}
3707+
if (diagnosedElement) {
3708+
invalid = true;
3709+
3710+
// For implicit checks, bail out early if anything failed.
3711+
if (check == SendableCheck::Implicit)
3712+
return true;
35723713
}
3714+
3715+
return false;
35733716
}
3574-
}
3717+
} visitor(nominal, dc, check);
35753718

3576-
return invalid;
3719+
return visitInstanceStorage(nominal, dc, visitor) || visitor.invalid;
35773720
}
35783721

35793722
bool swift::checkSendableConformance(

test/Concurrency/require-explicit-sendable.swift

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,13 +29,13 @@ public class C2 { // expected-warning{{public class 'C2' does not specify whethe
2929
}
3030

3131
public struct S3<T> { // expected-warning{{public generic struct 'S3' does not specify whether it is 'Sendable' or not}}
32-
// expected-note@-1{{add '@unchecked Sendable' conformance to generic struct 'S3' if this type manually implements concurrency safety}}{{21-21=: @unchecked Sendable}}
32+
// expected-note@-1{{consider making generic struct 'S3' conform to the 'Sendable' protocol}}{{2-2=\n\nextension S3: Sendable where T: Sendable { \}\n}}
3333
// expected-note@-2{{make generic struct 'S3' explicitly non-Sendable to suppress this warning}}{{2-2=\n\n@available(*, unavailable)\nextension S3: Sendable { \}\n}}
3434
var t: T
3535
}
3636

3737
public struct S4<T> { // expected-warning{{public generic struct 'S4' does not specify whether it is 'Sendable' or not}}
38-
// expected-note@-1{{add '@unchecked Sendable' conformance to generic struct 'S4' if this type manually implements concurrency safety}}{{21-21=: @unchecked Sendable}}
38+
// expected-note@-1{{add '@unchecked Sendable' conformance to generic struct 'S4' if this type manually implements concurrency safety}}{{2-2=\n\nextension S4: @unchecked Sendable where T: Sendable { \}\n}}
3939
// expected-note@-2{{make generic struct 'S4' explicitly non-Sendable to suppress this warning}}{{2-2=\n\n@available(*, unavailable)\nextension S4: Sendable { \}\n}}
4040
var t: T
4141
var c: C
@@ -70,3 +70,20 @@ func testMe(s5: S5, s7: S7) {
7070
acceptSendable(s5) // expected-warning{{conformance of 'S5' to 'Sendable' is unavailable}}
7171
acceptSendable(s7) // expected-warning{{conformance of 'S7' to 'Sendable' is unavailable}}
7272
}
73+
74+
public struct S8<T: Hashable, U, V> { // expected-warning{{public generic struct 'S8' does not specify whether it is 'Sendable' or not}}
75+
// expected-note@-1{{consider making generic struct 'S8' conform to the 'Sendable' protocol}}{{2-2=\n\nextension S8: Sendable where T: Sendable, U: Sendable, V: Sendable { \}\n}}
76+
// expected-note@-2{{make generic struct 'S8' explicitly non-Sendable to suppress this warning}}
77+
var member: [T: (U, V?)]
78+
}
79+
80+
public protocol P2 {
81+
associatedtype A
82+
}
83+
84+
public struct S9<T: P2 & Hashable> {
85+
// expected-warning@-1{{public generic struct 'S9' does not specify whether it is 'Sendable' or not}}
86+
// expected-note@-2{{consider making generic struct 'S9' conform to the 'Sendable' protocol}}{{2-2=\n\nextension S9: Sendable where T: Sendable, T.A: Sendable { \}\n}}
87+
// expected-note@-3{{make generic struct 'S9' explicitly non-Sendable to suppress this warning}}
88+
var dict: [T : T.A] = [:]
89+
}

0 commit comments

Comments
 (0)