Skip to content

Commit c036b5b

Browse files
DougGregorDougGregor
authored
Merge pull request swiftlang#41150 from DougGregor/preconcurrency-diagnostic-fixes
Make @preconcurrency suppress Sendable diagnostics more reliably.
2 parents 925ad7b + 5d59fe0 commit c036b5b

File tree

6 files changed

+164
-23
lines changed

6 files changed

+164
-23
lines changed

‎lib/Sema/TypeCheckConcurrency.cpp‎

Lines changed: 38 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -718,18 +718,20 @@ DiagnosticBehavior SendableCheckContext::diagnosticBehavior(
718718
// Determine whether the type was explicitly non-Sendable.
719719
auto nominalModule = nominal->getParentModule();
720720
bool isExplicitlyNonSendable = nominalModule->isConcurrencyChecked() ||
721-
isExplicitSendableConformance() ||
722721
hasExplicitSendableConformance(nominal);
723722

724723
// Determine whether this nominal type is visible via a @preconcurrency
725724
// import.
726725
auto import = findImportFor(nominal, fromDC);
727726

728727
// When the type is explicitly non-Sendable...
728+
auto sourceFile = fromDC->getParentSourceFile();
729729
if (isExplicitlyNonSendable) {
730-
// @preconcurrency imports downgrade the diagnostic to a warning.
730+
// @preconcurrency imports downgrade the diagnostic to a warning in Swift 6,
731731
if (import && import->options.contains(ImportFlags::Preconcurrency)) {
732-
// FIXME: Note that this @preconcurrency import was "used".
732+
if (sourceFile)
733+
sourceFile->setImportUsedPreconcurrency(*import);
734+
733735
return DiagnosticBehavior::Warning;
734736
}
735737

@@ -741,13 +743,25 @@ DiagnosticBehavior SendableCheckContext::diagnosticBehavior(
741743
// @preconcurrency suppresses the diagnostic in Swift 5.x, and
742744
// downgrades it to a warning in Swift 6 and later.
743745
if (import && import->options.contains(ImportFlags::Preconcurrency)) {
744-
// FIXME: Note that this @preconcurrency import was "used".
746+
if (sourceFile)
747+
sourceFile->setImportUsedPreconcurrency(*import);
748+
745749
return nominalModule->getASTContext().LangOpts.isSwiftVersionAtLeast(6)
746750
? DiagnosticBehavior::Warning
747751
: DiagnosticBehavior::Ignore;
748752
}
749753

750-
return defaultDiagnosticBehavior();
754+
auto defaultBehavior = defaultDiagnosticBehavior();
755+
756+
// If we are checking an implicit Sendable conformance, don't suppress
757+
// diagnostics for declarations in the same module. We want them so make
758+
// enclosing inferred types non-Sendable.
759+
if (defaultBehavior == DiagnosticBehavior::Ignore &&
760+
nominal->getParentSourceFile() &&
761+
conformanceCheck && *conformanceCheck == SendableCheck::Implicit)
762+
return DiagnosticBehavior::Warning;
763+
764+
return defaultBehavior;
751765
}
752766

753767
/// Produce a diagnostic for a single instance of a non-Sendable type where
@@ -769,14 +783,6 @@ static bool diagnoseSingleNonSendableType(
769783

770784
bool wasSuppressed = diagnose(type, behavior);
771785

772-
// If this type was imported from another module, try to find the
773-
// corresponding import.
774-
Optional<AttributedImport<swift::ImportedModule>> import;
775-
SourceFile *sourceFile = fromContext.fromDC->getParentSourceFile();
776-
if (nominal && nominal->getParentModule() != module) {
777-
import = findImportFor(nominal, fromContext.fromDC);
778-
}
779-
780786
if (behavior == DiagnosticBehavior::Ignore || wasSuppressed) {
781787
// Don't emit any other diagnostics.
782788
} else if (type->is<FunctionType>()) {
@@ -800,6 +806,14 @@ static bool diagnoseSingleNonSendableType(
800806
diag::non_sendable_nominal, nominal->getDescriptiveKind(),
801807
nominal->getName());
802808

809+
// This type was imported from another module; try to find the
810+
// corresponding import.
811+
Optional<AttributedImport<swift::ImportedModule>> import;
812+
SourceFile *sourceFile = fromContext.fromDC->getParentSourceFile();
813+
if (sourceFile) {
814+
import = findImportFor(nominal, fromContext.fromDC);
815+
}
816+
803817
// If we found the import that makes this nominal type visible, remark
804818
// that it can be @preconcurrency import.
805819
// Only emit this remark once per source file, because it can happen a
@@ -818,14 +832,6 @@ static bool diagnoseSingleNonSendableType(
818832
}
819833
}
820834

821-
// If we found an import that makes this nominal type visible, and that
822-
// was a @preconcurrency import, note that we have made use of the
823-
// attribute.
824-
if (import && import->options.contains(ImportFlags::Preconcurrency) &&
825-
sourceFile) {
826-
sourceFile->setImportUsedPreconcurrency(*import);
827-
}
828-
829835
return behavior == DiagnosticBehavior::Unspecified && !wasSuppressed;
830836
}
831837

@@ -3887,8 +3893,10 @@ static bool checkSendableInstanceStorage(
38873893
bool operator()(VarDecl *property, Type propertyType) {
38883894
// Classes with mutable properties are not Sendable.
38893895
if (property->supportsMutation() && isa<ClassDecl>(nominal)) {
3890-
if (check == SendableCheck::Implicit)
3896+
if (check == SendableCheck::Implicit) {
3897+
invalid = true;
38913898
return true;
3899+
}
38923900

38933901
auto behavior = SendableCheckContext(
38943902
dc, check).defaultDiagnosticBehavior();
@@ -3907,6 +3915,10 @@ static bool checkSendableInstanceStorage(
39073915
propertyType, SendableCheckContext(dc, check), property->getLoc(),
39083916
[&](Type type, DiagnosticBehavior behavior) {
39093917
if (check == SendableCheck::Implicit) {
3918+
// If we are to ignore this diagnose, just continue.
3919+
if (behavior == DiagnosticBehavior::Ignore)
3920+
return false;
3921+
39103922
invalid = true;
39113923
return true;
39123924
}
@@ -3934,6 +3946,10 @@ static bool checkSendableInstanceStorage(
39343946
elementType, SendableCheckContext(dc, check), element->getLoc(),
39353947
[&](Type type, DiagnosticBehavior behavior) {
39363948
if (check == SendableCheck::Implicit) {
3949+
// If we are to ignore this diagnose, just continue.
3950+
if (behavior == DiagnosticBehavior::Ignore)
3951+
return false;
3952+
39373953
invalid = true;
39383954
return true;
39393955
}

‎test/Concurrency/actor_isolation_unsafe.swift‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ struct S3_P1: P1 {
2929
nonisolated func onMainActor() { }
3030
}
3131

32-
struct S4_P1_quitely: P1 {
32+
struct S4_P1_quietly: P1 {
3333
@SomeGlobalActor func onMainActor() { }
3434
}
3535

‎test/Concurrency/sendable_preconcurrency.swift‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
// RUN: %empty-directory(%t)
2+
// RUN: %target-swift-frontend -emit-module -emit-module-path %t/StrictModule.swiftmodule -module-name StrictModule -warn-concurrency %S/Inputs/StrictModule.swift
3+
// RUN: %target-swift-frontend -emit-module -emit-module-path %t/NonStrictModule.swiftmodule -module-name NonStrictModule %S/Inputs/NonStrictModule.swift
4+
// RUN: %target-typecheck-verify-swift -disable-availability-checking -I %t
5+
6+
// REQUIRES: concurrency
7+
8+
import StrictModule
9+
@preconcurrency import NonStrictModule
10+
11+
actor A {
12+
func f() -> [StrictStruct: NonStrictClass] { [:] }
13+
}
14+
15+
class NS { } // expected-note 2{{class 'NS' does not conform to the 'Sendable' protocol}}
16+
17+
struct MyType {
18+
var nsc: NonStrictClass
19+
}
20+
21+
struct MyType2: Sendable {
22+
var nsc: NonStrictClass // no warning; @preconcurrency suppressed it
23+
var ns: NS // expected-warning{{stored property 'ns' of 'Sendable'-conforming struct 'MyType2' has non-sendable type 'NS'}}
24+
}
25+
26+
struct MyType3 {
27+
var nsc: NonStrictClass
28+
}
29+
30+
func testA(ns: NS, mt: MyType, mt2: MyType2, mt3: MyType3) async {
31+
Task {
32+
print(ns) // expected-warning{{capture of 'ns' with non-sendable type 'NS' in a `@Sendable` closure}}
33+
print(mt) // no warning: MyType is Sendable because we suppressed NonStrictClass's warning
34+
print(mt2)
35+
print(mt3)
36+
}
37+
}
38+
39+
extension NonStrictStruct: @unchecked Sendable { }

‎test/Concurrency/sendable_preconcurrency_unused.swift‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
// RUN: %empty-directory(%t)
2+
// RUN: %target-swift-frontend -emit-module -emit-module-path %t/StrictModule.swiftmodule -module-name StrictModule -warn-concurrency %S/Inputs/StrictModule.swift
3+
// RUN: %target-swift-frontend -emit-module -emit-module-path %t/NonStrictModule.swiftmodule -module-name NonStrictModule %S/Inputs/NonStrictModule.swift
4+
// RUN: %target-typecheck-verify-swift -disable-availability-checking -I %t
5+
6+
// REQUIRES: concurrency
7+
8+
@preconcurrency import NonStrictModule
9+
10+
struct MyType {
11+
var nsc: NonStrictClass
12+
}
13+
14+
func test(mt: MyType, nsc: NonStrictClass) {
15+
Task {
16+
print(mt)
17+
}
18+
}

‎test/Concurrency/sendable_without_preconcurrency.swift‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
// RUN: %empty-directory(%t)
2+
// RUN: %target-swift-frontend -emit-module -emit-module-path %t/StrictModule.swiftmodule -module-name StrictModule -warn-concurrency %S/Inputs/StrictModule.swift
3+
// RUN: %target-swift-frontend -emit-module -emit-module-path %t/NonStrictModule.swiftmodule -module-name NonStrictModule %S/Inputs/NonStrictModule.swift
4+
// RUN: %target-typecheck-verify-swift -disable-availability-checking -I %t
5+
6+
// REQUIRES: concurrency
7+
8+
import StrictModule
9+
import NonStrictModule
10+
11+
actor A {
12+
func f() -> [StrictStruct: NonStrictClass] { [:] }
13+
}
14+
15+
class NS { } // expected-note{{class 'NS' does not conform to the 'Sendable' protocol}}
16+
17+
struct MyType {
18+
var nsc: NonStrictClass
19+
}
20+
21+
struct MyType2 { // expected-note{{consider making struct 'MyType2' conform to the 'Sendable' protocol}}
22+
var nsc: NonStrictClass
23+
var ns: NS
24+
}
25+
26+
func testA(ns: NS, mt: MyType, mt2: MyType2) async {
27+
Task {
28+
print(ns) // expected-warning{{capture of 'ns' with non-sendable type 'NS' in a `@Sendable` closure}}
29+
print(mt) // no warning: MyType is Sendable because we suppressed NonStrictClass's warning
30+
print(mt2) // expected-warning{{capture of 'mt2' with non-sendable type 'MyType2' in a `@Sendable` closure}}
31+
}
32+
}
33+
34+
extension NonStrictStruct: @unchecked Sendable { }

‎test/Concurrency/sendable_without_preconcurrency_2.swift‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
// RUN: %empty-directory(%t)
2+
// RUN: %target-swift-frontend -emit-module -emit-module-path %t/StrictModule.swiftmodule -module-name StrictModule -warn-concurrency %S/Inputs/StrictModule.swift
3+
// RUN: %target-swift-frontend -emit-module -emit-module-path %t/NonStrictModule.swiftmodule -module-name NonStrictModule %S/Inputs/NonStrictModule.swift
4+
// RUN: %target-typecheck-verify-swift -disable-availability-checking -I %t
5+
6+
// REQUIRES: concurrency
7+
8+
import StrictModule
9+
import NonStrictModule // expected-remark{{add '@preconcurrency' to suppress 'Sendable'-related warnings from module 'NonStrictModule'}}
10+
11+
actor A {
12+
func f() -> [StrictStruct: NonStrictClass] { [:] }
13+
}
14+
15+
class NS { } // expected-note 2{{class 'NS' does not conform to the 'Sendable' protocol}}
16+
17+
struct MyType {
18+
var nsc: NonStrictClass
19+
}
20+
21+
struct MyType2: Sendable {
22+
var nsc: NonStrictClass // expected-warning{{stored property 'nsc' of 'Sendable'-conforming struct 'MyType2' has non-sendable type 'NonStrictClass'}}
23+
var ns: NS // expected-warning{{stored property 'ns' of 'Sendable'-conforming struct 'MyType2' has non-sendable type 'NS'}}
24+
}
25+
26+
func testA(ns: NS, mt: MyType, mt2: MyType2) async {
27+
Task {
28+
print(ns) // expected-warning{{capture of 'ns' with non-sendable type 'NS' in a `@Sendable` closure}}
29+
print(mt) // no warning: MyType is Sendable because we suppressed NonStrictClass's warning
30+
print(mt2)
31+
}
32+
}
33+
34+
extension NonStrictStruct: @unchecked Sendable { }

0 commit comments

Comments
 (0)