Skip to content

Commit bc4ae04

Browse files
committed
Add -require-explicit-sendable to warn about non-Sendable public types
Introduce a compiler flag that warnings about any public types defined in a module that are neither explicitly `Sendable` nor explicitly non-`Sendable` (the latter of which has no spelling currently), which is intended to help with auditing a module for Sendable conformances.
1 parent 305752e commit bc4ae04

File tree

9 files changed

+144
-18
lines changed

9 files changed

+144
-18
lines changed

include/swift/AST/DiagnosticsSema.def

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1949,6 +1949,15 @@ NOTE(non_sendable_nominal,none,
19491949
NOTE(add_nominal_sendable_conformance,none,
19501950
"consider making %0 %1 conform to the 'Sendable' protocol",
19511951
(DescriptiveDeclKind, DeclName))
1952+
WARNING(public_decl_needs_sendable,none,
1953+
"public %0 %1 does not specify whether it is 'Sendable' or not",
1954+
(DescriptiveDeclKind, DeclName))
1955+
NOTE(explicit_unchecked_sendable,none,
1956+
"add '@unchecked Sendable' conformance to %0 %1 if this type manually implements concurrency safety",
1957+
(DescriptiveDeclKind, DeclName))
1958+
NOTE(explicit_disable_sendable,none,
1959+
"make %0 %1 explicitly non-Sendable to suppress this warning",
1960+
(DescriptiveDeclKind, DeclName))
19521961

19531962
NOTE(required_by_opaque_return,none,
19541963
"required by opaque return type of %0 %1", (DescriptiveDeclKind, DeclName))

include/swift/Basic/LangOptions.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -176,6 +176,9 @@ namespace swift {
176176
// Availability macros definitions to be expanded at parsing.
177177
SmallVector<std::string, 4> AvailabilityMacros;
178178

179+
/// Require public declarations to declare that they are Sendable (or not).
180+
bool RequireExplicitSendable = false;
181+
179182
/// If false, '#file' evaluates to the full path rather than a
180183
/// human-readable string.
181184
bool EnableConcisePoundFile = false;

include/swift/Option/Options.td

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -413,6 +413,10 @@ def require_explicit_availability_target : Separate<["-"], "require-explicit-ava
413413
HelpText<"Suggest fix-its adding @available(<target>, *) to public declarations without availability">,
414414
MetaVarName<"<target>">;
415415

416+
def require_explicit_sendable : Flag<["-"], "require-explicit-sendable">,
417+
Flags<[FrontendOption, NoInteractiveOption]>,
418+
HelpText<"Require explicit Sendable annotations on public declarations">;
419+
416420
def define_availability : Separate<["-"], "define-availability">,
417421
Flags<[FrontendOption, NoInteractiveOption]>,
418422
HelpText<"Define an availability macro in the format 'macroName : iOS 13.0, macOS 10.15'">,

lib/Driver/ToolChains.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -233,6 +233,7 @@ void ToolChain::addCommonFrontendArgs(const OutputInfo &OI,
233233
inputArgs.AddLastArg(arguments, options::OPT_enable_library_evolution);
234234
inputArgs.AddLastArg(arguments, options::OPT_require_explicit_availability);
235235
inputArgs.AddLastArg(arguments, options::OPT_require_explicit_availability_target);
236+
inputArgs.AddLastArg(arguments, options::OPT_require_explicit_sendable);
236237
inputArgs.AddLastArg(arguments, options::OPT_enable_testing);
237238
inputArgs.AddLastArg(arguments, options::OPT_enable_private_imports);
238239
inputArgs.AddLastArg(arguments, options::OPT_g_Group);

lib/Frontend/CompilerInvocation.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -573,6 +573,7 @@ static bool ParseLangArgs(LangOptions &Opts, ArgList &Args,
573573
}
574574
}
575575

576+
Opts.RequireExplicitSendable |= Args.hasArg(OPT_require_explicit_sendable);
576577
for (const Arg *A : Args.filtered(OPT_define_availability)) {
577578
Opts.AvailabilityMacros.push_back(A->getValue());
578579
}

lib/Sema/TypeCheckConcurrency.cpp

Lines changed: 114 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -553,11 +553,32 @@ bool swift::isSendableType(ModuleDecl *module, Type type) {
553553
});
554554
}
555555

556+
/// Add Fix-It text for the given nominal type to adopt Sendable.
557+
static void addSendableFixIt(
558+
const NominalTypeDecl *nominal, InFlightDiagnostic &diag, bool unchecked) {
559+
if (nominal->getInherited().empty()) {
560+
SourceLoc fixItLoc = nominal->getBraces().Start;
561+
if (unchecked)
562+
diag.fixItInsert(fixItLoc, ": @unchecked Sendable");
563+
else
564+
diag.fixItInsert(fixItLoc, ": Sendable");
565+
} else {
566+
ASTContext &ctx = nominal->getASTContext();
567+
SourceLoc fixItLoc = nominal->getInherited().back().getSourceRange().End;
568+
fixItLoc = Lexer::getLocForEndOfToken(ctx.SourceMgr, fixItLoc);
569+
if (unchecked)
570+
diag.fixItInsert(fixItLoc, ", @unchecked Sendable");
571+
else
572+
diag.fixItInsert(fixItLoc, ", Sendable");
573+
}
574+
}
575+
556576
/// Produce a diagnostic for a single instance of a non-Sendable type where
557577
/// a Sendable type is required.
558578
static bool diagnoseSingleNonSendableType(
559579
Type type, ModuleDecl *module, SourceLoc loc,
560-
llvm::function_ref<bool(Type, DiagnosticBehavior)> diagnose) {
580+
llvm::function_ref<
581+
std::pair<DiagnosticBehavior, bool>(Type, DiagnosticBehavior)> diagnose) {
561582

562583
auto behavior = DiagnosticBehavior::Unspecified;
563584

@@ -594,23 +615,20 @@ static bool diagnoseSingleNonSendableType(
594615
behavior = DiagnosticBehavior::Warning;
595616
}
596617

597-
bool wasError = diagnose(type, behavior);
618+
DiagnosticBehavior actualBehavior;
619+
bool wasError;
620+
std::tie(actualBehavior, wasError) = diagnose(type, behavior);
598621

599-
if (type->is<FunctionType>()) {
622+
if (actualBehavior == DiagnosticBehavior::Ignore) {
623+
// Don't emit any other diagnostics.
624+
} else if (type->is<FunctionType>()) {
600625
ctx.Diags.diagnose(loc, diag::nonsendable_function_type);
601626
} else if (nominal && nominal->getParentModule() == module &&
602627
(isa<StructDecl>(nominal) || isa<EnumDecl>(nominal))) {
603628
auto note = nominal->diagnose(
604629
diag::add_nominal_sendable_conformance,
605630
nominal->getDescriptiveKind(), nominal->getName());
606-
if (nominal->getInherited().empty()) {
607-
SourceLoc fixItLoc = nominal->getBraces().Start;
608-
note.fixItInsert(fixItLoc, ": Sendable ");
609-
} else {
610-
SourceLoc fixItLoc = nominal->getInherited().back().getSourceRange().End;
611-
fixItLoc = Lexer::getLocForEndOfToken(ctx.SourceMgr, fixItLoc);
612-
note.fixItInsert(fixItLoc, ", Sendable");
613-
}
631+
addSendableFixIt(nominal, note, /*unchecked=*/false);
614632
} else if (nominal) {
615633
nominal->diagnose(
616634
diag::non_sendable_nominal, nominal->getDescriptiveKind(),
@@ -622,7 +640,8 @@ static bool diagnoseSingleNonSendableType(
622640

623641
bool swift::diagnoseNonSendableTypes(
624642
Type type, ModuleDecl *module, SourceLoc loc,
625-
llvm::function_ref<bool(Type, DiagnosticBehavior)> diagnose) {
643+
llvm::function_ref<
644+
std::pair<DiagnosticBehavior, bool>(Type, DiagnosticBehavior)> diagnose) {
626645
// If the Sendable protocol is missing, do nothing.
627646
auto proto = module->getASTContext().getProtocol(KnownProtocolKind::Sendable);
628647
if (!proto)
@@ -710,6 +729,81 @@ void swift::diagnoseMissingSendableConformance(
710729
type, module, loc, diag::non_sendable_type);
711730
}
712731

732+
static bool checkSendableInstanceStorage(
733+
NominalTypeDecl *nominal, DeclContext *dc, SendableCheck check);
734+
735+
void swift::diagnoseMissingExplicitSendable(NominalTypeDecl *nominal) {
736+
// Only diagnose when explicitly requested.
737+
ASTContext &ctx = nominal->getASTContext();
738+
if (!ctx.LangOpts.RequireExplicitSendable)
739+
return;
740+
741+
if (nominal->getLoc().isInvalid())
742+
return;
743+
744+
// Protocols aren't checked.
745+
if (isa<ProtocolDecl>(nominal))
746+
return;
747+
748+
// Actors are always Sendable.
749+
if (auto classDecl = dyn_cast<ClassDecl>(nominal))
750+
if (classDecl->isActor())
751+
return;
752+
753+
// Only public/open types have this check.
754+
if (!nominal->getFormalAccessScope(
755+
/*useDC=*/nullptr,
756+
/*treatUsableFromInlineAsPublic=*/true).isPublic())
757+
return;
758+
759+
// FIXME: Check for explicit "do not conform to Sendable" marker.
760+
761+
// Look for any conformance to `Sendable`.
762+
auto proto = ctx.getProtocol(KnownProtocolKind::Sendable);
763+
if (!proto)
764+
return;
765+
766+
SmallVector<ProtocolConformance *, 2> conformances;
767+
if (nominal->lookupConformance(proto, conformances))
768+
return;
769+
770+
// Diagnose it.
771+
nominal->diagnose(
772+
diag::public_decl_needs_sendable, nominal->getDescriptiveKind(),
773+
nominal->getName());
774+
775+
// Note to add a Sendable conformance, possibly an unchecked one.
776+
{
777+
bool isUnchecked = false;
778+
779+
// Non-final classes can only have @unchecked.
780+
if (auto classDecl = dyn_cast<ClassDecl>(nominal)) {
781+
if (!classDecl->isFinal())
782+
isUnchecked = true;
783+
}
784+
785+
// NOTE: We might need to introduce a conditional conformance here.
786+
if (!isUnchecked &&
787+
checkSendableInstanceStorage(
788+
nominal, nominal, SendableCheck::Implicit)) {
789+
isUnchecked = true;
790+
}
791+
792+
auto note = nominal->diagnose(
793+
isUnchecked ? diag::explicit_unchecked_sendable
794+
: diag::add_nominal_sendable_conformance,
795+
nominal->getDescriptiveKind(), nominal->getName());
796+
addSendableFixIt(nominal, note, isUnchecked);
797+
}
798+
799+
// Note to disable the warning.
800+
nominal->diagnose(
801+
diag::explicit_disable_sendable, nominal->getDescriptiveKind(),
802+
nominal->getName())
803+
.fixItInsert(nominal->getAttributeInsertionLoc(/*forModifier=*/false),
804+
"@_nonSendable ");
805+
}
806+
713807
/// Determine whether this is the main actor type.
714808
/// FIXME: the diagnostics engine has a copy of this.
715809
static bool isMainActor(Type type) {
@@ -3413,13 +3507,16 @@ static bool checkSendableInstanceStorage(
34133507
[&](Type type, DiagnosticBehavior suggestedBehavior) {
34143508
auto action = limitSendableInstanceBehavior(
34153509
langOpts, check, suggestedBehavior);
3510+
if (check == SendableCheck::Implicit)
3511+
return action;
3512+
34163513
property->diagnose(diag::non_concurrent_type_member,
34173514
false, property->getName(),
34183515
nominal->getDescriptiveKind(),
34193516
nominal->getName(),
34203517
propertyType)
34213518
.limitBehavior(action.first);
3422-
return action.second;
3519+
return action;
34233520
});
34243521

34253522
if (diagnosedProperty) {
@@ -3450,13 +3547,16 @@ static bool checkSendableInstanceStorage(
34503547
[&](Type type, DiagnosticBehavior suggestedBehavior) {
34513548
auto action = limitSendableInstanceBehavior(
34523549
langOpts, check, suggestedBehavior);
3550+
if (check == SendableCheck::Implicit)
3551+
return action;
3552+
34533553
element->diagnose(diag::non_concurrent_type_member,
34543554
true, element->getName(),
34553555
nominal->getDescriptiveKind(),
34563556
nominal->getName(),
34573557
type)
34583558
.limitBehavior(action.first);
3459-
return action.second;
3559+
return action;
34603560
});
34613561

34623562
if (diagnosedElement) {

lib/Sema/TypeCheckConcurrency.h

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -245,6 +245,10 @@ bool diagnoseNonSendableTypesInReference(
245245
void diagnoseMissingSendableConformance(
246246
SourceLoc loc, Type type, ModuleDecl *module);
247247

248+
/// If the given nominal type is public and does not explicitly
249+
/// state whether it conforms to Sendable, provide a diagnostic.
250+
void diagnoseMissingExplicitSendable(NominalTypeDecl *nominal);
251+
248252
/// Diagnose any non-Sendable types that occur within the given type, using
249253
/// the given diagnostic.
250254
///
@@ -255,7 +259,8 @@ void diagnoseMissingSendableConformance(
255259
/// \returns \c true if any diagnostics were produced, \c false otherwise.
256260
bool diagnoseNonSendableTypes(
257261
Type type, ModuleDecl *module, SourceLoc loc,
258-
llvm::function_ref<bool(Type, DiagnosticBehavior)> diagnose);
262+
llvm::function_ref<
263+
std::pair<DiagnosticBehavior, bool>(Type, DiagnosticBehavior)> diagnose);
259264

260265
namespace detail {
261266
template<typename T>
@@ -276,7 +281,8 @@ bool diagnoseNonSendableTypes(
276281
type, module, loc, [&](Type specificType, DiagnosticBehavior behavior) {
277282
ctx.Diags.diagnose(loc, diag, type, diagArgs...)
278283
.limitBehavior(behavior);
279-
return behavior == DiagnosticBehavior::Unspecified;
284+
return std::pair<DiagnosticBehavior, bool>(
285+
behavior, behavior == DiagnosticBehavior::Unspecified);
280286
});
281287
}
282288

lib/Sema/TypeCheckDeclPrimary.cpp

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2207,7 +2207,7 @@ class DeclChecker : public DeclVisitor<DeclChecker> {
22072207
visit(member);
22082208

22092209
checkInheritanceClause(ED);
2210-
2210+
diagnoseMissingExplicitSendable(ED);
22112211
checkAccessControl(ED);
22122212

22132213
TypeChecker::checkPatternBindingCaptures(ED);
@@ -2257,6 +2257,7 @@ class DeclChecker : public DeclVisitor<DeclChecker> {
22572257
TypeChecker::checkPatternBindingCaptures(SD);
22582258

22592259
checkInheritanceClause(SD);
2260+
diagnoseMissingExplicitSendable(SD);
22602261

22612262
checkAccessControl(SD);
22622263

@@ -2508,6 +2509,7 @@ class DeclChecker : public DeclVisitor<DeclChecker> {
25082509
}
25092510

25102511
checkInheritanceClause(CD);
2512+
diagnoseMissingExplicitSendable(CD);
25112513

25122514
checkAccessControl(CD);
25132515

test/Concurrency/concurrent_value_inference.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ struct S1 {
1010
var c: C2
1111
}
1212

13-
enum E1 { // expected-note{{consider making enum 'E1' conform to the 'Sendable' protocol}}{{9-9=: Sendable }}
13+
enum E1 { // expected-note {{consider making enum 'E1' conform to the 'Sendable' protocol}}{{9-9=: Sendable}}
1414
case base
1515
indirect case nested(E1)
1616
}

0 commit comments

Comments
 (0)