Merge pull request #41151 from DougGregor/preconcurrency-diagnostic-fixes-5.6

DougGregor · web-flow · commit e744f41dde45 · 2022-02-02T20:54:49.000-08:00
diff --git a/lib/Sema/TypeCheckConcurrency.cpp b/lib/Sema/TypeCheckConcurrency.cpp
@@ -714,18 +714,20 @@ DiagnosticBehavior SendableCheckContext::diagnosticBehavior(
   // Determine whether the type was explicitly non-Sendable.
   auto nominalModule = nominal->getParentModule();
   bool isExplicitlyNonSendable = nominalModule->isConcurrencyChecked() ||
-      isExplicitSendableConformance() ||
       hasExplicitSendableConformance(nominal);
 
   // Determine whether this nominal type is visible via a @preconcurrency
   // import.
   auto import = findImportFor(nominal, fromDC);
 
   // When the type is explicitly non-Sendable...
+  auto sourceFile = fromDC->getParentSourceFile();
   if (isExplicitlyNonSendable) {
-    // @preconcurrency imports downgrade the diagnostic to a warning.
+    // @preconcurrency imports downgrade the diagnostic to a warning in Swift 6,
     if (import && import->options.contains(ImportFlags::Preconcurrency)) {
-      // FIXME: Note that this @preconcurrency import was "used".
+      if (sourceFile)
+        sourceFile->setImportUsedPreconcurrency(*import);
+
       return DiagnosticBehavior::Warning;
     }
 
@@ -737,13 +739,25 @@ DiagnosticBehavior SendableCheckContext::diagnosticBehavior(
   // @preconcurrency suppresses the diagnostic in Swift 5.x, and
   // downgrades it to a warning in Swift 6 and later.
   if (import && import->options.contains(ImportFlags::Preconcurrency)) {
-    // FIXME: Note that this @preconcurrency import was "used".
+    if (sourceFile)
+      sourceFile->setImportUsedPreconcurrency(*import);
+
     return nominalModule->getASTContext().LangOpts.isSwiftVersionAtLeast(6)
         ? DiagnosticBehavior::Warning
         : DiagnosticBehavior::Ignore;
   }
 
-  return defaultDiagnosticBehavior();
+  auto defaultBehavior = defaultDiagnosticBehavior();
+
+  // If we are checking an implicit Sendable conformance, don't suppress
+  // diagnostics for declarations in the same module. We want them so make
+  // enclosing inferred types non-Sendable.
+  if (defaultBehavior == DiagnosticBehavior::Ignore &&
+      nominal->getParentSourceFile() &&
+      conformanceCheck && *conformanceCheck == SendableCheck::Implicit)
+    return DiagnosticBehavior::Warning;
+
+  return defaultBehavior;
 }
 
 /// Produce a diagnostic for a single instance of a non-Sendable type where
@@ -765,14 +779,6 @@ static bool diagnoseSingleNonSendableType(
 
   bool wasSuppressed = diagnose(type, behavior);
 
-  // If this type was imported from another module, try to find the
-  // corresponding import.
-  Optional<AttributedImport<swift::ImportedModule>> import;
-  SourceFile *sourceFile = fromContext.fromDC->getParentSourceFile();
-  if (nominal && nominal->getParentModule() != module) {
-    import = findImportFor(nominal, fromContext.fromDC);
-  }
-
   if (behavior == DiagnosticBehavior::Ignore || wasSuppressed) {
     // Don't emit any other diagnostics.
   } else if (type->is<FunctionType>()) {
@@ -796,6 +802,14 @@ static bool diagnoseSingleNonSendableType(
         diag::non_sendable_nominal, nominal->getDescriptiveKind(),
         nominal->getName());
 
+    // This type was imported from another module; try to find the
+    // corresponding import.
+    Optional<AttributedImport<swift::ImportedModule>> import;
+    SourceFile *sourceFile = fromContext.fromDC->getParentSourceFile();
+    if (sourceFile) {
+      import = findImportFor(nominal, fromContext.fromDC);
+    }
+
     // If we found the import that makes this nominal type visible, remark
     // that it can be @preconcurrency import.
     // Only emit this remark once per source file, because it can happen a
@@ -814,14 +828,6 @@ static bool diagnoseSingleNonSendableType(
     }
   }
 
-  // If we found an import that makes this nominal type visible, and that
-  // was a @preconcurrency import, note that we have made use of the
-  // attribute.
-  if (import && import->options.contains(ImportFlags::Preconcurrency) &&
-      sourceFile) {
-    sourceFile->setImportUsedPreconcurrency(*import);
-  }
-
   return behavior == DiagnosticBehavior::Unspecified && !wasSuppressed;
 }
 
@@ -3871,8 +3877,10 @@ static bool checkSendableInstanceStorage(
     bool operator()(VarDecl *property, Type propertyType) {
       // Classes with mutable properties are not Sendable.
       if (property->supportsMutation() && isa<ClassDecl>(nominal)) {
-        if (check == SendableCheck::Implicit)
+        if (check == SendableCheck::Implicit) {
+          invalid = true;
           return true;
+        }
 
         auto behavior = SendableCheckContext(
             dc, check).defaultDiagnosticBehavior();
@@ -3891,6 +3899,10 @@ static bool checkSendableInstanceStorage(
           propertyType, SendableCheckContext(dc, check), property->getLoc(),
           [&](Type type, DiagnosticBehavior behavior) {
             if (check == SendableCheck::Implicit) {
+              // If we are to ignore this diagnose, just continue.
+              if (behavior == DiagnosticBehavior::Ignore)
+                return false;
+
               invalid = true;
               return true;
             }
@@ -3918,6 +3930,10 @@ static bool checkSendableInstanceStorage(
           elementType, SendableCheckContext(dc, check), element->getLoc(),
           [&](Type type, DiagnosticBehavior behavior) {
             if (check == SendableCheck::Implicit) {
+              // If we are to ignore this diagnose, just continue.
+              if (behavior == DiagnosticBehavior::Ignore)
+                return false;
+
               invalid = true;
               return true;
             }
diff --git a/test/Concurrency/sendable_preconcurrency.swift b/test/Concurrency/sendable_preconcurrency.swift
@@ -0,0 +1,39 @@
+// RUN: %empty-directory(%t)
+// RUN: %target-swift-frontend -emit-module -emit-module-path %t/StrictModule.swiftmodule -module-name StrictModule -warn-concurrency %S/Inputs/StrictModule.swift
+// RUN: %target-swift-frontend -emit-module -emit-module-path %t/NonStrictModule.swiftmodule -module-name NonStrictModule %S/Inputs/NonStrictModule.swift
+// RUN: %target-typecheck-verify-swift -disable-availability-checking -I %t
+
+// REQUIRES: concurrency
+
+import StrictModule
+@preconcurrency import NonStrictModule
+
+actor A {
+  func f() -> [StrictStruct: NonStrictClass] { [:] }
+}
+
+class NS { } // expected-note 2{{class 'NS' does not conform to the 'Sendable' protocol}}
+
+struct MyType {
+  var nsc: NonStrictClass
+}
+
+struct MyType2: Sendable {
+  var nsc: NonStrictClass // no warning; @preconcurrency suppressed it
+  var ns: NS // expected-warning{{stored property 'ns' of 'Sendable'-conforming struct 'MyType2' has non-sendable type 'NS'}}
+}
+
+struct MyType3 {
+  var nsc: NonStrictClass
+}
+
+func testA(ns: NS, mt: MyType, mt2: MyType2, mt3: MyType3) async {
+  Task {
+    print(ns) // expected-warning{{capture of 'ns' with non-sendable type 'NS' in a `@Sendable` closure}}
+    print(mt) // no warning: MyType is Sendable because we suppressed NonStrictClass's warning
+    print(mt2)
+    print(mt3)
+  }
+}
+
+extension NonStrictStruct: @unchecked Sendable { }
diff --git a/test/Concurrency/sendable_preconcurrency_unused.swift b/test/Concurrency/sendable_preconcurrency_unused.swift
@@ -0,0 +1,18 @@
+// RUN: %empty-directory(%t)
+// RUN: %target-swift-frontend -emit-module -emit-module-path %t/StrictModule.swiftmodule -module-name StrictModule -warn-concurrency %S/Inputs/StrictModule.swift
+// RUN: %target-swift-frontend -emit-module -emit-module-path %t/NonStrictModule.swiftmodule -module-name NonStrictModule %S/Inputs/NonStrictModule.swift
+// RUN: %target-typecheck-verify-swift -disable-availability-checking -I %t
+
+// REQUIRES: concurrency
+
+@preconcurrency import NonStrictModule
+
+struct MyType {
+  var nsc: NonStrictClass
+}
+
+func test(mt: MyType, nsc: NonStrictClass) {
+  Task {
+    print(mt)
+  }
+}
diff --git a/test/Concurrency/sendable_without_preconcurrency.swift b/test/Concurrency/sendable_without_preconcurrency.swift
@@ -0,0 +1,34 @@
+// RUN: %empty-directory(%t)
+// RUN: %target-swift-frontend -emit-module -emit-module-path %t/StrictModule.swiftmodule -module-name StrictModule -warn-concurrency %S/Inputs/StrictModule.swift
+// RUN: %target-swift-frontend -emit-module -emit-module-path %t/NonStrictModule.swiftmodule -module-name NonStrictModule %S/Inputs/NonStrictModule.swift
+// RUN: %target-typecheck-verify-swift -disable-availability-checking -I %t
+
+// REQUIRES: concurrency
+
+import StrictModule
+import NonStrictModule
+
+actor A {
+  func f() -> [StrictStruct: NonStrictClass] { [:] }
+}
+
+class NS { } // expected-note{{class 'NS' does not conform to the 'Sendable' protocol}}
+
+struct MyType {
+  var nsc: NonStrictClass
+}
+
+struct MyType2 { // expected-note{{consider making struct 'MyType2' conform to the 'Sendable' protocol}}
+  var nsc: NonStrictClass
+  var ns: NS
+}
+
+func testA(ns: NS, mt: MyType, mt2: MyType2) async {
+  Task {
+    print(ns) // expected-warning{{capture of 'ns' with non-sendable type 'NS' in a `@Sendable` closure}}
+    print(mt) // no warning: MyType is Sendable because we suppressed NonStrictClass's warning
+    print(mt2) // expected-warning{{capture of 'mt2' with non-sendable type 'MyType2' in a `@Sendable` closure}}
+  }
+}
+
+extension NonStrictStruct: @unchecked Sendable { }
diff --git a/test/Concurrency/sendable_without_preconcurrency_2.swift b/test/Concurrency/sendable_without_preconcurrency_2.swift
@@ -0,0 +1,34 @@
+// RUN: %empty-directory(%t)
+// RUN: %target-swift-frontend -emit-module -emit-module-path %t/StrictModule.swiftmodule -module-name StrictModule -warn-concurrency %S/Inputs/StrictModule.swift
+// RUN: %target-swift-frontend -emit-module -emit-module-path %t/NonStrictModule.swiftmodule -module-name NonStrictModule %S/Inputs/NonStrictModule.swift
+// RUN: %target-typecheck-verify-swift -disable-availability-checking -I %t
+
+// REQUIRES: concurrency
+
+import StrictModule
+import NonStrictModule // expected-remark{{add '@preconcurrency' to suppress 'Sendable'-related warnings from module 'NonStrictModule'}}
+
+actor A {
+  func f() -> [StrictStruct: NonStrictClass] { [:] }
+}
+
+class NS { } // expected-note 2{{class 'NS' does not conform to the 'Sendable' protocol}}
+
+struct MyType {
+  var nsc: NonStrictClass
+}
+
+struct MyType2: Sendable {
+  var nsc: NonStrictClass // expected-warning{{stored property 'nsc' of 'Sendable'-conforming struct 'MyType2' has non-sendable type 'NonStrictClass'}}
+  var ns: NS // expected-warning{{stored property 'ns' of 'Sendable'-conforming struct 'MyType2' has non-sendable type 'NS'}}
+}
+
+func testA(ns: NS, mt: MyType, mt2: MyType2) async {
+  Task {
+    print(ns) // expected-warning{{capture of 'ns' with non-sendable type 'NS' in a `@Sendable` closure}}
+    print(mt) // no warning: MyType is Sendable because we suppressed NonStrictClass's warning
+    print(mt2)
+  }
+}
+
+extension NonStrictStruct: @unchecked Sendable { }
