Merge pull request #78837 from gottesmm/rdar-142661388

[rbi] Change Region Based Isolation for closures to not use the AST and instead just use SIL.

Author: gottesmm

SwiftCompilerSources/Sources/SIL/Instruction.swift

@@ -1736,3 +1736,6 @@ final public class ThunkInst : Instruction {
 
 final public class MergeIsolationRegionInst : Instruction {
 }
+
+final public class IgnoredUseInst : Instruction, UnaryInstruction {
+}

SwiftCompilerSources/Sources/SIL/Registration.swift

@@ -258,4 +258,5 @@ public func registerSILClasses() {
   register(CheckedCastAddrBranchInst.self)
   register(ThunkInst.self)
   register(MergeIsolationRegionInst.self)
+  register(IgnoredUseInst.self)
 }

docs/SIL/Instructions.md

@@ -5551,3 +5551,21 @@ sil-instruction ::= 'has_symbol' sil-decl-ref
 Returns true if each of the underlying symbol addresses associated with
 the given declaration are non-null. This can be used to determine
 whether a weakly-imported declaration is available at runtime.
+
+## Miscellaneous instructions
+
+### ignored_use
+
+```none
+sil-instruction ::= 'ignored_use'
+```
+
+This instruction acts as a synthetic use instruction that suppresses unused
+variable warnings. In Swift the equivalent operation is '_ = x'. This
+importantly also provides a way to find the source location for '_ = x' when
+emitting SIL diagnostics. It is only legal in Raw SIL and is removed as dead
+code when we convert to Canonical SIL.
+
+DISCUSSION: Before the introduction of this instruction, in certain cases,
+SILGen would just not emit anything for '_ = x'... so one could not emit
+diagnostics upon this case.

include/swift/AST/DiagnosticsSIL.def

@@ -1056,6 +1056,12 @@ NOTE(regionbasedisolation_typed_tns_passed_to_sending_closure_helper_have_value_
 NOTE(regionbasedisolation_typed_tns_passed_to_sending_closure_helper_multiple_value, none,
      "closure captures non-Sendable %0",
      (DeclName))
+NOTE(regionbasedisolation_closure_captures, none,
+     "closure captures %0",
+     (DeclName))
+NOTE(regionbasedisolation_closure_captures_actor, none,
+     "closure captures %0 allowing access to %1 state within the closure",
+     (DeclName, StringRef))
 
 NOTE(regionbasedisolation_typed_tns_passed_to_sending_callee, none,
      "Passing %0 value of non-Sendable type %1 as a 'sending' parameter to %2 %3 risks causing races inbetween %0 uses and uses reachable from %3",

include/swift/SIL/InstructionUtils.h

@@ -129,6 +129,11 @@ bool isEndOfScopeMarker(SILInstruction *user);
 /// only used in recognizable patterns without otherwise "escaping".
 bool isIncidentalUse(SILInstruction *user);
 
+/// Returns true if this is a move only wrapper use.
+///
+/// E.x.: moveonlywrapper_to_copyable_addr, copyable_to_moveonlywrapper_value
+bool isMoveOnlyWrapperUse(SILInstruction *user);
+
 /// Return true if the given `user` instruction modifies the value's refcount
 /// without propagating the value or having any other effect aside from
 /// potentially destroying the value itself (and executing associated cleanups).

include/swift/SIL/SILBuilder.h

@@ -3089,6 +3089,15 @@ class SILBuilder {
         getModule(), getSILDebugLocation(Loc), Decl));
   }
 
+  //===--------------------------------------------------------------------===//
+  // Misc Uses
+  //===--------------------------------------------------------------------===//
+
+  IgnoredUseInst *createIgnoredUse(SILLocation loc, SILValue value) {
+    return insert(new (getModule())
+                      IgnoredUseInst(getSILDebugLocation(loc), value));
+  }
+
   //===--------------------------------------------------------------------===//
   // Private Helper Methods
   //===--------------------------------------------------------------------===//

include/swift/SIL/SILCloner.h

@@ -3796,6 +3796,14 @@ void SILCloner<ImplClass>::visitHasSymbolInst(HasSymbolInst *Inst) {
                                          Inst->getDecl()));
 }
 
+template <typename ImplClass>
+void SILCloner<ImplClass>::visitIgnoredUseInst(IgnoredUseInst *Inst) {
+  getBuilder().setCurrentDebugScope(getOpScope(Inst->getDebugScope()));
+  recordClonedInstruction(
+      Inst, getBuilder().createIgnoredUse(getOpLocation(Inst->getLoc()),
+                                          getOpValue(Inst->getOperand())));
+}
+
 } // end namespace swift
 
 #endif

include/swift/SIL/SILInstruction.h

@@ -11725,6 +11725,17 @@ class MergeIsolationRegionInst final
   }
 };
 
+/// An instruction that represents a semantic-less use that is used to
+/// suppresses unused value variable warnings. E.x.: _ = x.
+class IgnoredUseInst final
+    : public UnaryInstructionBase<SILInstructionKind::IgnoredUseInst,
+                                  NonValueInstruction> {
+  friend SILBuilder;
+
+  IgnoredUseInst(SILDebugLocation loc, SILValue operand)
+      : UnaryInstructionBase(loc, operand) {}
+};
+
 inline SILType *AllocRefInstBase::getTypeStorage() {
   // If the size of the subclasses are equal, then all of this compiles away.
   if (auto I = dyn_cast<AllocRefInst>(this))

include/swift/SIL/SILNodes.def

@@ -897,6 +897,9 @@ NON_VALUE_INST(MarkUnresolvedMoveAddrInst, mark_unresolved_move_addr,
 NON_VALUE_INST(MergeIsolationRegionInst, merge_isolation_region,
                SILInstruction, None, DoesNotRelease)
 
+NON_VALUE_INST(IgnoredUseInst, ignored_use,
+               SILInstruction, None, DoesNotRelease)
+
 NON_VALUE_INST(IncrementProfilerCounterInst, increment_profiler_counter,
                SILInstruction, MayReadWrite, DoesNotRelease)
 

include/swift/SILOptimizer/Utils/PartitionUtils.h

@@ -1374,6 +1374,12 @@ struct PartitionOpEvaluator {
       Region sentRegion = p.getRegion(sentElement);
       bool isClosureCapturedElt = false;
       SILDynamicMergedIsolationInfo sentRegionIsolation;
+
+      // TODO: Today we only return the first element in our region that has
+      // some form of isolation. This causes us to in the case of sending
+      // partial_applies to only emit a diagnostic for the first element in the
+      // capture list of the partial_apply. If we returned a list of potential
+      // errors... we could emit the error for each capture individually.
       auto pairOpt = getIsolationRegionInfo(sentRegion, op.getSourceOp());
       if (!pairOpt) {
         return handleError(UnknownCodePatternError(op));

lib/IRGen/IRGenSIL.cpp

@@ -1273,6 +1273,9 @@ class IRGenSILFunction :
   visitMoveOnlyWrapperToCopyableBoxInst(MoveOnlyWrapperToCopyableBoxInst *i) {
     llvm_unreachable("OSSA instruction");
   }
+
+  void visitIgnoredUseInst(IgnoredUseInst *i) {}
+
   void
   visitMoveOnlyWrapperToCopyableAddrInst(MoveOnlyWrapperToCopyableAddrInst *i) {
     auto e = getLoweredExplosion(i->getOperand());

lib/SIL/IR/OperandOwnership.cpp

@@ -214,6 +214,7 @@ OPERAND_OWNERSHIP(TrivialUse, GlobalAddr)
 // The dealloc_stack_ref operand needs to have NonUse ownership because
 // this use comes after the last consuming use (which is usually a dealloc_ref).
 OPERAND_OWNERSHIP(NonUse, DeallocStackRef)
+OPERAND_OWNERSHIP(InstantaneousUse, IgnoredUse)
 
 // Use an owned or guaranteed value only for the duration of the operation.
 OPERAND_OWNERSHIP(InstantaneousUse, ExistentialMetatype)

lib/SIL/IR/SILPrinter.cpp

@@ -2873,6 +2873,10 @@ class SILPrinter : public SILInstructionVisitor<SILPrinter> {
     *this << GI->getFormalResumeType();
   }
 
+  void visitIgnoredUseInst(IgnoredUseInst *i) {
+    *this << getIDAndType(i->getOperand());
+  }
+
   void visitGetAsyncContinuationAddrInst(GetAsyncContinuationAddrInst *GI) {
     if (GI->throws())
       *this << "[throws] ";

lib/SIL/Parser/ParseSIL.cpp

@@ -5077,6 +5077,11 @@ bool SILParser::parseSpecificSILInstruction(SILBuilder &B,
     }
     break;
   }
+  case SILInstructionKind::IgnoredUseInst:
+    if (parseTypedValueRef(Val, B) || parseSILDebugLocation(InstLoc, B))
+      return true;
+    ResultVal = B.createIgnoredUse(InstLoc, Val);
+    break;
 
   case SILInstructionKind::DeallocStackInst:
     if (parseTypedValueRef(Val, B) || parseSILDebugLocation(InstLoc, B))

lib/SIL/Utils/InstructionUtils.cpp

@@ -335,7 +335,21 @@ bool swift::isEndOfScopeMarker(SILInstruction *user) {
 
 bool swift::isIncidentalUse(SILInstruction *user) {
   return isEndOfScopeMarker(user) || user->isDebugInstruction() ||
-         isa<FixLifetimeInst>(user) || isa<EndLifetimeInst>(user);
+         isa<FixLifetimeInst>(user) || isa<EndLifetimeInst>(user) ||
+         isa<IgnoredUseInst>(user);
+}
+
+bool swift::isMoveOnlyWrapperUse(SILInstruction *user) {
+  switch (user->getKind()) {
+  case SILInstructionKind::MoveOnlyWrapperToCopyableValueInst:
+  case SILInstructionKind::MoveOnlyWrapperToCopyableBoxInst:
+  case SILInstructionKind::MoveOnlyWrapperToCopyableAddrInst:
+  case SILInstructionKind::CopyableToMoveOnlyWrapperValueInst:
+  case SILInstructionKind::CopyableToMoveOnlyWrapperAddrInst:
+    return true;
+  default:
+    return false;
+  }
 }
 
 bool swift::onlyAffectsRefCount(SILInstruction *user) {
@@ -623,6 +637,7 @@ RuntimeEffect swift::getRuntimeEffect(SILInstruction *inst, SILType &impactType)
   case SILInstructionKind::DebugStepInst:
   case SILInstructionKind::FunctionExtractIsolationInst:
   case SILInstructionKind::TypeValueInst:
+  case SILInstructionKind::IgnoredUseInst:
     return RuntimeEffect::NoEffect;
 
   case SILInstructionKind::OpenExistentialMetatypeInst:

lib/SIL/Verifier/SILVerifier.cpp

@@ -719,6 +719,7 @@ struct ImmutableAddressUseVerifier {
       case SILInstructionKind::KeyPathInst:
       case SILInstructionKind::SwitchEnumAddrInst:
       case SILInstructionKind::SelectEnumAddrInst:
+      case SILInstructionKind::IgnoredUseInst:
         break;
       case SILInstructionKind::DebugValueInst:
         if (cast<DebugValueInst>(inst)->hasAddrVal())

lib/SILGen/SILGenDecl.cpp

@@ -1582,38 +1582,34 @@ void SILGenFunction::emitPatternBinding(PatternBindingDecl *PBD, unsigned idx,
     return nullptr;
   };
 
-  auto emitInitializer = [&](Expr *initExpr, VarDecl *var, bool forLocalContext,
-                             InitializationPtr &initialization) {
-    // If an initial value expression was specified by the decl, emit it into
-    // the initialization.
-    FullExpr Scope(Cleanups, CleanupLocation(initExpr));
-
-    if (forLocalContext) {
-      if (auto *orig = var->getOriginalWrappedProperty()) {
-        if (auto *initExpr = getWrappedValueExpr(var)) {
-          auto value = emitRValue(initExpr);
-          emitApplyOfPropertyWrapperBackingInitializer(
+  auto *initExpr = PBD->getExecutableInit(idx);
+
+  // If we do not have an explicit initialization expression, just mark the
+  // initialization as unfinished for DI to resolve.
+  if (!initExpr) {
+    return initialization->finishUninitialized(*this);
+  }
+
+  // Otherwise, an initial value expression was specified by the decl... emit it
+  // into the initialization.
+  FullExpr Scope(Cleanups, CleanupLocation(initExpr));
+
+  auto *singleVar = PBD->getSingleVar();
+  bool isLocalSingleVar =
+      singleVar && singleVar->getDeclContext()->isLocalContext();
+  if (isLocalSingleVar) {
+    if (auto *orig = singleVar->getOriginalWrappedProperty()) {
+      if (auto *initExpr = getWrappedValueExpr(singleVar)) {
+        auto value = emitRValue(initExpr);
+        emitApplyOfPropertyWrapperBackingInitializer(
             PBD, orig, getForwardingSubstitutionMap(), std::move(value))
             .forwardInto(*this, SILLocation(PBD), initialization.get());
-          return;
-        }
+        return;
       }
     }
-
-    emitExprInto(initExpr, initialization.get());
-  };
-
-  auto *singleVar = PBD->getSingleVar();
-  if (auto *Init = PBD->getExecutableInit(idx)) {
-    // If an initial value expression was specified by the decl, emit it into
-    // the initialization.
-    bool isLocalVar =
-        singleVar && singleVar->getDeclContext()->isLocalContext();
-    emitInitializer(Init, singleVar, isLocalVar, initialization);
-  } else {
-    // Otherwise, mark it uninitialized for DI to resolve.
-    initialization->finishUninitialized(*this);
   }
+
+  emitExprInto(initExpr, initialization.get());
 }
 
 void SILGenFunction::visitPatternBindingDecl(PatternBindingDecl *PBD,
@@ -2421,18 +2417,23 @@ void BlackHoleInitialization::performPackExpansionInitialization(
 
 void BlackHoleInitialization::copyOrInitValueInto(SILGenFunction &SGF, SILLocation loc,
                                                   ManagedValue value, bool isInit) {
-  // Normally we do not do anything if we have a black hole
-  // initialization... but if we have a move only object, insert a move value.
-  if (!value.getType().isMoveOnly())
+  // If we do not have a noncopyable type, just insert an ignored use.
+  if (!value.getType().isMoveOnly()) {
+    SGF.B.createIgnoredUse(loc, value.getValue());
     return;
+  }
 
-  // If we have an address, then this will create a new temporary allocation
-  // which will trigger the move checker. If we have an object though, we need
-  // to insert an extra move_value to make sure the object checker behaves
-  // correctly.
+  // If we have a noncopyable type, we need to do a little more work to satisfy
+  // the move checkers. If we have an address, then this will create a new
+  // temporary allocation which will trigger the move checker...
   value = value.ensurePlusOne(SGF, loc);
-  if (value.getType().isAddress())
+  if (value.getType().isAddress()) {
+    SGF.B.createIgnoredUse(loc, value.getValue());
     return;
+  }
 
+  // If we have an object though, we need to insert an extra move_value to make
+  // sure the object checker behaves correctly.
   value = SGF.B.createMoveValue(loc, value);
+  SGF.B.createIgnoredUse(loc, value.getValue());
 }

lib/SILGen/SILGenExpr.cpp

@@ -5376,7 +5376,19 @@ static void emitSimpleAssignment(SILGenFunction &SGF, SILLocation loc,
           value = value.ensurePlusOne(SGF, loc);
           if (value.getType().isObject())
             value = SGF.B.createMoveValue(loc, value);
+          SGF.B.createIgnoredUse(loc, value.getValue());
+          return;
+        }
+
+        // Emit the ignored use instruction like we would do in emitIgnoredExpr.
+        if (!rv.isNull()) {
+          SmallVector<ManagedValue, 16> values;
+          std::move(rv).getAll(values);
+          for (auto v : values) {
+            SGF.B.createIgnoredUse(loc, v.getValue());
+          }
         }
+
         return;
       }
 
@@ -7027,7 +7039,24 @@ void SILGenFunction::emitIgnoredExpr(Expr *E) {
 
   // Otherwise, emit the result (to get any side effects), but produce it at +0
   // if that allows simplification.
-  emitRValue(E, SGFContext::AllowImmediatePlusZero);
+  RValue rv = emitRValue(E, SGFContext::AllowImmediatePlusZero);
+
+  // Return early if we do not have any result.
+  if (rv.isNull())
+    return;
+
+  // Then emit ignored use on all of the rvalue components. We purposely do not
+  // implode the tuple since if we have a tuple formation like:
+  //
+  // let _ = (x, y)
+  //
+  // we want the use to be on x and y and not on the imploded tuple. It also
+  // helps us avoid emitting unnecessary code.
+  SmallVector<ManagedValue, 16> values;
+  std::move(rv).getAll(values);
+  for (auto v : values) {
+    B.createIgnoredUse(E, v.getValue());
+  }
 }
 
 /// Emit the given expression as an r-value, then (if it is a tuple), combine

lib/SILOptimizer/Analysis/RegionAnalysis.cpp

@@ -3103,6 +3103,7 @@ CONSTANT_TRANSLATION(DebugStepInst, Ignored)
 CONSTANT_TRANSLATION(IncrementProfilerCounterInst, Ignored)
 CONSTANT_TRANSLATION(SpecifyTestInst, Ignored)
 CONSTANT_TRANSLATION(TypeValueInst, Ignored)
+CONSTANT_TRANSLATION(IgnoredUseInst, Ignored)
 
 //===---
 // Require

lib/SILOptimizer/Mandatory/DiagnoseUnreachable.cpp

@@ -775,6 +775,14 @@ static bool simplifyBlocksWithCallsToNoReturn(SILBasicBlock &BB,
     if (isa<EndBorrowInst>(currInst))
       return false;
 
+    // If we have an ignored use whose operand is our no return call, ignore it.
+    if (auto *i = dyn_cast<IgnoredUseInst>(currInst)) {
+      if (auto *svi = dyn_cast<SingleValueInstruction>(i->getOperand());
+          svi && getAsCallToNoReturn(svi)) {
+        return false;
+      }
+    }
+
     // destroy_value [dead_end] instructions are inserted at the availability
     // boundary by lifetime completion.  Such instructions correctly mark the
     // lifetime boundary of the destroyed value and never arise from dead user