Merge pull request #82884 from DougGregor/isolated-conformances-region-based-diags

[SE-0470] Track the potential introduction of isolated conformances in regions

Author: DougGregor

include/swift/AST/DiagnosticsSIL.def

@@ -1098,6 +1098,10 @@ NOTE(regionbasedisolation_typed_sendneversendable_via_arg_callee, none,
       "sending %0 value of non-Sendable type %1 to %2 %kind3 risks causing races in between %0 and %2 uses",
       (StringRef, Type, StringRef, const ValueDecl *))
 
+NOTE(regionbasedisolation_isolated_conformance_introduced, none,
+     "isolated conformance to %kind0 can be introduced here",
+     (const ValueDecl *))
+
 // Error that is only used when the send non sendable emitter cannot discover any
 // information to give a better diagnostic.
 ERROR(regionbasedisolation_task_or_actor_isolated_sent, none,

include/swift/SILOptimizer/Utils/SILIsolationInfo.h

@@ -190,7 +190,7 @@ class SILIsolationInfo {
     NonisolatedNonsendingTaskIsolated = 0x4,
 
     /// The maximum number of bits used by a Flag.
-    MaxNumBits = 2,
+    MaxNumBits = 3,
   };
 
   using Options = OptionSet<Flag>;
@@ -208,13 +208,19 @@ class SILIsolationInfo {
   /// derived isolatedValue from.
   ActorInstance actorInstance;
 
+  /// When the isolation is introduced due to a (potentially) isolated
+  /// conformance, the protocol whose conformance might be isolated.
+  ProtocolDecl *isolatedConformance = nullptr;
+
   unsigned kind : 8;
   unsigned options : 8;
 
   SILIsolationInfo(SILValue isolatedValue, SILValue actorInstance,
-                   ActorIsolation actorIsolation, Options options = Options())
+                   ActorIsolation actorIsolation, Options options = Options(),
+                   ProtocolDecl *isolatedConformance = nullptr)
       : actorIsolation(actorIsolation), isolatedValue(isolatedValue),
-        actorInstance(ActorInstance::getForValue(actorInstance)), kind(Actor),
+        actorInstance(ActorInstance::getForValue(actorInstance)),
+        isolatedConformance(isolatedConformance), kind(Actor),
         options(options.toRaw()) {
     assert((!actorInstance ||
             (actorIsolation.getKind() == ActorIsolation::ActorInstance &&
@@ -226,15 +232,20 @@ class SILIsolationInfo {
   }
 
   SILIsolationInfo(SILValue isolatedValue, ActorInstance actorInstance,
-                   ActorIsolation actorIsolation, Options options = Options())
+                   ActorIsolation actorIsolation, Options options = Options(),
+                   ProtocolDecl *isolatedConformance = nullptr)
       : actorIsolation(actorIsolation), isolatedValue(isolatedValue),
-        actorInstance(actorInstance), kind(Actor), options(options.toRaw()) {
+        actorInstance(actorInstance), isolatedConformance(isolatedConformance),
+        kind(Actor), options(options.toRaw())
+  {
     assert(actorInstance);
     assert(actorIsolation.getKind() == ActorIsolation::ActorInstance);
   }
 
-  SILIsolationInfo(Kind kind, SILValue isolatedValue)
-      : actorIsolation(), isolatedValue(isolatedValue), kind(kind), options(0) {
+  SILIsolationInfo(Kind kind, SILValue isolatedValue,
+                   ProtocolDecl *isolatedConformance = nullptr)
+      : actorIsolation(), isolatedValue(isolatedValue),
+        isolatedConformance(isolatedConformance), kind(kind), options(0) {
   }
 
   SILIsolationInfo(Kind kind, Options options = Options())
@@ -261,6 +272,12 @@ class SILIsolationInfo {
     return getOptions().contains(Flag::UnsafeNonIsolated);
   }
 
+  // Retrieve the protocol to which there is (or could be) an isolated
+  // conformance.
+  ProtocolDecl *getIsolatedConformance() const {
+    return isolatedConformance;
+  }
+
   SILIsolationInfo withUnsafeNonIsolated(bool newValue = true) const {
     assert(*this && "Cannot be unknown");
     auto self = *this;
@@ -292,6 +309,26 @@ class SILIsolationInfo {
     return self;
   }
 
+  /// Produce a new isolation info value that merges in the given isolated
+  /// conformance value.
+  ///
+  /// If both isolation infos have an isolation conformance, pick one
+  /// arbitrarily. Otherwise, the result has no isolated conformance.
+  SILIsolationInfo
+  withMergedIsolatedConformance(ProtocolDecl *newIsolatedConformance) const {
+    SILIsolationInfo result(*this);
+    if (!isolatedConformance || !newIsolatedConformance) {
+      result.isolatedConformance = nullptr;
+      return result;
+    }
+
+    result.isolatedConformance =
+      ProtocolDecl::compare(isolatedConformance, newIsolatedConformance) <= 0
+        ? isolatedConformance
+        : newIsolatedConformance;
+    return result;
+  }
+
   /// Returns true if this actor isolation is derived from an unapplied
   /// isolation parameter. When merging, we allow for this to be merged with a
   /// more specific isolation kind.
@@ -458,10 +495,13 @@ class SILIsolationInfo {
             Flag::UnappliedIsolatedAnyParameter};
   }
 
-  static SILIsolationInfo getGlobalActorIsolated(SILValue value,
-                                                 Type globalActorType) {
+  static SILIsolationInfo getGlobalActorIsolated(
+      SILValue value,
+      Type globalActorType,
+      ProtocolDecl *isolatedConformance = nullptr) {
     return {value, SILValue() /*no actor instance*/,
-            ActorIsolation::forGlobalActor(globalActorType)};
+            ActorIsolation::forGlobalActor(globalActorType),
+            Options(), isolatedConformance};
   }
 
   static SILIsolationInfo getGlobalActorIsolated(SILValue value,
@@ -473,8 +513,9 @@ class SILIsolationInfo {
                                                     isolation.getGlobalActor());
   }
 
-  static SILIsolationInfo getTaskIsolated(SILValue value) {
-    return {Kind::Task, value};
+  static SILIsolationInfo getTaskIsolated(
+      SILValue value, ProtocolDecl *isolatedConformance = nullptr) {
+    return {Kind::Task, value, isolatedConformance};
   }
 
   /// Attempt to infer the isolation region info for \p inst.

lib/SILOptimizer/Analysis/RegionAnalysis.cpp

@@ -297,6 +297,9 @@ static SILIsolationInfo getIsolationForCastConformances(
 
   const auto &destLayout = destType.getExistentialLayout();
   for (auto proto : destLayout.getProtocols()) {
+    if (proto->isMarkerProtocol())
+      continue;
+
     // If the source type already conforms to the protocol, we won't be looking
     // it up dynamically.
     if (!lookupConformance(sourceType, proto, /*allowMissing=*/false).isInvalid())
@@ -313,12 +316,12 @@ static SILIsolationInfo getIsolationForCastConformances(
     // Otherwise, it's task-isolated.
     if (functionIsolation && functionIsolation->isGlobalActor()) {
       return SILIsolationInfo::getGlobalActorIsolated(
-          value, functionIsolation->getGlobalActor());
+          value, functionIsolation->getGlobalActor(), proto);
     }
 
     // Consider the cast to be task-isolated, because the runtime could find
     // a conformance that is isolated to the current context.
-    return SILIsolationInfo::getTaskIsolated(value);
+    return SILIsolationInfo::getTaskIsolated(value, proto);
   }
 
   return {};
@@ -4131,6 +4134,9 @@ SILIsolationInfo
 PartitionOpTranslator::getIsolationFromConformances(
     SILValue value, ArrayRef<ProtocolConformanceRef> conformances) {
   for (auto conformance: conformances) {
+    if (conformance.getProtocol()->isMarkerProtocol())
+      continue;
+
     // If the conformance is a pack, recurse.
     if (conformance.isPack()) {
       auto pack = conformance.getPack();
@@ -4149,7 +4155,7 @@ PartitionOpTranslator::getIsolationFromConformances(
       auto isolation = conformance.getConcrete()->getIsolation();
       if (isolation.isGlobalActor()) {
         return SILIsolationInfo::getGlobalActorIsolated(
-            value, isolation.getGlobalActor());
+            value, isolation.getGlobalActor(), conformance.getProtocol());
       }
 
       continue;
@@ -4164,7 +4170,8 @@ PartitionOpTranslator::getIsolationFromConformances(
       if (sendableMetatype &&
           lookupConformance(conformance.getType(), sendableMetatype,
                             /*allowMissing=*/false).isInvalid()) {
-        return SILIsolationInfo::getTaskIsolated(value);
+        return SILIsolationInfo::getTaskIsolated(value,
+                                                 conformance.getProtocol());
       }
     }
   }
@@ -4175,7 +4182,7 @@ PartitionOpTranslator::getIsolationFromConformances(
 TranslationSemantics
 PartitionOpTranslator::visitInitExistentialAddrInst(InitExistentialAddrInst *ieai) {
   auto conformanceIsolationInfo = getIsolationFromConformances(
-      ieai->getResult(0), ieai->getConformances());
+      ieai, ieai->getConformances());
 
   translateSILMultiAssign(ieai->getResults(),
                           makeOperandRefRange(ieai->getAllOperands()),
@@ -4187,7 +4194,7 @@ PartitionOpTranslator::visitInitExistentialAddrInst(InitExistentialAddrInst *iea
 TranslationSemantics
 PartitionOpTranslator::visitInitExistentialRefInst(InitExistentialRefInst *ieri) {
   auto conformanceIsolationInfo = getIsolationFromConformances(
-      ieri->getResult(0), ieri->getConformances());
+      ieri, ieri->getConformances());
 
   translateSILMultiAssign(ieri->getResults(),
                           makeOperandRefRange(ieri->getAllOperands()),
@@ -4199,7 +4206,7 @@ PartitionOpTranslator::visitInitExistentialRefInst(InitExistentialRefInst *ieri)
 TranslationSemantics
 PartitionOpTranslator::visitInitExistentialValueInst(InitExistentialValueInst *ievi) {
   auto conformanceIsolationInfo = getIsolationFromConformances(
-      ievi->getResult(0), ievi->getConformances());
+      ievi, ievi->getConformances());
 
   translateSILMultiAssign(ievi->getResults(),
                           makeOperandRefRange(ievi->getAllOperands()),

lib/SILOptimizer/Mandatory/SendNonSendable.cpp

@@ -1312,6 +1312,18 @@ class SendNeverSentDiagnosticEmitter {
   ~SendNeverSentDiagnosticEmitter() {
     if (!emittedErrorDiagnostic) {
       emitUnknownPatternError();
+    } else if (auto proto = isolationRegionInfo.getIsolationInfo()
+                   .getIsolatedConformance()) {
+       // If the diagnostic comes from a (potentially) isolated conformance,
+       // add a note saying so and indicating where the isolated conformance
+       // can come in.
+       if (auto value = isolationRegionInfo.getIsolationInfo().getIsolatedValue()) {
+         if (auto loc = value.getLoc()) {
+           diagnoseNote(
+               loc, diag::regionbasedisolation_isolated_conformance_introduced,
+               proto);
+         }
+       }
     }
   }
 
@@ -1326,6 +1338,13 @@ class SendNeverSentDiagnosticEmitter {
   }
 
   std::optional<DiagnosticBehavior> getBehaviorLimit() const {
+    // If the failure is due to an isolated conformance, downgrade the error
+    // to a warning prior to Swift 7.
+    if (isolationRegionInfo.getIsolationInfo().getIsolatedConformance() &&
+        !sendingOperand->get()->getType().getASTType()->getASTContext().LangOpts
+          .isSwiftVersionAtLeast(7))
+      return DiagnosticBehavior::Warning;
+
     return sendingOperand->get()->getType().getConcurrencyDiagnosticBehavior(
         getOperand()->getFunction());
   }
@@ -1577,8 +1596,7 @@ class SendNeverSentDiagnosticEmitter {
         getIsolationRegionInfo().printForDiagnostics(getFunction());
 
     diagnoseNote(loc, diag::regionbasedisolation_named_send_nt_asynclet_capture,
-                 name, descriptiveKindStr)
-        .limitBehaviorIf(getBehaviorLimit());
+                 name, descriptiveKindStr);
   }
 
   void emitNamedIsolation(SILLocation loc, Identifier name,

lib/SILOptimizer/Utils/SILIsolationInfo.cpp

@@ -1070,6 +1070,10 @@ SILIsolationInfo SILIsolationInfo::get(SILArgument *arg) {
 }
 
 void SILIsolationInfo::printOptions(llvm::raw_ostream &os) const {
+  if (isolatedConformance) {
+    os << "isolated-conformance-to(" << isolatedConformance->getName() << ")";
+  }
+
   auto opts = getOptions();
   if (!opts)
     return;
@@ -1255,9 +1259,11 @@ void SILIsolationInfo::Profile(llvm::FoldingSetNodeID &id) const {
     return;
   case Task:
     id.AddPointer(getIsolatedValue());
+    id.AddPointer(getIsolatedConformance());
     return;
   case Actor:
     id.AddPointer(getIsolatedValue());
+    id.AddPointer(getIsolatedConformance());
     getActorIsolation().Profile(id);
     return;
   }
@@ -1570,7 +1576,7 @@ SILDynamicMergedIsolationInfo::merge(SILIsolationInfo other) const {
     // If both innerInfo and other have the same isolation, we are obviously
     // done. Just return innerInfo since we could return either.
     if (innerInfo.hasSameIsolation(other))
-      return {innerInfo};
+      return {innerInfo.withMergedIsolatedConformance(other.getIsolatedConformance())};
 
     // Ok, there is some difference in between innerInfo and other. Lets see if
     // they are both actor instance isolated and if either are unapplied
@@ -1579,9 +1585,9 @@ SILDynamicMergedIsolationInfo::merge(SILIsolationInfo other) const {
     if (innerInfo.getActorIsolation().isActorInstanceIsolated() &&
         other.getActorIsolation().isActorInstanceIsolated()) {
       if (innerInfo.isUnappliedIsolatedAnyParameter())
-        return other;
+        return other.withMergedIsolatedConformance(innerInfo.getIsolatedConformance());
       if (other.isUnappliedIsolatedAnyParameter())
-        return innerInfo;
+        return innerInfo.withMergedIsolatedConformance(other.getIsolatedConformance());
     }
 
     // Otherwise, they do not match... so return None to signal merge failure.
@@ -1610,7 +1616,8 @@ SILDynamicMergedIsolationInfo::merge(SILIsolationInfo other) const {
   if (innerInfo.isTaskIsolated() && other.isTaskIsolated()) {
     if (innerInfo.isNonisolatedNonsendingTaskIsolated() ||
         other.isNonisolatedNonsendingTaskIsolated())
-      return other.withNonisolatedNonsendingTaskIsolated(true);
+      return other.withNonisolatedNonsendingTaskIsolated(true)
+        .withMergedIsolatedConformance(innerInfo.getIsolatedConformance());
   }
 
   // Otherwise, just return other.