Use new Sendable staging infrastructure consistently.

The main effect of this change is that diagnostics about Sendable
conformances now follow the same minimal/full logic used for other
Sendable diagnostics, rather than having their own separate
computation.

Author: DougGregor

include/swift/AST/DiagnosticsSema.def

@@ -4601,9 +4601,9 @@ WARNING(non_sendable_keypath_access,none,
         "cannot form key path that accesses non-sendable type %0",
         (Type))
 ERROR(non_concurrent_type_member,none,
-      "%select{stored property %1|associated value %1}0 of "
-      "'Sendable'-conforming %2 %3 has non-sendable type %4",
-      (bool, DeclName, DescriptiveDeclKind, DeclName, Type))
+      "%select{stored property %2|associated value %2}1 of "
+      "'Sendable'-conforming %3 %4 has non-sendable type %0",
+      (Type, bool, DeclName, DescriptiveDeclKind, DeclName))
 ERROR(concurrent_value_class_mutable_property,none,
       "stored property %0 of 'Sendable'-conforming %1 %2 is mutable",
       (DeclName, DescriptiveDeclKind, DeclName))

lib/Sema/TypeCheckConcurrency.cpp

@@ -618,10 +618,25 @@ static DiagnosticBehavior defaultSendableDiagnosticBehavior(
   return DiagnosticBehavior::Unspecified;
 }
 
+bool SendableCheckContext::isExplicitSendableConformance() const {
+  if (!conformanceCheck)
+    return false;
+
+  switch (*conformanceCheck) {
+  case SendableCheck::Explicit:
+    return true;
+
+  case SendableCheck::ImpliedByStandardProtocol:
+  case SendableCheck::Implicit:
+    return false;
+  }
+}
+
 DiagnosticBehavior SendableCheckContext::defaultDiagnosticBehavior() const {
   // If we're not supposed to diagnose existing data races from this context,
   // ignore the diagnostic entirely.
-  if (!shouldDiagnoseExistingDataRaces(fromDC))
+  if (!isExplicitSendableConformance() &&
+      !shouldDiagnoseExistingDataRaces(fromDC))
     return DiagnosticBehavior::Ignore;
 
   return defaultSendableDiagnosticBehavior(fromDC->getASTContext().LangOpts);
@@ -633,21 +648,8 @@ DiagnosticBehavior SendableCheckContext::diagnosticBehavior(
     NominalTypeDecl *nominal) const {
   // Determine whether the type was explicitly non-Sendable.
   auto nominalModule = nominal->getParentModule();
-  bool isExplicitlyNonSendable = nominalModule->isConcurrencyChecked();
-
-  // If we are performing an explicit conformance check, always consider this
-  // to be an explicitly non-Sendable type.
-  if (conformanceCheck) {
-    switch (*conformanceCheck) {
-    case SendableCheck::Explicit:
-      isExplicitlyNonSendable = true;
-      break;
-
-    case SendableCheck::ImpliedByStandardProtocol:
-    case SendableCheck::Implicit:
-      break;
-    }
-  }
+  bool isExplicitlyNonSendable = nominalModule->isConcurrencyChecked() ||
+      isExplicitSendableConformance();
 
   // Determine whether this nominal type is visible via a @_predatesConcurrency
   // import.
@@ -679,8 +681,7 @@ DiagnosticBehavior SendableCheckContext::diagnosticBehavior(
 /// a Sendable type is required.
 static bool diagnoseSingleNonSendableType(
     Type type, SendableCheckContext fromContext, SourceLoc loc,
-    llvm::function_ref<
-      std::pair<DiagnosticBehavior, bool>(Type, DiagnosticBehavior)> diagnose) {
+    llvm::function_ref<bool(Type, DiagnosticBehavior)> diagnose) {
 
   auto behavior = DiagnosticBehavior::Unspecified;
 
@@ -693,11 +694,9 @@ static bool diagnoseSingleNonSendableType(
     behavior = fromContext.defaultDiagnosticBehavior();
   }
 
-  DiagnosticBehavior actualBehavior;
-  bool wasError;
-  std::tie(actualBehavior, wasError) = diagnose(type, behavior);
+  bool wasSuppressed = diagnose(type, behavior);
 
-  if (actualBehavior == DiagnosticBehavior::Ignore) {
+  if (behavior == DiagnosticBehavior::Ignore || wasSuppressed) {
     // Don't emit any other diagnostics.
   } else if (type->is<FunctionType>()) {
     ctx.Diags.diagnose(loc, diag::nonsendable_function_type);
@@ -713,13 +712,12 @@ static bool diagnoseSingleNonSendableType(
         nominal->getName());
   }
 
-  return wasError;
+  return behavior == DiagnosticBehavior::Unspecified && !wasSuppressed;
 }
 
 bool swift::diagnoseNonSendableTypes(
     Type type, SendableCheckContext fromContext, SourceLoc loc,
-    llvm::function_ref<
-      std::pair<DiagnosticBehavior, bool>(Type, DiagnosticBehavior)> diagnose) {
+    llvm::function_ref<bool(Type, DiagnosticBehavior)> diagnose) {
   auto module = fromContext.fromDC->getParentModule();
 
   // If the Sendable protocol is missing, do nothing.
@@ -3640,44 +3638,6 @@ bool swift::contextUsesConcurrencyFeatures(const DeclContext *dc) {
   return false;
 }
 
-/// Limit the diagnostic behavior used when performing checks for the Sendable
-/// instance storage of Sendable types.
-///
-/// \returns a pair containing the diagnostic behavior that should be used
-/// for this diagnostic, as well as a Boolean value indicating whether to
-/// treat this as an error.
-static std::pair<DiagnosticBehavior, bool> limitSendableInstanceBehavior(
-    const LangOptions &langOpts, SendableCheck check,
-    DiagnosticBehavior suggestedBehavior) {
-  // Is an error suggested?
-  bool suggestedError = suggestedBehavior == DiagnosticBehavior::Unspecified ||
-      suggestedBehavior == DiagnosticBehavior::Error;
-  switch (check) {
-  case SendableCheck::Implicit:
-    // For implicit checks, we always ignore the diagnostic and fail.
-    return std::make_pair(DiagnosticBehavior::Ignore, true);
-
-  case SendableCheck::Explicit:
-    // Bump warnings up to errors due to explicit Sendable conformance.
-    if (suggestedBehavior == DiagnosticBehavior::Warning)
-      return std::make_pair(DiagnosticBehavior::Unspecified, true);
-
-    return std::make_pair(suggestedBehavior, suggestedError);
-
-  case SendableCheck::ImpliedByStandardProtocol:
-    // If we aren't in Swift 6, downgrade diagnostics.
-    if (!langOpts.isSwiftVersionAtLeast(6)) {
-      if (langOpts.WarnConcurrency &&
-          suggestedBehavior != DiagnosticBehavior::Ignore)
-        return std::make_pair(DiagnosticBehavior::Warning, false);
-
-      return std::make_pair(DiagnosticBehavior::Ignore, false);
-    }
-
-    return std::make_pair(suggestedBehavior, suggestedError);
-  }
-}
-
 namespace {
   /// Visit the instance storage of the given nominal type as seen through
   /// the given declaration context.
@@ -3749,38 +3709,36 @@ static bool checkSendableInstanceStorage(
         if (check == SendableCheck::Implicit)
           return true;
 
-        auto action = limitSendableInstanceBehavior(
-            langOpts, check, DiagnosticBehavior::Unspecified);
-
-        property->diagnose(diag::concurrent_value_class_mutable_property,
-                           property->getName(), nominal->getDescriptiveKind(),
-                           nominal->getName())
-            .limitBehavior(action.first);
-        invalid = invalid || action.second;
+        auto behavior = SendableCheckContext(
+            dc, check).defaultDiagnosticBehavior();
+        if (behavior != DiagnosticBehavior::Ignore) {
+          property->diagnose(diag::concurrent_value_class_mutable_property,
+                             property->getName(), nominal->getDescriptiveKind(),
+                             nominal->getName())
+              .limitBehavior(behavior);
+        }
+        invalid = invalid || (behavior == DiagnosticBehavior::Unspecified);
         return true;
       }
 
       // Check that the property type is Sendable.
-      bool diagnosedProperty = diagnoseNonSendableTypes(
+      diagnoseNonSendableTypes(
           propertyType, SendableCheckContext(dc, check), property->getLoc(),
-          [&](Type type, DiagnosticBehavior suggestedBehavior) {
-            auto action = limitSendableInstanceBehavior(
-                langOpts, check, suggestedBehavior);
-            if (check == SendableCheck::Implicit)
-              return action;
+          [&](Type type, DiagnosticBehavior behavior) {
+            if (check == SendableCheck::Implicit) {
+              invalid = true;
+              return true;
+            }
 
             property->diagnose(diag::non_concurrent_type_member,
-                               false, property->getName(),
+                               propertyType, false, property->getName(),
                                nominal->getDescriptiveKind(),
-                               nominal->getName(),
-                               propertyType)
-                .limitBehavior(action.first);
-            return action;
+                               nominal->getName())
+                .limitBehavior(behavior);
+            return false;
           });
 
-      if (diagnosedProperty) {
-        invalid = true;
-
+      if (invalid) {
         // For implicit checks, bail out early if anything failed.
         if (check == SendableCheck::Implicit)
           return true;
@@ -3791,26 +3749,23 @@ static bool checkSendableInstanceStorage(
 
     /// Handle an enum associated value.
     bool operator()(EnumElementDecl *element, Type elementType) {
-      bool diagnosedElement = diagnoseNonSendableTypes(
+      diagnoseNonSendableTypes(
           elementType, SendableCheckContext(dc, check), element->getLoc(),
-          [&](Type type, DiagnosticBehavior suggestedBehavior) {
-            auto action = limitSendableInstanceBehavior(
-                langOpts, check, suggestedBehavior);
-            if (check == SendableCheck::Implicit)
-              return action;
+          [&](Type type, DiagnosticBehavior behavior) {
+            if (check == SendableCheck::Implicit) {
+              invalid = true;
+              return true;
+            }
 
-            element->diagnose(diag::non_concurrent_type_member,
+            element->diagnose(diag::non_concurrent_type_member, type,
                               true, element->getName(),
                               nominal->getDescriptiveKind(),
-                              nominal->getName(),
-                              type)
-                .limitBehavior(action.first);
-            return action;
+                              nominal->getName())
+                .limitBehavior(behavior);
+            return false;
           });
 
-      if (diagnosedElement) {
-        invalid = true;
-
+      if (invalid) {
         // For implicit checks, bail out early if anything failed.
         if (check == SendableCheck::Implicit)
           return true;
@@ -3859,46 +3814,43 @@ bool swift::checkSendableConformance(
 
   // Sendable can only be used in the same source file.
   auto conformanceDecl = conformanceDC->getAsDecl();
-  const LangOptions &langOpts = conformanceDC->getASTContext().LangOpts;
-  DiagnosticBehavior behavior;
-  bool diagnosticCausesFailure;
-  std::tie(behavior, diagnosticCausesFailure) = limitSendableInstanceBehavior(
-      langOpts, check, DiagnosticBehavior::Unspecified);
-  if (!conformanceDC->getParentSourceFile() ||
+  auto behavior = SendableCheckContext(conformanceDC)
+      .defaultDiagnosticBehavior();
+  if (conformanceDC->getParentSourceFile() &&
+      nominal->getParentSourceFile() &&
       conformanceDC->getParentSourceFile() != nominal->getParentSourceFile()) {
     conformanceDecl->diagnose(diag::concurrent_value_outside_source_file,
                               nominal->getDescriptiveKind(),
                               nominal->getName())
-        .limitBehavior(behavior);
+      .limitBehavior(behavior);
 
-    if (diagnosticCausesFailure)
+    if (behavior == DiagnosticBehavior::Unspecified)
       return true;
   }
 
-  if (classDecl) {
+  if (classDecl && classDecl->getParentSourceFile()) {
+    bool isInherited = isa<InheritedProtocolConformance>(conformance);
+
     // An non-final class cannot conform to `Sendable`.
     if (!classDecl->isSemanticallyFinal()) {
       classDecl->diagnose(diag::concurrent_value_nonfinal_class,
                           classDecl->getName())
-          .limitBehavior(behavior);
+        .limitBehavior(behavior);
 
-      if (diagnosticCausesFailure)
+      if (behavior == DiagnosticBehavior::Unspecified)
         return true;
     }
 
-    // A 'Sendable' class cannot inherit from another class, although
-    // we allow `NSObject` for Objective-C interoperability.
-    if (!isa<InheritedProtocolConformance>(conformance)) {
+    if (!isInherited) {
+      // A 'Sendable' class cannot inherit from another class, although
+      // we allow `NSObject` for Objective-C interoperability.
       if (auto superclassDecl = classDecl->getSuperclassDecl()) {
         if (!superclassDecl->isNSObject()) {
           classDecl->diagnose(
               diag::concurrent_value_inherit,
               nominal->getASTContext().LangOpts.EnableObjCInterop,
-              classDecl->getName())
-              .limitBehavior(behavior);
-
-          if (diagnosticCausesFailure)
-            return true;
+              classDecl->getName());
+          return true;
         }
       }
     }

lib/Sema/TypeCheckConcurrency.h

@@ -279,6 +279,9 @@ struct SendableCheckContext {
   /// Determine the diagnostic behavior when referencing the given nominal
   /// type in this context.
   DiagnosticBehavior diagnosticBehavior(NominalTypeDecl *nominal) const;
+
+  /// Whether we are in an explicit conformance to Sendable.
+  bool isExplicitSendableConformance() const;
 };
 
 /// Diagnose any non-Sendable types that occur within the given type, using
@@ -291,8 +294,7 @@ struct SendableCheckContext {
 /// \returns \c true if any diagnostics were produced, \c false otherwise.
 bool diagnoseNonSendableTypes(
     Type type, SendableCheckContext fromContext, SourceLoc loc,
-    llvm::function_ref<
-      std::pair<DiagnosticBehavior, bool>(Type, DiagnosticBehavior)> diagnose);
+    llvm::function_ref<bool(Type, DiagnosticBehavior)> diagnose);
 
 namespace detail {
   template<typename T>
@@ -314,10 +316,12 @@ bool diagnoseNonSendableTypes(
   return diagnoseNonSendableTypes(
       type, fromContext, loc, [&](Type specificType,
       DiagnosticBehavior behavior) {
-    ctx.Diags.diagnose(loc, diag, type, diagArgs...)
-      .limitBehavior(behavior);
-    return std::pair<DiagnosticBehavior, bool>(
-        behavior, behavior == DiagnosticBehavior::Unspecified);
+    if (behavior != DiagnosticBehavior::Ignore) {
+      ctx.Diags.diagnose(loc, diag, type, diagArgs...)
+        .limitBehavior(behavior);
+    }
+
+    return false;
   });
 }
 

test/Concurrency/concurrent_value_checking.swift

@@ -231,11 +231,11 @@ func concurrentClosures<T>(_: T) {
 // Sendable checking
 // ----------------------------------------------------------------------
 struct S1: Sendable {
-  var nc: NotConcurrent // expected-error{{stored property 'nc' of 'Sendable'-conforming struct 'S1' has non-sendable type 'NotConcurrent'}}
+  var nc: NotConcurrent // expected-warning{{stored property 'nc' of 'Sendable'-conforming struct 'S1' has non-sendable type 'NotConcurrent'}}
 }
 
 struct S2<T>: Sendable {
-  var nc: T // expected-error{{stored property 'nc' of 'Sendable'-conforming generic struct 'S2' has non-sendable type 'T'}}
+  var nc: T // expected-warning{{stored property 'nc' of 'Sendable'-conforming generic struct 'S2' has non-sendable type 'T'}}
 }
 
 struct S3<T> {
@@ -246,7 +246,7 @@ struct S3<T> {
 extension S3: Sendable where T: Sendable { }
 
 enum E1: Sendable {
-  case payload(NotConcurrent) // expected-error{{associated value 'payload' of 'Sendable'-conforming enum 'E1' has non-sendable type 'NotConcurrent'}}
+  case payload(NotConcurrent) // expected-warning{{associated value 'payload' of 'Sendable'-conforming enum 'E1' has non-sendable type 'NotConcurrent'}}
 }
 
 enum E2<T> {
@@ -256,8 +256,8 @@ enum E2<T> {
 extension E2: Sendable where T: Sendable { }
 
 final class C1: Sendable {
-  let nc: NotConcurrent? = nil // expected-error{{stored property 'nc' of 'Sendable'-conforming class 'C1' has non-sendable type 'NotConcurrent?'}}
-  var x: Int = 0 // expected-error{{stored property 'x' of 'Sendable'-conforming class 'C1' is mutable}}
+  let nc: NotConcurrent? = nil // expected-warning{{stored property 'nc' of 'Sendable'-conforming class 'C1' has non-sendable type 'NotConcurrent?'}}
+  var x: Int = 0 // expected-warning{{stored property 'x' of 'Sendable'-conforming class 'C1' is mutable}}
   let i: Int = 0
 }
 
@@ -281,7 +281,7 @@ class C6: C5 {
 
 final class C7<T>: Sendable { }
 
-class C9: Sendable { } // expected-error{{non-final class 'C9' cannot conform to 'Sendable'; use '@unchecked Sendable'}}
+class C9: Sendable { } // expected-warning{{non-final class 'C9' cannot conform to 'Sendable'; use '@unchecked Sendable'}}
 
 // ----------------------------------------------------------------------
 // @unchecked Sendable disabling checking