Merge pull request #74673 from gottesmm/pr-820f4317d3273ec87c02ce9d16e1dc14a759dd40

[transfer-non-sendable] Teach SILIsolationInfo how to handle "look through instructions" when finding actor instances.

Author: gottesmm

include/swift/SILOptimizer/Utils/SILIsolationInfo.h

@@ -55,10 +55,16 @@ class ActorInstance {
   ActorInstance(SILValue value, Kind kind)
       : value(value, std::underlying_type<Kind>::type(kind)) {}
 
+  /// We want to look through certain instructions like end_init_ref that have
+  /// the appropriate actor type but could disguise the actual underlying value
+  /// that we want to represent our actor.
+  static SILValue lookThroughInsts(SILValue value);
+
 public:
   ActorInstance() : ActorInstance(SILValue(), Kind::Value) {}
 
   static ActorInstance getForValue(SILValue value) {
+    value = lookThroughInsts(value);
     return ActorInstance(value, Kind::Value);
   }
 
@@ -411,6 +417,10 @@ class SILDynamicMergedIsolationInfo {
   void printForDiagnostics(llvm::raw_ostream &os) const {
     innerInfo.printForDiagnostics(os);
   }
+
+  SWIFT_DEBUG_DUMPER(dumpForDiagnostics()) {
+    innerInfo.dumpForDiagnostics();
+  }
 };
 
 } // namespace swift

lib/SILOptimizer/Utils/SILIsolationInfo.cpp

@@ -394,6 +394,7 @@ SILIsolationInfo SILIsolationInfo::get(SILInstruction *inst) {
         // TODO: We really should be doing this based off of an Operand. Then
         // we would get the SILValue() for the first element. Today this can
         // only mess up isolation history.
+
         return SILIsolationInfo::getActorInstanceIsolated(
             SILValue(), isolatedOp->get(), nomDecl);
       }
@@ -1100,6 +1101,30 @@ bool SILIsolationInfo::isNonSendableType(SILType type, SILFunction *fn) {
   return !type.isSendable(fn);
 }
 
+//===----------------------------------------------------------------------===//
+//                            MARK: ActorInstance
+//===----------------------------------------------------------------------===//
+
+SILValue ActorInstance::lookThroughInsts(SILValue value) {
+  if (!value)
+    return value;
+
+  while (auto *svi = dyn_cast<SingleValueInstruction>(value)) {
+    if (isa<EndInitLetRefInst>(svi) || isa<CopyValueInst>(svi) ||
+        isa<MoveValueInst>(svi) || isa<ExplicitCopyValueInst>(svi) ||
+        isa<BeginBorrowInst>(svi) ||
+        isa<CopyableToMoveOnlyWrapperValueInst>(svi) ||
+        isa<MoveOnlyWrapperToCopyableValueInst>(svi)) {
+      value = lookThroughInsts(svi->getOperand(0));
+      continue;
+    }
+
+    break;
+  }
+
+  return value;
+}
+
 //===----------------------------------------------------------------------===//
 //                    MARK: SILDynamicMergedIsolationInfo
 //===----------------------------------------------------------------------===//

lib/SILOptimizer/Utils/VariableNameUtils.cpp

@@ -626,7 +626,8 @@ SILValue VariableNameInferrer::findDebugInfoProvidingValueHelper(
         isa<CopyableToMoveOnlyWrapperAddrInst>(searchValue) ||
         isa<MoveOnlyWrapperToCopyableAddrInst>(searchValue) ||
         isa<MoveOnlyWrapperToCopyableValueInst>(searchValue) ||
-        isa<CopyableToMoveOnlyWrapperValueInst>(searchValue)) {
+        isa<CopyableToMoveOnlyWrapperValueInst>(searchValue) ||
+        isa<EndInitLetRefInst>(searchValue)) {
       searchValue = cast<SingleValueInstruction>(searchValue)->getOperand(0);
       continue;
     }

test/Concurrency/async_task_locals_basic_warnings_bug_isolation.swift

@@ -2,14 +2,10 @@
 // RUN: %target-swift-frontend -plugin-path %swift-plugin-dir -emit-module -emit-module-path %t/OtherActors.swiftmodule -module-name OtherActors %S/Inputs/OtherActors.swift -disable-availability-checking
 
 // RUN: %target-swift-frontend -I %t -plugin-path %swift-plugin-dir -disable-availability-checking -strict-concurrency=complete -parse-as-library %s -emit-sil -o /dev/null -verify
-// RUN: %target-swift-frontend -I %t -plugin-path %swift-plugin-dir -disable-availability-checking -strict-concurrency=complete -parse-as-library %s -emit-sil -o /dev/null -verify -enable-upcoming-feature RegionBasedIsolation
 
 // REQUIRES: concurrency
 // REQUIRES: asserts
 
-// FIXME: rdar://125078448 is resolved
-// XFAIL: *
-
 actor Test {
 
   @TaskLocal static var local: Int?

test/Concurrency/silisolationinfo_inference.sil

@@ -32,6 +32,7 @@ sil @constructNonSendableKlassIndirect : $@convention(thin) () -> @out NonSendab
 sil @constructNonSendableKlassIndirectAsync : $@convention(thin) @async () -> @out NonSendableKlass
 sil @useUnmanagedNonSendableKlass : $@convention(thin) (@guaranteed @sil_unmanaged NonSendableKlass) -> ()
 
+sil @useActor : $@convention(thin) (@sil_isolated @guaranteed MyActor) -> @owned NonSendableKlass
 
 ///////////////////////
 // MARK: Basic Tests //
@@ -699,3 +700,24 @@ bb7:
   %9999 = tuple ()
   return %9999 : $()
 }
+
+// CHECK-LABEL: begin running test 1 of 1 on lookthrough_test: sil-isolation-info-inference with: @trace[0]
+// CHECK-NEXT: Input Value:   %7 = apply %6(%5) : $@convention(thin) (@sil_isolated @guaranteed MyActor) -> @owned NonSendableKlass
+// CHECK-NEXT: Isolation: 'argument'-isolated
+// CHECK-NEXT: end running test 1 of 1 on lookthrough_test: sil-isolation-info-inference with: @trace[0]
+sil [ossa] @lookthrough_test : $@convention(thin) (@owned MyActor) -> @owned NonSendableKlass {
+bb0(%0 : @owned $MyActor):
+  specify_test "sil-isolation-info-inference @trace[0]"
+  debug_value %0 : $MyActor, let, name "argument"
+  %1 = end_init_let_ref %0 : $MyActor
+  %1a = copy_value %1 : $MyActor
+  %1b = move_value %1a : $MyActor
+  %1c = begin_borrow %1b : $MyActor
+  %2 = function_ref @useActor : $@convention(thin) (@sil_isolated @guaranteed MyActor) -> @owned NonSendableKlass
+  %3 = apply %2(%1c) : $@convention(thin) (@sil_isolated @guaranteed MyActor) -> @owned NonSendableKlass
+  debug_value [trace] %3 : $NonSendableKlass
+  end_borrow %1c : $MyActor
+  destroy_value %1 : $MyActor
+  destroy_value %1b : $MyActor
+  return %3 : $NonSendableKlass
+}

test/Concurrency/transfernonsendable_initializers.swift

@@ -50,7 +50,13 @@ actor ActorWithSynchronousNonIsolatedInit {
     }
   }
 
+  init(ns: NonSendableKlass) async {
+    self.k = NonSendableKlass()
+    self.isolatedHelper(ns)
+  }
+
   nonisolated func helper(_ newK: NonSendableKlass) {}
+  func isolatedHelper(_ newK: NonSendableKlass) {}
 }
 
 func initActorWithSyncNonIsolatedInit() {

test/Concurrency/transfernonsendable_ownership.swift

@@ -105,3 +105,19 @@ func testInOutError(_ x: inout Klass) async {
   // expected-note @-1 {{sending global actor 'CustomActor'-isolated 'x' to main actor-isolated global function 'transferToMain'}}
   _ = consume x // expected-note {{consumed here}}
 } // expected-note {{used here}}
+
+actor ActorTestCase {
+  let k = Klass()
+
+  // TODO: This crashes the compiler since we attempt to hop_to_executor to the
+  // value that is materialized onto disk.
+  //
+  // consuming func test() async {
+  //   await transferToMain(k)
+  // }
+
+  borrowing func test2() async {
+    await transferToMain(k) // expected-warning {{sending 'self.k' risks causing data races}}
+    // expected-note @-1 {{sending 'self'-isolated 'self.k' to main actor-isolated global function 'transferToMain' risks causing data races between main actor-isolated and 'self'-isolated uses}}
+  }
+}

test/SILOptimizer/variable_name_inference.sil

@@ -866,10 +866,10 @@ bb0:
 }
 
 // CHECK-LABEL: begin running test 1 of 1 on begin_borrow_var_decl_3: variable-name-inference with: @trace[0]
-// CHECK-LABEL: Input Value:   %1 = apply %0() : $@convention(thin) () -> @owned Klass
-// CHECK-LABEL: Name: 'unknown'
-// CHECK-LABEL: Root: 'unknown'
-// CHECK-LABEL: end running test 1 of 1 on begin_borrow_var_decl_3: variable-name-inference with: @trace[0]
+// CHECK: Input Value:   %1 = apply %0() : $@convention(thin) () -> @owned Klass
+// CHECK: Name: 'unknown'
+// CHECK: Root: 'unknown'
+// CHECK: end running test 1 of 1 on begin_borrow_var_decl_3: variable-name-inference with: @trace[0]
 sil [ossa] @begin_borrow_var_decl_3 : $@convention(thin) () -> () {
 bb0:
   specify_test "variable-name-inference @trace[0]"
@@ -883,3 +883,17 @@ bb0:
   %9999 = tuple ()
   return %9999 : $()
 }
+
+// CHECK-LABEL: begin running test 1 of 1 on infer_through_end_init_let_ref: variable-name-inference with: @trace[0]
+// CHECK: Input Value:   %2 = end_init_let_ref %0 : $Klass
+// CHECK: Name: 'self'
+// CHECK: Root: %0 = argument of bb0 : $Klass
+// CHECK: end running test 1 of 1 on infer_through_end_init_let_ref: variable-name-inference with: @trace[0]
+sil [ossa] @infer_through_end_init_let_ref : $@convention(thin) (@owned Klass) -> @owned Klass {
+bb0(%0 : @owned $Klass):
+  specify_test "variable-name-inference @trace[0]"
+  debug_value %0 : $Klass, let, name "self", argno 2
+  %1 = end_init_let_ref %0 : $Klass
+  debug_value [trace] %1 : $Klass
+  return %1 : $Klass
+}