Skip to content

Commit 11cb94f

Browse files
hborlahborla
committed
[Concurrency] Use .limitBehaviorUntilSwiftVersion for concurrency diagnostics
that are suppressed or downgraded until Swift 6. There are a few benefits to using a `UntilSwiftVersion` diagnostic engine API, including the diagnostic wrapping to communicate that the mistake will be an error in Swift 6, and to include the mistake in the frontend statistic for Swift 6 errors.
1 parent 50945ef commit 11cb94f

10 files changed

+160
-104
lines changed

‎include/swift/AST/DiagnosticEngine.h‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -575,6 +575,17 @@ namespace swift {
575575
return limitBehavior(limit);
576576
}
577577

578+
/// Conditionally limit the diagnostic behavior if the given \c limit
579+
/// is not \c None.
580+
InFlightDiagnostic &limitBehaviorIf(
581+
llvm::Optional<DiagnosticBehavior> limit) {
582+
if (!limit) {
583+
return *this;
584+
}
585+
586+
return limitBehavior(*limit);
587+
}
588+
578589
/// Limit the diagnostic behavior to \c limit until the specified
579590
/// version.
580591
///

‎include/swift/AST/DiagnosticsSema.def‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5270,7 +5270,10 @@ NOTE(note_add_async_and_throws_to_decl,none,
52705270
NOTE(note_add_distributed_to_decl,none,
52715271
"add 'distributed' to %0 to make this %kindonly0 satisfy the protocol requirement",
52725272
(const ValueDecl *))
5273-
ERROR(add_globalactor_to_function,none,
5273+
ERROR(invalid_isolated_calls_in_body,none,
5274+
"calls to '@%0'-isolated' code in %kind1",
5275+
(StringRef, const ValueDecl *))
5276+
NOTE(add_globalactor_to_function,none,
52745277
"add '@%0' to make %kind1 part of global actor %2",
52755278
(StringRef, const ValueDecl *, Type))
52765279
FIXIT(insert_globalactor_attr, "@%0 ", (Type))

‎lib/AST/DiagnosticEngine.cpp‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -331,8 +331,14 @@ InFlightDiagnostic &
331331
InFlightDiagnostic::limitBehaviorUntilSwiftVersion(
332332
DiagnosticBehavior limit, unsigned majorVersion) {
333333
if (!Engine->languageVersion.isVersionAtLeast(majorVersion)) {
334-
limitBehavior(limit)
335-
.wrapIn(diag::error_in_future_swift_version, majorVersion);
334+
// If the behavior limit is a warning or less, wrap the diagnostic
335+
// in a message that this will become an error in a later Swift
336+
// version. We do this before limiting the behavior, because
337+
// wrapIn will result in the behavior of the wrapping diagnostic.
338+
if (limit >= DiagnosticBehavior::Warning)
339+
wrapIn(diag::error_in_future_swift_version, majorVersion);
340+
341+
limitBehavior(limit);
336342
}
337343

338344
if (majorVersion == 6) {

‎lib/Sema/TypeCheckConcurrency.cpp‎

Lines changed: 86 additions & 68 deletions
Original file line numberDiff line numberDiff line change
@@ -734,16 +734,6 @@ static bool shouldDiagnoseExistingDataRaces(const DeclContext *dc) {
734734
});
735735
}
736736

737-
/// Determine the default diagnostic behavior for this language mode.
738-
static DiagnosticBehavior defaultSendableDiagnosticBehavior(
739-
const LangOptions &langOpts) {
740-
// Prior to Swift 6, all Sendable-related diagnostics are warnings at most.
741-
if (!langOpts.isSwiftVersionAtLeast(6))
742-
return DiagnosticBehavior::Warning;
743-
744-
return DiagnosticBehavior::Unspecified;
745-
}
746-
747737
bool SendableCheckContext::isExplicitSendableConformance() const {
748738
if (!conformanceCheck)
749739
return false;
@@ -766,7 +756,7 @@ DiagnosticBehavior SendableCheckContext::defaultDiagnosticBehavior() const {
766756
!shouldDiagnoseExistingDataRaces(fromDC))
767757
return DiagnosticBehavior::Ignore;
768758

769-
return defaultSendableDiagnosticBehavior(fromDC->getASTContext().LangOpts);
759+
return DiagnosticBehavior::Warning;
770760
}
771761

772762
DiagnosticBehavior
@@ -856,38 +846,8 @@ findImportFor(const DeclContext *dc, const DeclContext *fromDC) {
856846
/// nominal type.
857847
DiagnosticBehavior SendableCheckContext::diagnosticBehavior(
858848
NominalTypeDecl *nominal) const {
859-
// Determine whether this nominal type is visible via a @preconcurrency
860-
// import.
861-
auto import = findImportFor(nominal, fromDC);
862-
auto sourceFile = fromDC->getParentSourceFile();
863-
864-
// When the type is explicitly non-Sendable...
865-
if (hasExplicitSendableConformance(nominal)) {
866-
// @preconcurrency imports downgrade the diagnostic to a warning in Swift 6,
867-
if (import && import->options.contains(ImportFlags::Preconcurrency)) {
868-
if (sourceFile)
869-
sourceFile->setImportUsedPreconcurrency(*import);
870-
871-
return DiagnosticBehavior::Warning;
872-
}
873-
874-
return defaultSendableDiagnosticBehavior(fromDC->getASTContext().LangOpts);
875-
}
876-
877-
// When the type is implicitly non-Sendable...
878-
879-
// @preconcurrency suppresses the diagnostic in Swift 5.x, and
880-
// downgrades it to a warning in Swift 6 and later.
881-
if (import && import->options.contains(ImportFlags::Preconcurrency)) {
882-
if (sourceFile)
883-
sourceFile->setImportUsedPreconcurrency(*import);
884-
885-
// `@preconcurrency` suppresses diagnostics until the imported
886-
// module enables Swift 6.
887-
return import->module.importedModule->isConcurrencyChecked()
888-
? DiagnosticBehavior::Warning
889-
: DiagnosticBehavior::Ignore;
890-
}
849+
if (hasExplicitSendableConformance(nominal))
850+
return DiagnosticBehavior::Warning;
891851

892852
DiagnosticBehavior defaultBehavior = implicitSendableDiagnosticBehavior();
893853

@@ -902,6 +862,38 @@ DiagnosticBehavior SendableCheckContext::diagnosticBehavior(
902862
return defaultBehavior;
903863
}
904864

865+
llvm::Optional<DiagnosticBehavior>
866+
SendableCheckContext::preconcurrencyBehavior(Decl *decl) const {
867+
if (!decl)
868+
return llvm::None;
869+
870+
if (auto *nominal = dyn_cast<NominalTypeDecl>(decl)) {
871+
// Determine whether this nominal type is visible via a @preconcurrency
872+
// import.
873+
auto import = findImportFor(nominal, fromDC);
874+
auto sourceFile = fromDC->getParentSourceFile();
875+
876+
if (!import || !import->options.contains(ImportFlags::Preconcurrency))
877+
return llvm::None;
878+
879+
if (sourceFile)
880+
sourceFile->setImportUsedPreconcurrency(*import);
881+
882+
// When the type is explicitly non-Sendable, @preconcurrency imports
883+
// downgrade the diagnostic to a warning in Swift 6.
884+
if (hasExplicitSendableConformance(nominal))
885+
return DiagnosticBehavior::Warning;
886+
887+
// When the type is implicitly non-Sendable, `@preconcurrency` suppresses
888+
// diagnostics until the imported module enables Swift 6.
889+
return import->module.importedModule->isConcurrencyChecked()
890+
? DiagnosticBehavior::Warning
891+
: DiagnosticBehavior::Ignore;
892+
}
893+
894+
return llvm::None;
895+
}
896+
905897
static bool shouldDiagnosePreconcurrencyImports(SourceFile &sf) {
906898
switch (sf.Kind) {
907899
case SourceFileKind::Interface:
@@ -2106,7 +2098,8 @@ namespace {
21062098

21072099
/// Note when the enclosing context could be put on a global actor.
21082100
// FIXME: This should handle closures too.
2109-
static bool missingGlobalActorOnContext(DeclContext *dc, Type globalActor, DiagnosticBehavior behavior) {
2101+
static bool missingGlobalActorOnContext(DeclContext *dc, Type globalActor,
2102+
DiagnosticBehavior behavior) {
21102103
// If we are in a synchronous function on the global actor,
21112104
// suggest annotating with the global actor itself.
21122105
if (auto fn = findAnnotatableFunction(dc)) {
@@ -2125,13 +2118,19 @@ namespace {
21252118
return false;
21262119

21272120
case ActorIsolation::Unspecified:
2121+
if (behavior != DiagnosticBehavior::Note) {
2122+
fn->diagnose(diag::invalid_isolated_calls_in_body,
2123+
globalActor->getWithoutParens().getString(), fn)
2124+
.limitBehaviorUntilSwiftVersion(behavior, 6);
2125+
}
2126+
2127+
// Always emit the note with fix-it.
21282128
fn->diagnose(diag::add_globalactor_to_function,
21292129
globalActor->getWithoutParens().getString(),
21302130
fn, globalActor)
2131-
.limitBehavior(behavior)
2132-
.fixItInsert(fn->getAttributeInsertionLoc(false),
2133-
diag::insert_globalactor_attr, globalActor);
2134-
return true;
2131+
.fixItInsert(fn->getAttributeInsertionLoc(false),
2132+
diag::insert_globalactor_attr, globalActor);
2133+
return true;
21352134
}
21362135
}
21372136
}
@@ -2163,7 +2162,7 @@ namespace {
21632162
// Diagnose actor_isolated_non_self_reference as note
21642163
// if fix-it provided in missingGlobalActorOnContext
21652164
ctx.Diags.diagnose(error.loc, error.diag)
2166-
.limitBehavior(behavior);
2165+
.limitBehaviorUntilSwiftVersion(behavior, 6);
21672166
}
21682167
}
21692168

@@ -2188,7 +2187,7 @@ namespace {
21882187
// Diagnose actor_isolated_call as note if
21892188
// fix-it provided in missingGlobalActorOnContext
21902189
ctx.Diags.diagnose(error.loc, error.diag)
2191-
.limitBehavior(behavior);
2190+
.limitBehaviorUntilSwiftVersion(behavior, 6);
21922191
}
21932192
}
21942193

@@ -3012,12 +3011,21 @@ namespace {
30123011
import && import->options.contains(ImportFlags::Preconcurrency);
30133012
const auto isPreconcurrencyUnspecifiedIsolation =
30143013
isPreconcurrencyImport && isolation.isUnspecified();
3014+
3015+
// If the global variable is preconcurrency without an explicit
3016+
// isolation, ignore the warning. Otherwise, limit the behavior
3017+
// to a warning until Swift 6.
3018+
DiagnosticBehavior limit;
3019+
if (isPreconcurrencyUnspecifiedIsolation) {
3020+
limit = DiagnosticBehavior::Ignore;
3021+
} else {
3022+
limit = DiagnosticBehavior::Warning;
3023+
}
3024+
30153025
ctx.Diags.diagnose(loc, diag::shared_mutable_state_access, value)
3016-
.warnUntilSwiftVersionIf(!isPreconcurrencyUnspecifiedIsolation, 6)
3017-
.limitBehaviorIf(isPreconcurrencyImport, DiagnosticBehavior::Warning)
3018-
.limitBehaviorIf(!ctx.isSwiftVersionAtLeast(6) &&
3019-
isPreconcurrencyUnspecifiedIsolation,
3020-
DiagnosticBehavior::Ignore);
3026+
.limitBehaviorUntilSwiftVersion(limit, 6)
3027+
// Preconcurrency global variables are warnings even in Swift 6
3028+
.limitBehaviorIf(isPreconcurrencyImport, DiagnosticBehavior::Warning);
30213029
value->diagnose(diag::kind_declared_here, value->getDescriptiveKind());
30223030
if (const auto sourceFile = getDeclContext()->getParentSourceFile();
30233031
sourceFile && isPreconcurrencyImport) {
@@ -5248,7 +5256,7 @@ void swift::checkOverrideActorIsolation(ValueDecl *value) {
52485256
value->diagnose(
52495257
diag::actor_isolation_override_mismatch, isolation,
52505258
value, overriddenIsolation)
5251-
.limitBehavior(behavior);
5259+
.limitBehaviorUntilSwiftVersion(behavior, 6);
52525260
overridden->diagnose(diag::overridden_here);
52535261
}
52545262

@@ -5346,7 +5354,7 @@ static bool checkSendableInstanceStorage(
53465354
&& SendableCheckContext(dc, check)
53475355
.defaultDiagnosticBehavior() != DiagnosticBehavior::Ignore) {
53485356
sd->diagnose(diag::sendable_raw_storage, sd->getName())
5349-
.limitBehavior(behavior);
5357+
.limitBehaviorUntilSwiftVersion(behavior, 6);
53505358
}
53515359
return true;
53525360
}
@@ -5381,7 +5389,7 @@ static bool checkSendableInstanceStorage(
53815389
property
53825390
->diagnose(diag::concurrent_value_class_mutable_property,
53835391
property->getName(), nominal)
5384-
.limitBehavior(behavior);
5392+
.limitBehaviorUntilSwiftVersion(behavior, 6);
53855393
}
53865394
invalid = invalid || (behavior == DiagnosticBehavior::Unspecified);
53875395
return true;
@@ -5393,10 +5401,13 @@ static bool checkSendableInstanceStorage(
53935401
}
53945402

53955403
// Check that the property type is Sendable.
5404+
SendableCheckContext context(dc, check);
53965405
diagnoseNonSendableTypes(
5397-
propertyType, SendableCheckContext(dc, check),
5406+
propertyType, context,
53985407
/*inDerivedConformance*/Type(), property->getLoc(),
53995408
[&](Type type, DiagnosticBehavior behavior) {
5409+
auto preconcurrency =
5410+
context.preconcurrencyBehavior(type->getAnyNominal());
54005411
if (isImplicitSendableCheck(check)) {
54015412
// If this is for an externally-visible conformance, fail.
54025413
if (check == SendableCheck::ImplicitForExternallyVisible) {
@@ -5405,8 +5416,9 @@ static bool checkSendableInstanceStorage(
54055416
}
54065417

54075418
// If we are to ignore this diagnostic, just continue.
5408-
if (behavior == DiagnosticBehavior::Ignore)
5409-
return false;
5419+
if (behavior == DiagnosticBehavior::Ignore ||
5420+
preconcurrency == DiagnosticBehavior::Ignore)
5421+
return true;
54105422

54115423
invalid = true;
54125424
return true;
@@ -5415,7 +5427,8 @@ static bool checkSendableInstanceStorage(
54155427
property->diagnose(diag::non_concurrent_type_member,
54165428
propertyType, false, property->getName(),
54175429
nominal)
5418-
.limitBehavior(behavior);
5430+
.limitBehaviorUntilSwiftVersion(behavior, 6)
5431+
.limitBehaviorIf(preconcurrency);
54195432
return false;
54205433
});
54215434

@@ -5430,10 +5443,13 @@ static bool checkSendableInstanceStorage(
54305443

54315444
/// Handle an enum associated value.
54325445
bool operator()(EnumElementDecl *element, Type elementType) override {
5446+
SendableCheckContext context (dc, check);
54335447
diagnoseNonSendableTypes(
5434-
elementType, SendableCheckContext(dc, check),
5448+
elementType, context,
54355449
/*inDerivedConformance*/Type(), element->getLoc(),
54365450
[&](Type type, DiagnosticBehavior behavior) {
5451+
auto preconcurrency =
5452+
context.preconcurrencyBehavior(elementType->getAnyNominal());
54375453
if (isImplicitSendableCheck(check)) {
54385454
// If this is for an externally-visible conformance, fail.
54395455
if (check == SendableCheck::ImplicitForExternallyVisible) {
@@ -5442,7 +5458,8 @@ static bool checkSendableInstanceStorage(
54425458
}
54435459

54445460
// If we are to ignore this diagnostic, just continue.
5445-
if (behavior == DiagnosticBehavior::Ignore)
5461+
if (behavior == DiagnosticBehavior::Ignore ||
5462+
preconcurrency == DiagnosticBehavior::Ignore)
54465463
return false;
54475464

54485465
invalid = true;
@@ -5451,7 +5468,8 @@ static bool checkSendableInstanceStorage(
54515468

54525469
element->diagnose(diag::non_concurrent_type_member, type,
54535470
true, element->getName(), nominal)
5454-
.limitBehavior(behavior);
5471+
.limitBehaviorUntilSwiftVersion(behavior, 6)
5472+
.limitBehaviorIf(preconcurrency);
54555473
return false;
54565474
});
54575475

@@ -5510,7 +5528,7 @@ bool swift::checkSendableConformance(
55105528
nominal->getOutermostParentSourceFile()) {
55115529
conformanceDecl->diagnose(diag::concurrent_value_outside_source_file,
55125530
nominal)
5513-
.limitBehavior(behavior);
5531+
.limitBehaviorUntilSwiftVersion(behavior, 6);
55145532

55155533
if (behavior == DiagnosticBehavior::Unspecified)
55165534
return true;
@@ -5523,7 +5541,7 @@ bool swift::checkSendableConformance(
55235541
if (!classDecl->isSemanticallyFinal()) {
55245542
classDecl->diagnose(diag::concurrent_value_nonfinal_class,
55255543
classDecl->getName())
5526-
.limitBehavior(behavior);
5544+
.limitBehaviorUntilSwiftVersion(behavior, 6);
55275545

55285546
if (behavior == DiagnosticBehavior::Unspecified)
55295547
return true;
@@ -5538,7 +5556,7 @@ bool swift::checkSendableConformance(
55385556
->diagnose(diag::concurrent_value_inherit,
55395557
nominal->getASTContext().LangOpts.EnableObjCInterop,
55405558
classDecl->getName())
5541-
.limitBehavior(behavior);
5559+
.limitBehaviorUntilSwiftVersion(behavior, 6);
55425560

55435561
if (behavior == DiagnosticBehavior::Unspecified)
55445562
return true;

‎lib/Sema/TypeCheckConcurrency.h‎

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -387,6 +387,9 @@ struct SendableCheckContext {
387387
/// type in this context.
388388
DiagnosticBehavior diagnosticBehavior(NominalTypeDecl *nominal) const;
389389

390+
llvm::Optional<DiagnosticBehavior>
391+
preconcurrencyBehavior(Decl *decl) const;
392+
390393
/// Whether we are in an explicit conformance to Sendable.
391394
bool isExplicitSendableConformance() const;
392395
};
@@ -434,13 +437,15 @@ bool diagnoseNonSendableTypes(
434437
return diagnoseNonSendableTypes(
435438
type, fromContext, derivedConformance, typeLoc,
436439
[&](Type specificType, DiagnosticBehavior behavior) {
440+
auto preconcurrency =
441+
fromContext.preconcurrencyBehavior(type->getAnyNominal());
437442

438-
if (behavior != DiagnosticBehavior::Ignore) {
439-
ctx.Diags.diagnose(diagnoseLoc, diag, type, diagArgs...)
440-
.limitBehavior(behavior);
441-
}
443+
ctx.Diags.diagnose(diagnoseLoc, diag, type, diagArgs...)
444+
.limitBehaviorUntilSwiftVersion(behavior, 6)
445+
.limitBehaviorIf(preconcurrency);
442446

443-
return false;
447+
return (behavior == DiagnosticBehavior::Ignore ||
448+
preconcurrency == DiagnosticBehavior::Ignore);
444449
});
445450
}
446451

0 commit comments

Comments
 (0)