Merge pull request #41352 from DougGregor/public-frozen-sendable-preconcurrency-fix-5.6

Limit inference of Sendable for public, frozen types relying on preconcurrency

Author: DougGregor

lib/Sema/TypeCheckConcurrency.cpp

@@ -641,6 +641,7 @@ bool SendableCheckContext::isExplicitSendableConformance() const {
 
   case SendableCheck::ImpliedByStandardProtocol:
   case SendableCheck::Implicit:
+  case SendableCheck::ImplicitForExternallyVisible:
     return false;
   }
 }
@@ -754,7 +755,7 @@ DiagnosticBehavior SendableCheckContext::diagnosticBehavior(
   // enclosing inferred types non-Sendable.
   if (defaultBehavior == DiagnosticBehavior::Ignore &&
       nominal->getParentSourceFile() &&
-      conformanceCheck && *conformanceCheck == SendableCheck::Implicit)
+      conformanceCheck && isImplicitSendableCheck(*conformanceCheck))
     return DiagnosticBehavior::Warning;
 
   return defaultBehavior;
@@ -3877,7 +3878,7 @@ 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 (isImplicitSendableCheck(check)) {
           invalid = true;
           return true;
         }
@@ -3898,8 +3899,14 @@ static bool checkSendableInstanceStorage(
       diagnoseNonSendableTypes(
           propertyType, SendableCheckContext(dc, check), property->getLoc(),
           [&](Type type, DiagnosticBehavior behavior) {
-            if (check == SendableCheck::Implicit) {
-              // If we are to ignore this diagnose, just continue.
+            if (isImplicitSendableCheck(check)) {
+              // If this is for an externally-visible conformance, fail.
+              if (check == SendableCheck::ImplicitForExternallyVisible) {
+                invalid = true;
+                return true;
+              }
+
+              // If we are to ignore this diagnostic, just continue.
               if (behavior == DiagnosticBehavior::Ignore)
                 return false;
 
@@ -3917,7 +3924,7 @@ static bool checkSendableInstanceStorage(
 
       if (invalid) {
         // For implicit checks, bail out early if anything failed.
-        if (check == SendableCheck::Implicit)
+        if (isImplicitSendableCheck(check))
           return true;
       }
 
@@ -3929,8 +3936,14 @@ static bool checkSendableInstanceStorage(
       diagnoseNonSendableTypes(
           elementType, SendableCheckContext(dc, check), element->getLoc(),
           [&](Type type, DiagnosticBehavior behavior) {
-            if (check == SendableCheck::Implicit) {
-              // If we are to ignore this diagnose, just continue.
+            if (isImplicitSendableCheck(check)) {
+              // If this is for an externally-visible conformance, fail.
+              if (check == SendableCheck::ImplicitForExternallyVisible) {
+                invalid = true;
+                return true;
+              }
+
+              // If we are to ignore this diagnostic, just continue.
               if (behavior == DiagnosticBehavior::Ignore)
                 return false;
 
@@ -3948,7 +3961,7 @@ static bool checkSendableInstanceStorage(
 
       if (invalid) {
         // For implicit checks, bail out early if anything failed.
-        if (check == SendableCheck::Implicit)
+        if (isImplicitSendableCheck(check))
           return true;
       }
 
@@ -4177,21 +4190,27 @@ ProtocolConformance *GetImplicitSendableRequest::evaluate(
   if (!isa<StructDecl>(nominal) && !isa<EnumDecl>(nominal))
     return nullptr;
 
-  // Public, non-frozen structs and enums defined in Swift don't get implicit
-  // Sendable conformances.
-  if (!nominal->getASTContext().LangOpts.EnableInferPublicSendable &&
-      nominal->getFormalAccessScope(
-          /*useDC=*/nullptr,
-          /*treatUsableFromInlineAsPublic=*/true).isPublic() &&
-      !(nominal->hasClangNode() ||
-        nominal->getAttrs().hasAttribute<FixedLayoutAttr>() ||
-        nominal->getAttrs().hasAttribute<FrozenAttr>())) {
+  SendableCheck check;
+
+  // Okay to infer Sendable conformance for non-public types or when
+  // specifically requested.
+  if (nominal->getASTContext().LangOpts.EnableInferPublicSendable ||
+      !nominal->getFormalAccessScope(
+          /*useDC=*/nullptr, /*treatUsableFromInlineAsPublic=*/true)
+            .isPublic()) {
+    check = SendableCheck::Implicit;
+  } else if (nominal->hasClangNode() ||
+             nominal->getAttrs().hasAttribute<FixedLayoutAttr>() ||
+             nominal->getAttrs().hasAttribute<FrozenAttr>()) {
+    // @_frozen public types can also infer Sendable, but be more careful here.
+    check = SendableCheck::ImplicitForExternallyVisible;
+  } else {
+    // No inference.
     return nullptr;
   }
 
   // Check the instance storage for Sendable conformance.
-  if (checkSendableInstanceStorage(
-          nominal, nominal, SendableCheck::Implicit))
+  if (checkSendableInstanceStorage(nominal, nominal, check))
     return nullptr;
 
   return formConformance(nullptr);

lib/Sema/TypeCheckConcurrency.h

@@ -265,8 +265,25 @@ enum class SendableCheck {
 
   /// Implicit conformance to Sendable.
   Implicit,
+
+  /// Implicit conformance to Sendable that would be externally-visible, i.e.,
+  /// for a public @_frozen type.
+  ImplicitForExternallyVisible,
 };
 
+/// Whether this sendable check is implicit.
+static inline bool isImplicitSendableCheck(SendableCheck check) {
+  switch (check) {
+  case SendableCheck::Explicit:
+  case SendableCheck::ImpliedByStandardProtocol:
+    return false;
+
+  case SendableCheck::Implicit:
+  case SendableCheck::ImplicitForExternallyVisible:
+    return true;
+  }
+}
+
 /// Describes the context in which a \c Sendable check occurs.
 struct SendableCheckContext {
   const DeclContext * const fromDC;

test/ModuleInterface/Inputs/preconcurrency.swift

@@ -0,0 +1 @@
+public class NotSendable { }

test/ModuleInterface/actor_isolation.swift

@@ -1,14 +1,17 @@
 // RUN: %empty-directory(%t)
-// RUN: %target-swift-frontend -emit-module -o %t/Test.swiftmodule -emit-module-interface-path %t/Test.swiftinterface -module-name Test -enable-experimental-concurrency %s
+// RUN: %target-swift-frontend -emit-module -o %t/Preconcurrency.swiftmodule -module-name Preconcurrency %S/Inputs/preconcurrency.swift
+
+// RUN: %target-swift-frontend -emit-module -o %t/Test.swiftmodule -emit-module-interface-path %t/Test.swiftinterface -module-name Test -enable-experimental-concurrency -I %t %s
 // RUN: %FileCheck %s < %t/Test.swiftinterface
 // RUN: %FileCheck %s -check-prefix SYNTHESIZED < %t/Test.swiftinterface
-// RUN: %target-swift-frontend -typecheck-module-from-interface -module-name Test %t/Test.swiftinterface
+// RUN: %target-swift-frontend -typecheck-module-from-interface -module-name Test %t/Test.swiftinterface -I %t 
 
-// RUN: %target-swift-frontend -emit-module -o /dev/null -merge-modules %t/Test.swiftmodule -disable-objc-attr-requires-foundation-module -emit-module-interface-path %t/TestFromModule.swiftinterface -module-name Test -enable-experimental-concurrency
+// RUN: %target-swift-frontend -emit-module -o /dev/null -merge-modules %t/Test.swiftmodule -disable-objc-attr-requires-foundation-module -emit-module-interface-path %t/TestFromModule.swiftinterface -module-name Test -enable-experimental-concurrency -I %t 
 // RUN: %FileCheck %s < %t/TestFromModule.swiftinterface
-// RUN: %target-swift-frontend -typecheck-module-from-interface -module-name Test %t/TestFromModule.swiftinterface
+// RUN: %target-swift-frontend -typecheck-module-from-interface -module-name Test %t/TestFromModule.swiftinterface -I %t 
 
 // REQUIRES: concurrency
+import Preconcurrency
 
 // CHECK: public actor SomeActor
 
@@ -97,6 +100,13 @@ public class C8 : P2 {
   public func method() {}
 }
 
+// CHECK-NOT: StructWithImplicitlyNonSendable{{.*}}Sendable
+@available(SwiftStdlib 5.1, *)
+@_frozen
+public struct StructWithImplicitlyNonSendable {
+  var ns: NotSendable? = nil
+}
+
 // FIXME: Work around a bug where module printing depends on the "synthesized"
 // bit in conformances which is not serialized and not present in the textual
 // form.