Merge pull request #72435 from gottesmm/pr-89ce065b7f50a41a58d8fb358d0145913d8b53a3

[region-isolation] Convert more diagnostics from type isolation form to named variable form

Author: gottesmm

lib/SILOptimizer/Mandatory/TransferNonSendable.cpp

@@ -479,9 +479,6 @@ class UseAfterTransferDiagnosticEmitter {
                  diag::regionbasedisolation_named_info_transfer_yields_race,
                  name, isolationCrossing.getCallerIsolation(),
                  isolationCrossing.getCalleeIsolation());
-    // Then emit the note about where the variable is defined.
-    diagnoseNote(variableDefinedLoc, diag::variable_defined_here,
-                 false /*variable*/);
     emitRequireInstDiagnostics();
   }
 

lib/SILOptimizer/Utils/VariableNameUtils.cpp

@@ -51,6 +51,11 @@ findRootValueForNonTupleTempAllocation(AllocationInst *allocInst,
       }
     }
 
+    if (auto *sbi = dyn_cast<StoreBorrowInst>(&inst)) {
+      if (sbi->getDest() == allocInst)
+        return sbi->getSrc();
+    }
+
     // If we do not identify the write... return SILValue(). We weren't able
     // to understand the write.
     break;
@@ -128,6 +133,57 @@ static SILValue findRootValueForTupleTempAllocation(AllocationInst *allocInst,
 
     if (auto *si = dyn_cast<StoreInst>(&inst)) {
       if (si->getOwnershipQualifier() != StoreOwnershipQualifier::Assign) {
+        // Check if we are updating the entire tuple value.
+        if (si->getDest() == allocInst) {
+          // If we already found a root address (meaning we were processing
+          // tuple_elt_addr), bail. We have some sort of unhandled mix of
+          // copy_addr and store.
+          if (foundRootAddress)
+            return SILValue();
+
+          // If we already found a destructure, return SILValue(). We are
+          // initializing twice.
+          if (foundDestructure)
+            return SILValue();
+
+          // We are looking for a pattern where we construct a tuple from
+          // destructured parts.
+          if (auto *ti = dyn_cast<TupleInst>(si->getSrc())) {
+            for (auto p : llvm::enumerate(ti->getOperandValues())) {
+              SILValue value = lookThroughOwnershipInsts(p.value());
+              if (auto *dti = dyn_cast_or_null<DestructureTupleInst>(
+                      value->getDefiningInstruction())) {
+                // We should always go through the same dti.
+                if (foundDestructure && foundDestructure != dti)
+                  return SILValue();
+                if (!foundDestructure)
+                  foundDestructure = dti;
+
+                // If we have a mixmatch of indices, we cannot look through.
+                if (p.index() != dti->getIndexOfResult(value))
+                  return SILValue();
+                if (tupleValues[p.index()])
+                  return SILValue();
+                tupleValues[p.index()] = value;
+
+                // If we have completely covered the tuple, break.
+                --numEltsLeft;
+                if (!numEltsLeft)
+                  break;
+              }
+            }
+
+            // If we haven't completely covered the tuple, return SILValue(). We
+            // should completely cover the tuple.
+            if (numEltsLeft)
+              return SILValue();
+
+            // Otherwise, break since we are done.
+            break;
+          }
+        }
+
+        // If we store to a tuple_element_addr, update for a single value.
         if (auto *tei = dyn_cast<TupleElementAddrInst>(si->getDest())) {
           if (tei->getOperand() == allocInst) {
             unsigned i = tei->getFieldIndex();
@@ -183,7 +239,7 @@ static SILValue findRootValueForTupleTempAllocation(AllocationInst *allocInst,
 
 SILValue VariableNameInferrer::getRootValueForTemporaryAllocation(
     AllocationInst *allocInst) {
-  struct AddressWalker : public TransitiveAddressWalker<AddressWalker> {
+  struct AddressWalker final : public TransitiveAddressWalker<AddressWalker> {
     AddressWalkerState &state;
 
     AddressWalker(AddressWalkerState &state) : state(state) {}
@@ -194,6 +250,12 @@ SILValue VariableNameInferrer::getRootValueForTemporaryAllocation(
       return true;
     }
 
+    TransitiveUseVisitation visitTransitiveUseAsEndPointUse(Operand *use) {
+      if (auto *sbi = dyn_cast<StoreBorrowInst>(use->getUser()))
+        return TransitiveUseVisitation::OnlyUser;
+      return TransitiveUseVisitation::OnlyUses;
+    }
+
     void onError(Operand *use) { state.foundError = true; }
   };
 
@@ -288,6 +350,13 @@ SILValue VariableNameInferrer::findDebugInfoProvidingValueHelper(
       return allocInst;
     }
 
+    // If we have a store_borrow, always look at the dest. We are going to see
+    // if we can determine if dest is a temporary alloc_stack.
+    if (auto *sbi = dyn_cast<StoreBorrowInst>(searchValue)) {
+      searchValue = sbi->getDest();
+      continue;
+    }
+
     if (auto *globalAddrInst = dyn_cast<GlobalAddrInst>(searchValue)) {
       variableNamePath.push_back(globalAddrInst);
       return globalAddrInst;
@@ -487,7 +556,9 @@ SILValue VariableNameInferrer::findDebugInfoProvidingValueHelper(
         isa<ConvertFunctionInst>(searchValue) ||
         isa<MarkUninitializedInst>(searchValue) ||
         isa<CopyableToMoveOnlyWrapperAddrInst>(searchValue) ||
-        isa<MoveOnlyWrapperToCopyableAddrInst>(searchValue)) {
+        isa<MoveOnlyWrapperToCopyableAddrInst>(searchValue) ||
+        isa<MoveOnlyWrapperToCopyableValueInst>(searchValue) ||
+        isa<CopyableToMoveOnlyWrapperValueInst>(searchValue)) {
       searchValue = cast<SingleValueInstruction>(searchValue)->getOperand(0);
       continue;
     }

test/Concurrency/experimental_feature_strictconcurrency.swift

@@ -27,7 +27,7 @@ struct Test2: TestProtocol { // expected-warning{{conformance of 'C2' to 'Sendab
 
 @MainActor
 func iterate(stream: AsyncStream<Int>) async {
-  nonisolated(unsafe) var it = stream.makeAsyncIterator() // expected-region-isolation-note {{variable defined here}}
+  nonisolated(unsafe) var it = stream.makeAsyncIterator()
   // FIXME: Region isolation should consider a value from a 'nonisolated(unsafe)'
   // declaration to be in a disconnected region
 

test/Concurrency/sendable_checking.swift

@@ -274,7 +274,7 @@ final class NonSendable {
 
 @available(SwiftStdlib 5.1, *)
 func testNonSendableBaseArg() async {
-  let t = NonSendable() // expected-tns-note {{variable defined here}}
+  let t = NonSendable()
   await t.update()
   // expected-targeted-and-complete-warning @-1 {{passing argument of non-sendable type 'NonSendable' into main actor-isolated context may introduce data races}}
   // expected-tns-warning @-2 {{transferring 't' may cause a race}}