[region-isolation] Treat as Sendable values that are meant to be ignored since they are marked with preconcurrency

rdar://133531625

Author: gottesmm

include/swift/SIL/SILType.h

@@ -964,6 +964,11 @@ class SILType {
   bool isAnyActor() const { return getASTType()->isAnyActorType(); }
 
   /// Returns true if this function conforms to the Sendable protocol.
+  ///
+  /// NOTE: For diagnostics this is not always the correct thing to check since
+  /// non-Sendable types afflicted with preconcurrency can have different
+  /// semantic requirements around diagnostics. \see
+  /// getConcurrencyDiagnosticBehavior.
   bool isSendable(SILFunction *fn) const;
 
   /// False if SILValues of this type cannot be used outside the scope of their
@@ -978,6 +983,16 @@ class SILType {
     return !isNoEscapeFunction() && isEscapable(function);
   }
 
+  /// Return the expected concurrency diagnostic behavior for this SILType.
+  ///
+  /// This allows one to know if the type is marked with preconcurrency and thus
+  /// should have diagnostics ignored or converted to warnings instead of
+  /// errors.
+  ///
+  /// \returns nil if we were unable to find such information for this type.
+  std::optional<DiagnosticBehavior>
+  getConcurrencyDiagnosticBehavior(SILFunction *fn) const;
+
   //
   // Accessors for types used in SIL instructions:
   //

lib/AST/Type.cpp

@@ -16,36 +16,37 @@
 
 #define DEBUG_TYPE "ast-types"
 
-#include "swift/AST/Types.h"
+#include "clang/AST/Type.h"
 #include "ForeignRepresentationInfo.h"
 #include "swift/AST/ASTContext.h"
 #include "swift/AST/ClangModuleLoader.h"
+#include "swift/AST/Concurrency.h"
 #include "swift/AST/ConformanceLookup.h"
-#include "swift/AST/ExistentialLayout.h"
-#include "swift/AST/ReferenceCounting.h"
-#include "swift/AST/TypeCheckRequests.h"
-#include "swift/AST/TypeVisitor.h"
-#include "swift/AST/TypeWalker.h"
 #include "swift/AST/Decl.h"
+#include "swift/AST/ExistentialLayout.h"
 #include "swift/AST/GenericEnvironment.h"
 #include "swift/AST/LazyResolver.h"
 #include "swift/AST/Module.h"
 #include "swift/AST/PackConformance.h"
 #include "swift/AST/ParameterList.h"
 #include "swift/AST/PrettyStackTrace.h"
 #include "swift/AST/ProtocolConformance.h"
+#include "swift/AST/ReferenceCounting.h"
 #include "swift/AST/SILLayout.h"
 #include "swift/AST/SubstitutionMap.h"
+#include "swift/AST/TypeCheckRequests.h"
 #include "swift/AST/TypeLoc.h"
 #include "swift/AST/TypeRepr.h"
 #include "swift/AST/TypeTransform.h"
+#include "swift/AST/TypeVisitor.h"
+#include "swift/AST/TypeWalker.h"
+#include "swift/AST/Types.h"
 #include "swift/Basic/Assertions.h"
 #include "swift/Basic/Compiler.h"
-#include "clang/AST/Type.h"
 #include "llvm/ADT/APFloat.h"
+#include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallPtrSet.h"
 #include "llvm/ADT/SmallString.h"
-#include "llvm/ADT/STLExtras.h"
 #include "llvm/Support/Compiler.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/raw_ostream.h"
@@ -4883,3 +4884,50 @@ StringRef swift::getNameForParamSpecifier(ParamSpecifier specifier) {
     return "implicitly_copyable_consuming";
   }
 }
+
+std::optional<DiagnosticBehavior>
+TypeBase::getConcurrencyDiagnosticBehaviorLimit(DeclContext *declCtx) const {
+  auto *self = const_cast<TypeBase *>(this);
+
+  if (auto *nomDecl = self->getNominalOrBoundGenericNominal()) {
+    // First try to just grab the exact concurrency diagnostic behavior.
+    if (auto result =
+            swift::getConcurrencyDiagnosticBehaviorLimit(nomDecl, declCtx)) {
+      return result;
+    }
+
+    // But if we get nothing, see if we can come up with diagnostic behavior by
+    // merging our fields if we have a struct.
+    if (auto *structDecl = dyn_cast<StructDecl>(nomDecl)) {
+      std::optional<DiagnosticBehavior> diagnosticBehavior;
+      auto substMap = self->getContextSubstitutionMap();
+      for (auto storedProperty : structDecl->getStoredProperties()) {
+        auto lhs = diagnosticBehavior.value_or(DiagnosticBehavior::Unspecified);
+        auto astType = storedProperty->getInterfaceType().subst(substMap);
+        auto rhs = astType->getConcurrencyDiagnosticBehaviorLimit(declCtx);
+        auto result = lhs.merge(rhs.value_or(DiagnosticBehavior::Unspecified));
+        if (result != DiagnosticBehavior::Unspecified)
+          diagnosticBehavior = result;
+      }
+      return diagnosticBehavior;
+    }
+  }
+
+  // When attempting to determine the diagnostic behavior limit of a tuple, just
+  // merge for each of the elements.
+  if (auto *tupleType = self->getAs<TupleType>()) {
+    std::optional<DiagnosticBehavior> diagnosticBehavior;
+    for (auto tupleType : tupleType->getElements()) {
+      auto lhs = diagnosticBehavior.value_or(DiagnosticBehavior::Unspecified);
+
+      auto type = tupleType.getType()->getCanonicalType();
+      auto rhs = type->getConcurrencyDiagnosticBehaviorLimit(declCtx);
+      auto result = lhs.merge(rhs.value_or(DiagnosticBehavior::Unspecified));
+      if (result != DiagnosticBehavior::Unspecified)
+        diagnosticBehavior = result;
+    }
+    return diagnosticBehavior;
+  }
+
+  return {};
+}

lib/SIL/IR/SILType.cpp

@@ -25,6 +25,7 @@
 #include "swift/SIL/SILModule.h"
 #include "swift/SIL/Test.h"
 #include "swift/SIL/TypeLowering.h"
+#include "swift/Sema/Concurrency.h"
 #include <tuple>
 
 using namespace swift;
@@ -1255,6 +1256,15 @@ bool SILType::isSendable(SILFunction *fn) const {
   return getASTType()->isSendableType();
 }
 
+std::optional<DiagnosticBehavior>
+SILType::getConcurrencyDiagnosticBehavior(SILFunction *fn) const {
+  auto declRef = fn->getDeclRef();
+  if (!declRef)
+    return {};
+  auto *fromDC = declRef.getInnermostDeclContext();
+  return getASTType()->getConcurrencyDiagnosticBehaviorLimit(fromDC);
+}
+
 namespace swift::test {
 // Arguments:
 // - SILValue: value

lib/SILOptimizer/Mandatory/TransferNonSendable.cpp

@@ -94,39 +94,23 @@ static SILValue stripFunctionConversions(SILValue val) {
 }
 
 static std::optional<DiagnosticBehavior>
-getDiagnosticBehaviorLimitForValue(SILValue value) {
-  auto *nom = value->getType().getNominalOrBoundGenericNominal();
-  if (!nom)
-    return {};
-
-  auto declRef = value->getFunction()->getDeclRef();
-  if (!declRef)
-    return {};
-
-  auto *fromDC = declRef.getInnermostDeclContext();
-  return getConcurrencyDiagnosticBehaviorLimit(nom, fromDC);
-}
-
-static std::optional<DiagnosticBehavior>
-getDiagnosticBehaviorLimitForCapturedValue(CapturedValue value) {
+getDiagnosticBehaviorLimitForCapturedValue(SILFunction *fn,
+                                           CapturedValue value) {
   ValueDecl *decl = value.getDecl();
-  auto *nom = decl->getInterfaceType()->getNominalOrBoundGenericNominal();
-  if (!nom)
-    return {};
-
-  auto *fromDC = decl->getInnermostDeclContext();
-  return getConcurrencyDiagnosticBehaviorLimit(nom, fromDC);
+  auto *ctx = decl->getInnermostDeclContext();
+  auto type = fn->mapTypeIntoContext(decl->getInterfaceType());
+  return type->getConcurrencyDiagnosticBehaviorLimit(ctx);
 }
 
 /// Find the most conservative diagnostic behavior by taking the max over all
 /// DiagnosticBehavior for the captured values.
 static std::optional<DiagnosticBehavior>
 getDiagnosticBehaviorLimitForCapturedValues(
-    ArrayRef<CapturedValue> capturedValues) {
+    SILFunction *fn, ArrayRef<CapturedValue> capturedValues) {
   std::optional<DiagnosticBehavior> diagnosticBehavior;
   for (auto value : capturedValues) {
     auto lhs = diagnosticBehavior.value_or(DiagnosticBehavior::Unspecified);
-    auto rhs = getDiagnosticBehaviorLimitForCapturedValue(value).value_or(
+    auto rhs = getDiagnosticBehaviorLimitForCapturedValue(fn, value).value_or(
         DiagnosticBehavior::Unspecified);
     auto result = lhs.merge(rhs);
     if (result != DiagnosticBehavior::Unspecified)
@@ -668,8 +652,11 @@ class UseAfterTransferDiagnosticEmitter {
       emitUnknownPatternError();
   }
 
+  SILFunction *getFunction() const { return transferOp->getFunction(); }
+
   std::optional<DiagnosticBehavior> getBehaviorLimit() const {
-    return getDiagnosticBehaviorLimitForValue(transferOp->get());
+    return transferOp->get()->getType().getConcurrencyDiagnosticBehavior(
+        getFunction());
   }
 
   /// If we can find a callee decl name, return that. None otherwise.
@@ -1329,6 +1316,8 @@ class TransferNonTransferrableDiagnosticEmitter {
 
   Operand *getOperand() const { return info.transferredOperand; }
 
+  SILFunction *getFunction() const { return getOperand()->getFunction(); }
+
   SILValue getNonTransferrableValue() const {
     return info.nonTransferrable.dyn_cast<SILValue>();
   }
@@ -1338,7 +1327,9 @@ class TransferNonTransferrableDiagnosticEmitter {
   }
 
   std::optional<DiagnosticBehavior> getBehaviorLimit() const {
-    return getDiagnosticBehaviorLimitForValue(info.transferredOperand->get());
+    return info.transferredOperand->get()
+        ->getType()
+        .getConcurrencyDiagnosticBehavior(getOperand()->getFunction());
   }
 
   /// If we can find a callee decl name, return that. None otherwise.
@@ -1460,8 +1451,8 @@ class TransferNonTransferrableDiagnosticEmitter {
                   diag::regionbasedisolation_typed_tns_passed_sending_closure,
                   descriptiveKindStr)
         .highlight(loc.getSourceRange())
-        .limitBehaviorIf(
-            getDiagnosticBehaviorLimitForCapturedValue(capturedValue));
+        .limitBehaviorIf(getDiagnosticBehaviorLimitForCapturedValue(
+            getFunction(), capturedValue));
 
     auto capturedLoc = RegularLocation(capturedValue.getLoc());
     if (getIsolationRegionInfo().getIsolationInfo().isTaskIsolated()) {
@@ -1496,8 +1487,8 @@ class TransferNonTransferrableDiagnosticEmitter {
       }
     }
 
-    auto behaviorLimit =
-        getDiagnosticBehaviorLimitForCapturedValues(capturedValues);
+    auto behaviorLimit = getDiagnosticBehaviorLimitForCapturedValues(
+        getFunction(), capturedValues);
     diagnoseError(loc,
                   diag::regionbasedisolation_typed_tns_passed_sending_closure,
                   descriptiveKindStr)
@@ -2059,8 +2050,13 @@ class InOutSendingNotDisconnectedDiagnosticEmitter {
       emitUnknownPatternError();
   }
 
+  SILFunction *getFunction() const {
+    return info.inoutSendingParam->getFunction();
+  }
+
   std::optional<DiagnosticBehavior> getBehaviorLimit() const {
-    return getDiagnosticBehaviorLimitForValue(info.inoutSendingParam);
+    return info.inoutSendingParam->getType().getConcurrencyDiagnosticBehavior(
+        getFunction());
   }
 
   void emitUnknownPatternError() {
@@ -2178,8 +2174,11 @@ class AssignIsolatedIntoSendingResultDiagnosticEmitter {
       emitUnknownPatternError();
   }
 
-  std::optional<DiagnosticBehavior> getBehaviorLimit() const {
-    return getDiagnosticBehaviorLimitForValue(info.outSendingResult);
+  SILFunction *getFunction() const { return info.srcOperand->getFunction(); }
+
+  std::optional<DiagnosticBehavior> getConcurrencyDiagnosticBehavior() const {
+    return info.outSendingResult->getType().getConcurrencyDiagnosticBehavior(
+        getFunction());
   }
 
   void emitUnknownPatternError() {
@@ -2190,7 +2189,7 @@ class AssignIsolatedIntoSendingResultDiagnosticEmitter {
 
     diagnoseError(info.srcOperand->getUser(),
                   diag::regionbasedisolation_unknown_pattern)
-        .limitBehaviorIf(getBehaviorLimit());
+        .limitBehaviorIf(getConcurrencyDiagnosticBehavior());
   }
 
   void emit();
@@ -2326,7 +2325,7 @@ void AssignIsolatedIntoSendingResultDiagnosticEmitter::emit() {
         info.srcOperand,
         diag::regionbasedisolation_out_sending_cannot_be_actor_isolated_named,
         *varName, descriptiveKindStr)
-        .limitBehaviorIf(getBehaviorLimit());
+        .limitBehaviorIf(getConcurrencyDiagnosticBehavior());
 
     diagnoseNote(
         info.srcOperand,
@@ -2342,7 +2341,7 @@ void AssignIsolatedIntoSendingResultDiagnosticEmitter::emit() {
       info.srcOperand,
       diag::regionbasedisolation_out_sending_cannot_be_actor_isolated_type,
       type, descriptiveKindStr)
-      .limitBehaviorIf(getBehaviorLimit());
+      .limitBehaviorIf(getConcurrencyDiagnosticBehavior());
 
   diagnoseNote(
       info.srcOperand,

lib/SILOptimizer/Utils/SILIsolationInfo.cpp

@@ -1173,8 +1173,26 @@ bool SILIsolationInfo::isNonSendableType(SILType type, SILFunction *fn) {
     return false;
   }
 
-  // Otherwise, delegate to seeing if type conforms to the Sendable protocol.
-  return !type.isSendable(fn);
+  // First before we do anything, see if we have a Sendable type. In such a
+  // case, just return true early.
+  //
+  // DISCUSSION: It is important that we do this first since otherwise calling
+  // getConcurrencyDiagnosticBehavior could cause us to prevent a
+  // "preconcurrency" unneeded diagnostic when just using Sendable values. We
+  // only want to trigger that if we analyze a non-Sendable type.
+  if (type.isSendable(fn))
+    return false;
+
+  // Grab out behavior. If it is none, then we have a type that we want to treat
+  // as non-Sendable.
+  auto behavior = type.getConcurrencyDiagnosticBehavior(fn);
+  if (!behavior)
+    return true;
+
+  // Finally, if we are not supposed to ignore, then we have a true non-Sendable
+  // type. Types whose diagnostics we are supposed to ignore, we want to treat
+  // as Sendable.
+  return *behavior != DiagnosticBehavior::Ignore;
 }
 
 //===----------------------------------------------------------------------===//

test/Concurrency/sendable_preconcurrency.swift

@@ -31,16 +31,12 @@ struct MyType3 {
 }
 
 func testA(ns: NS, mt: MyType, mt2: MyType2, mt3: MyType3, sc: StrictClass, nsc: NonStrictClass) async {
-  // This is task isolated since we are capturing function arguments... but
-  // since we are merging NonStrictClass from a preconcurrency module, the whole
-  // error is squelched since we allow for preconcurrency to apply to the entire
-  // region.
-  Task {
+  Task { // expected-tns-warning {{passing closure as a 'sending' parameter risks causing data races between code in the current task and concurrent execution of the closure}}
     print(ns)
     print(mt)
     print(mt2)
     print(mt3)
-    print(sc)
+    print(sc) // expected-tns-note {{closure captures 'sc' which is accessible to code in the current task}}
     print(nsc)
   }
 }