Skip to content

Commit d54abea

Browse files
committed
Implement customizable Sendable conformance diagnostics.
Rework Sendable checking to be completely based on "missing" conformances, so that we can individually diagnose missing Sendable conformances based on both the module in which the conformance check happened as well as where the type was declared. The basic rules here are to only diagnose if either the module where the non-Sendable type was declared or the module where it was checked was compiled with a mode that consistently diagnoses `Sendable`, either by virtue of being Swift 6 or because `-warn-concurrency` was provided on the command line. And have that diagnostic be an error in Swift 6 or warning in Swift 5.x. There is much tuning to be done here.
1 parent 392ba00 commit d54abea

28 files changed

+434
-220
lines changed

Diff for: include/swift/AST/Decl.h

+6-2
Original file line numberDiff line numberDiff line change
@@ -598,7 +598,7 @@ class alignas(1 << DeclAlignInBits) Decl {
598598
HasAnyUnavailableValues : 1
599599
);
600600

601-
SWIFT_INLINE_BITFIELD(ModuleDecl, TypeDecl, 1+1+1+1+1+1+1+1+1+1+1,
601+
SWIFT_INLINE_BITFIELD(ModuleDecl, TypeDecl, 1+1+1+1+1+1+1+1+1+1+1+1,
602602
/// If the module is compiled as static library.
603603
StaticLibrary : 1,
604604

@@ -633,7 +633,11 @@ class alignas(1 << DeclAlignInBits) Decl {
633633
IsMainModule : 1,
634634

635635
/// Whether this module has incremental dependency information available.
636-
HasIncrementalInfo : 1
636+
HasIncrementalInfo : 1,
637+
638+
/// Whether this module has been compiled with comprehensive checking for
639+
/// concurrency, e.g., Sendable checking.
640+
IsConcurrencyChecked : 1
637641
);
638642

639643
SWIFT_INLINE_BITFIELD(PrecedenceGroupDecl, Decl, 1+2,

Diff for: include/swift/AST/DiagnosticsSema.def

+10-10
Original file line numberDiff line numberDiff line change
@@ -1939,6 +1939,9 @@ NOTE(nonsendable_function_type,none,
19391939
NOTE(nonsendable_tuple_type,none,
19401940
"a tuple type must be composed of 'Sendable' elements to conform to "
19411941
"'Sendable'", ())
1942+
NOTE(add_nominal_sendable_conformance,none,
1943+
"add conformance of %0 %1 to the 'Sendable' protocol",
1944+
(DescriptiveDeclKind, DeclName))
19421945

19431946
NOTE(required_by_opaque_return,none,
19441947
"required by opaque return type of %0 %1", (DescriptiveDeclKind, DeclName))
@@ -4492,20 +4495,20 @@ NOTE(protocol_isolated_to_global_actor_here,none,
44924495
ERROR(isolated_parameter_not_actor,none,
44934496
"'isolated' parameter has non-actor type %0", (Type))
44944497

4495-
WARNING(non_concurrent_param_type,none,
4498+
WARNING(non_sendable_param_type,none,
44964499
"cannot pass argument of non-sendable type %0 across actors",
44974500
(Type))
4498-
WARNING(non_concurrent_result_type,none,
4501+
WARNING(non_sendable_result_type,none,
44994502
"cannot call function returning non-sendable type %0 across "
45004503
"actors", (Type))
4501-
WARNING(non_concurrent_property_type,none,
4502-
"cannot use %0 %1 with a non-sendable type %2 "
4504+
WARNING(non_sendable_property_type,none,
4505+
"cannot use %1 %2 with a non-sendable type %0 "
45034506
"%select{across actors|from concurrently-executed code}3",
4504-
(DescriptiveDeclKind, DeclName, Type, bool))
4505-
WARNING(non_concurrent_keypath_capture,none,
4507+
(Type, DescriptiveDeclKind, DeclName, bool))
4508+
WARNING(non_sendable_keypath_capture,none,
45064509
"cannot form key path that captures non-sendable type %0",
45074510
(Type))
4508-
WARNING(non_concurrent_keypath_access,none,
4511+
WARNING(non_sendable_keypath_access,none,
45094512
"cannot form key path that accesses non-sendable type %0",
45104513
(Type))
45114514
ERROR(non_concurrent_type_member,none,
@@ -4528,9 +4531,6 @@ ERROR(concurrent_value_inherit,none,
45284531
(bool, DeclName))
45294532
ERROR(non_sendable_type,none,
45304533
"type %0 does not conform to the 'Sendable' protocol", (Type))
4531-
ERROR(non_sendable_function_type,none,
4532-
"function type %0 must be marked '@Sendable' to conform to 'Sendable'",
4533-
(Type))
45344534

45354535
WARNING(unchecked_conformance_not_special,none,
45364536
"@unchecked conformance to %0 has no meaning", (Type))

Diff for: include/swift/AST/Module.h

+10
Original file line numberDiff line numberDiff line change
@@ -513,6 +513,16 @@ class ModuleDecl : public DeclContext, public TypeDecl {
513513
return Bits.ModuleDecl.IsMainModule;
514514
}
515515

516+
/// Whether this module has been compiled with comprehensive checking for
517+
/// concurrency, e.g., Sendable checking.
518+
bool isConcurrencyChecked() const {
519+
return Bits.ModuleDecl.IsConcurrencyChecked;
520+
}
521+
522+
void setIsConcurrencyChecked(bool value = true) {
523+
Bits.ModuleDecl.IsConcurrencyChecked = value;
524+
}
525+
516526
/// For the main module, retrieves the list of primary source files being
517527
/// compiled, that is, the files we're generating code for.
518528
ArrayRef<SourceFile *> getPrimarySourceFiles() const;

Diff for: include/swift/AST/ProtocolConformanceRef.h

+21
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@
1919
#include "swift/Basic/Debug.h"
2020
#include "llvm/ADT/Hashing.h"
2121
#include "llvm/ADT/PointerUnion.h"
22+
#include "llvm/ADT/STLExtras.h"
23+
#include "swift/AST/ProtocolConformanceRef.h"
2224
#include "swift/AST/Requirement.h"
2325
#include "swift/AST/TypeAlignments.h"
2426
#include "swift/AST/Type.h"
@@ -29,6 +31,7 @@ namespace llvm {
2931

3032
namespace swift {
3133

34+
class BuiltinProtocolConformance;
3235
class ConcreteDeclRef;
3336
class ProtocolConformance;
3437
enum class EffectKind : uint8_t;
@@ -72,6 +75,11 @@ class ProtocolConformanceRef {
7275
return ProtocolConformanceRef();
7376
}
7477

78+
/// Retrieve an invalid or missing conformance, as appropriate, when a
79+
/// legitimate conformance doesn't exist.
80+
static ProtocolConformanceRef forMissingOrInvalid(
81+
Type type, ProtocolDecl *proto);
82+
7583
bool isInvalid() const {
7684
return !Union;
7785
}
@@ -103,6 +111,19 @@ class ProtocolConformanceRef {
103111
/// cannot be depended on to always exist.
104112
bool hasMissingConformance(ModuleDecl *module) const;
105113

114+
/// Enumerate the missing conformances in this conformance.
115+
///
116+
/// Calls \c fn with each missing conformance found within this conformance,
117+
/// including this conformance or any conditional conformances it depends on.
118+
/// If the invocation of \c fn returns \c true, the traversal exits early
119+
/// and the overall function returns \c true.
120+
///
121+
/// \returns \c true if any invocation of \c fn returned true,
122+
/// \c false otherwise.
123+
bool forEachMissingConformance(
124+
ModuleDecl *module,
125+
llvm::function_ref<bool(BuiltinProtocolConformance *missing)> fn) const;
126+
106127
using OpaqueValue = void*;
107128
OpaqueValue getOpaqueValue() const { return Union.getOpaqueValue(); }
108129
static ProtocolConformanceRef getFromOpaqueValue(OpaqueValue value) {

Diff for: include/swift/Option/Options.td

+1-1
Original file line numberDiff line numberDiff line change
@@ -659,7 +659,7 @@ def warn_swift3_objc_inference : Flag<["-"], "warn-swift3-objc-inference">,
659659
Flags<[FrontendOption, DoesNotAffectIncrementalBuild, HelpHidden]>;
660660

661661
def warn_concurrency : Flag<["-"], "warn-concurrency">,
662-
Flags<[FrontendOption, DoesNotAffectIncrementalBuild]>,
662+
Flags<[FrontendOption, DoesNotAffectIncrementalBuild, ModuleInterfaceOptionIgnorable]>,
663663
HelpText<"Warn about code that is unsafe according to the Swift Concurrency "
664664
"model and will become ill-formed in a future language version">;
665665

Diff for: include/swift/Serialization/Validation.h

+8
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,7 @@ class ExtendedValidationInfo {
103103
unsigned ResilienceStrategy : 2;
104104
unsigned IsImplicitDynamicEnabled : 1;
105105
unsigned IsAllowModuleWithCompilerErrorsEnabled : 1;
106+
unsigned IsConcurrencyChecked : 1;
106107
} Bits;
107108
public:
108109
ExtendedValidationInfo() : Bits() {}
@@ -155,6 +156,13 @@ class ExtendedValidationInfo {
155156

156157
StringRef getModuleABIName() const { return ModuleABIName; }
157158
void setModuleABIName(StringRef name) { ModuleABIName = name; }
159+
160+
bool isConcurrencyChecked() const {
161+
return Bits.IsConcurrencyChecked;
162+
}
163+
void setIsConcurrencyChecked(bool val = true) {
164+
Bits.IsConcurrencyChecked = val;
165+
}
158166
};
159167

160168
/// Returns info about the serialized AST in the given data.

Diff for: lib/AST/GenericSignature.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -458,7 +458,7 @@ GenericSignatureImpl::lookupConformance(CanType type,
458458
if (type->isTypeParameter())
459459
return ProtocolConformanceRef(proto);
460460

461-
return M->lookupConformance(type, proto);
461+
return M->lookupConformance(type, proto, /*allowMissing=*/true);
462462
}
463463

464464
bool GenericSignatureImpl::requiresClass(Type type) const {

Diff for: lib/AST/Module.cpp

+25-26
Original file line numberDiff line numberDiff line change
@@ -489,6 +489,7 @@ ModuleDecl::ModuleDecl(Identifier name, ASTContext &ctx,
489489
Bits.ModuleDecl.IsNonSwiftModule = 0;
490490
Bits.ModuleDecl.IsMainModule = 0;
491491
Bits.ModuleDecl.HasIncrementalInfo = 0;
492+
Bits.ModuleDecl.IsConcurrencyChecked = 0;
492493
}
493494

494495
ImplicitImportList ModuleDecl::getImplicitImports() const {
@@ -978,6 +979,21 @@ static bool shouldCreateMissingConformances(ProtocolDecl *proto) {
978979
return proto->isSpecificProtocol(KnownProtocolKind::Sendable);
979980
}
980981

982+
ProtocolConformanceRef ProtocolConformanceRef::forMissingOrInvalid(
983+
Type type, ProtocolDecl *proto) {
984+
// Introduce "missing" conformances when appropriate, so that type checking
985+
// (and even code generation) can continue.
986+
ASTContext &ctx = proto->getASTContext();
987+
if (shouldCreateMissingConformances(proto)) {
988+
return ProtocolConformanceRef(
989+
ctx.getBuiltinConformance(
990+
type, proto, GenericSignature(), { },
991+
BuiltinConformanceKind::Missing));
992+
}
993+
994+
return ProtocolConformanceRef::forInvalid();
995+
}
996+
981997
ProtocolConformanceRef ModuleDecl::lookupConformance(Type type,
982998
ProtocolDecl *protocol,
983999
bool allowMissing) {
@@ -1007,23 +1023,6 @@ ProtocolConformanceRef ModuleDecl::lookupConformance(Type type,
10071023
return result;
10081024
}
10091025

1010-
/// Retrieve an invalid or missing conformance, as appropriate, when a
1011-
/// legitimate conformance doesn't exist.
1012-
static ProtocolConformanceRef getInvalidOrMissingConformance(
1013-
Type type, ProtocolDecl *proto) {
1014-
// Introduce "missing" conformances when appropriate, so that type checking
1015-
// (and even code generation) can continue.
1016-
ASTContext &ctx = proto->getASTContext();
1017-
if (shouldCreateMissingConformances(proto)) {
1018-
return ProtocolConformanceRef(
1019-
ctx.getBuiltinConformance(
1020-
type, proto, GenericSignature(), { },
1021-
BuiltinConformanceKind::Missing));
1022-
}
1023-
1024-
return ProtocolConformanceRef::forInvalid();
1025-
}
1026-
10271026
/// Synthesize a builtin tuple type conformance to the given protocol, if
10281027
/// appropriate.
10291028
static ProtocolConformanceRef getBuiltinTupleTypeConformance(
@@ -1083,7 +1082,7 @@ static ProtocolConformanceRef getBuiltinTupleTypeConformance(
10831082
ctx.getSpecializedConformance(type, genericConformance, subMap));
10841083
}
10851084

1086-
return getInvalidOrMissingConformance(type, protocol);
1085+
return ProtocolConformanceRef::forMissingOrInvalid(type, protocol);
10871086
}
10881087

10891088
/// Whether the given function type conforms to Sendable.
@@ -1116,7 +1115,7 @@ static ProtocolConformanceRef getBuiltinFunctionTypeConformance(
11161115
BuiltinConformanceKind::Synthesized));
11171116
}
11181117

1119-
return getInvalidOrMissingConformance(type, protocol);
1118+
return ProtocolConformanceRef::forMissingOrInvalid(type, protocol);
11201119
}
11211120

11221121
/// Synthesize a builtin metatype type conformance to the given protocol, if
@@ -1131,7 +1130,7 @@ static ProtocolConformanceRef getBuiltinMetaTypeTypeConformance(
11311130
BuiltinConformanceKind::Synthesized));
11321131
}
11331132

1134-
return getInvalidOrMissingConformance(type, protocol);
1133+
return ProtocolConformanceRef::forMissingOrInvalid(type, protocol);
11351134
}
11361135

11371136
/// Synthesize a builtin type conformance to the given protocol, if
@@ -1146,7 +1145,7 @@ static ProtocolConformanceRef getBuiltinBuiltinTypeConformance(
11461145
BuiltinConformanceKind::Synthesized));
11471146
}
11481147

1149-
return getInvalidOrMissingConformance(type, protocol);
1148+
return ProtocolConformanceRef::forMissingOrInvalid(type, protocol);
11501149
}
11511150

11521151
ProtocolConformanceRef
@@ -1184,7 +1183,7 @@ LookupConformanceInModuleRequest::evaluate(
11841183
return ProtocolConformanceRef(protocol);
11851184
}
11861185

1187-
return getInvalidOrMissingConformance(type, protocol);
1186+
return ProtocolConformanceRef::forMissingOrInvalid(type, protocol);
11881187
}
11891188

11901189
// An existential conforms to a protocol if the protocol is listed in the
@@ -1193,7 +1192,7 @@ LookupConformanceInModuleRequest::evaluate(
11931192
if (type->isExistentialType()) {
11941193
auto result = mod->lookupExistentialConformance(type, protocol);
11951194
if (result.isInvalid())
1196-
return getInvalidOrMissingConformance(type, protocol);
1195+
return ProtocolConformanceRef::forMissingOrInvalid(type, protocol);
11971196
return result;
11981197
}
11991198

@@ -1231,13 +1230,13 @@ LookupConformanceInModuleRequest::evaluate(
12311230

12321231
// If we don't have a nominal type, there are no conformances.
12331232
if (!nominal || isa<ProtocolDecl>(nominal))
1234-
return getInvalidOrMissingConformance(type, protocol);
1233+
return ProtocolConformanceRef::forMissingOrInvalid(type, protocol);
12351234

12361235
// Find the (unspecialized) conformance.
12371236
SmallVector<ProtocolConformance *, 2> conformances;
12381237
if (!nominal->lookupConformance(protocol, conformances)) {
12391238
if (!protocol->isSpecificProtocol(KnownProtocolKind::Sendable))
1240-
return getInvalidOrMissingConformance(type, protocol);
1239+
return ProtocolConformanceRef::forMissingOrInvalid(type, protocol);
12411240

12421241
// Try to infer Sendable conformance.
12431242
GetImplicitSendableRequest cvRequest{nominal};
@@ -1246,7 +1245,7 @@ LookupConformanceInModuleRequest::evaluate(
12461245
conformances.clear();
12471246
conformances.push_back(conformance);
12481247
} else {
1249-
return getInvalidOrMissingConformance(type, protocol);
1248+
return ProtocolConformanceRef::forMissingOrInvalid(type, protocol);
12501249
}
12511250
}
12521251

Diff for: lib/AST/ProtocolConformance.cpp

+14-7
Original file line numberDiff line numberDiff line change
@@ -1665,23 +1665,30 @@ SourceLoc swift::extractNearestSourceLoc(const ProtocolConformanceRef conformanc
16651665
}
16661666

16671667
bool ProtocolConformanceRef::hasMissingConformance(ModuleDecl *module) const {
1668+
return forEachMissingConformance(module,
1669+
[](BuiltinProtocolConformance *builtin) {
1670+
return true;
1671+
});
1672+
}
1673+
1674+
bool ProtocolConformanceRef::forEachMissingConformance(
1675+
ModuleDecl *module,
1676+
llvm::function_ref<bool(BuiltinProtocolConformance *missing)> fn) const {
16681677
if (!isConcrete())
16691678
return false;
16701679

1671-
// Is this a missing Sendable conformance?
1672-
const ProtocolConformance *concreteConf = getConcrete();
1673-
const RootProtocolConformance *rootConf = concreteConf->getRootConformance();
1680+
// Is this a missing conformance?
1681+
ProtocolConformance *concreteConf = getConcrete();
1682+
RootProtocolConformance *rootConf = concreteConf->getRootConformance();
16741683
if (auto builtinConformance = dyn_cast<BuiltinProtocolConformance>(rootConf)){
1675-
if (builtinConformance->isMissing() && builtinConformance->getProtocol()->isSpecificProtocol(
1676-
KnownProtocolKind::Sendable)) {
1684+
if (builtinConformance->isMissing() && fn(builtinConformance))
16771685
return true;
1678-
}
16791686
}
16801687

16811688
// Check conformances that are part of this conformance.
16821689
auto subMap = concreteConf->getSubstitutions(module);
16831690
for (auto conformance : subMap.getConformances()) {
1684-
if (conformance.hasMissingConformance(module))
1691+
if (conformance.forEachMissingConformance(module, fn))
16851692
return true;
16861693
}
16871694

Diff for: lib/AST/SubstitutionMap.cpp

+4-2
Original file line numberDiff line numberDiff line change
@@ -365,8 +365,10 @@ SubstitutionMap::lookupConformance(CanType type, ProtocolDecl *proto) const {
365365

366366
// If the type doesn't conform to this protocol, the result isn't formed
367367
// from these requirements.
368-
if (!genericSig->requiresProtocol(type, proto))
369-
return ProtocolConformanceRef::forInvalid();
368+
if (!genericSig->requiresProtocol(type, proto)) {
369+
Type substType = type.subst(*this);
370+
return ProtocolConformanceRef::forMissingOrInvalid(substType, proto);
371+
}
370372

371373
auto accessPath =
372374
genericSig->getConformanceAccessPath(type, proto);

Diff for: lib/Frontend/Frontend.cpp

+3
Original file line numberDiff line numberDiff line change
@@ -975,6 +975,9 @@ ModuleDecl *CompilerInstance::getMainModule() const {
975975
}
976976
if (Invocation.getFrontendOptions().EnableLibraryEvolution)
977977
MainModule->setResilienceStrategy(ResilienceStrategy::Resilient);
978+
if (Invocation.getLangOptions().WarnConcurrency ||
979+
Invocation.getLangOptions().isSwiftVersionAtLeast(6))
980+
MainModule->setIsConcurrencyChecked(true);
978981

979982
// Register the main module with the AST context.
980983
Context->addLoadedModule(MainModule);

Diff for: lib/IDE/CodeCompletion.cpp

+3-3
Original file line numberDiff line numberDiff line change
@@ -2627,7 +2627,6 @@ class CompletionLookup final : public swift::VisibleDeclConsumer {
26272627
void analyzeActorIsolation(const ValueDecl *VD, Type T, bool &implicitlyAsync,
26282628
Optional<NotRecommendedReason> &NotRecommended) {
26292629
auto isolation = getActorIsolation(const_cast<ValueDecl *>(VD));
2630-
auto &ctx = VD->getASTContext();
26312630

26322631
switch (isolation.getKind()) {
26332632
case ActorIsolation::DistributedActorInstance: {
@@ -2646,7 +2645,7 @@ class CompletionLookup final : public swift::VisibleDeclConsumer {
26462645
// if the context has adopted concurrency.
26472646
if (!CanCurrDeclContextHandleAsync &&
26482647
!completionContextUsesConcurrencyFeatures(CurrDeclContext) &&
2649-
!ctx.LangOpts.WarnConcurrency) {
2648+
!CurrDeclContext->getParentModule()->isConcurrencyChecked()) {
26502649
return;
26512650
}
26522651
LLVM_FALLTHROUGH;
@@ -2664,7 +2663,8 @@ class CompletionLookup final : public swift::VisibleDeclConsumer {
26642663
}
26652664

26662665
// If the reference is 'async', all types must be 'Sendable'.
2667-
if (ctx.LangOpts.WarnConcurrency && implicitlyAsync && T) {
2666+
if (CurrDeclContext->getParentModule()->isConcurrencyChecked() &&
2667+
implicitlyAsync && T) {
26682668
auto *M = CurrDeclContext->getParentModule();
26692669
if (isa<VarDecl>(VD)) {
26702670
if (!isSendableType(M, T)) {

0 commit comments

Comments
 (0)