Merge pull request #76558 from gottesmm/pr-39b63781333d46b16f086a7c192efc67b26255db

gottesmm · web-flow · commit fb281331b372 · 2024-09-19T09:18:33.000-07:00
[concurrency] Behind the flag UnspecifiedMeansMainActorIsolated, try inferring by default main actor isolation instead of nonisolated for unspecified.
diff --git a/include/swift/AST/ActorIsolation.h b/include/swift/AST/ActorIsolation.h
@@ -133,6 +133,8 @@ class ActorIsolation {
     return ActorIsolation(GlobalActor, globalActor);
   }
 
+  static ActorIsolation forMainActor(ASTContext &ctx);
+
   static ActorIsolation forErased() {
     return ActorIsolation(Erased);
   }
diff --git a/include/swift/Basic/Features.def b/include/swift/Basic/Features.def
@@ -411,6 +411,9 @@ EXPERIMENTAL_FEATURE(WarnUnsafe, true)
 // Enable values in generic signatures, e.g. <let N: Int>
 EXPERIMENTAL_FEATURE(ValueGenerics, true)
 
+// When a parameter has unspecified isolation, infer it as main actor isolated.
+EXPERIMENTAL_FEATURE(UnspecifiedMeansMainActorIsolated, false)
+
 #undef EXPERIMENTAL_FEATURE_EXCLUDED_FROM_MODULE_INTERFACE
 #undef EXPERIMENTAL_FEATURE
 #undef UPCOMING_FEATURE
diff --git a/lib/AST/ActorIsolation.cpp b/lib/AST/ActorIsolation.cpp
@@ -0,0 +1,21 @@
+//===--- ActorIsolation.cpp -----------------------------------------------===//
+//
+// This source file is part of the Swift.org open source project
+//
+// Copyright (c) 2014 - 2024 Apple Inc. and the Swift project authors
+// Licensed under Apache License v2.0 with Runtime Library Exception
+//
+// See https://swift.org/LICENSE.txt for license information
+// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
+//
+//===----------------------------------------------------------------------===//
+
+#include "swift/AST/ActorIsolation.h"
+#include "swift/AST/ASTContext.h"
+
+using namespace swift;
+
+ActorIsolation ActorIsolation::forMainActor(ASTContext &ctx) {
+  return ActorIsolation::forGlobalActor(
+      ctx.getMainActorType()->mapTypeOutOfContext());
+}
diff --git a/lib/AST/CMakeLists.txt b/lib/AST/CMakeLists.txt
@@ -9,6 +9,7 @@ add_swift_host_library(swiftAST STATIC
   AbstractSourceFileDepGraphFactory.cpp
   AccessNotes.cpp
   AccessRequests.cpp
+  ActorIsolation.cpp
   ArgumentList.cpp
   ASTBridging.cpp
   ASTContext.cpp
diff --git a/lib/AST/FeatureSet.cpp b/lib/AST/FeatureSet.cpp
@@ -204,6 +204,7 @@ UNINTERESTING_FEATURE(ExtractConstantsFromMembers)
 UNINTERESTING_FEATURE(FixedArrays)
 UNINTERESTING_FEATURE(GroupActorErrors)
 UNINTERESTING_FEATURE(SameElementRequirements)
+UNINTERESTING_FEATURE(UnspecifiedMeansMainActorIsolated)
 
 static bool usesFeatureSendingArgsAndResults(Decl *decl) {
   auto isFunctionTypeWithSending = [](Type type) {
diff --git a/lib/Sema/TypeCheckConcurrency.cpp b/lib/Sema/TypeCheckConcurrency.cpp
@@ -5495,6 +5495,12 @@ InferredActorIsolation ActorIsolationRequest::evaluate(
   ActorIsolation defaultIsolation = ActorIsolation::forUnspecified();
   IsolationSource defaultIsolationSource;
 
+  // If we are supposed to infer main actor isolation by default, make our
+  // default isolation main actor.
+  if (ctx.LangOpts.hasFeature(Feature::UnspecifiedMeansMainActorIsolated)) {
+    defaultIsolation = ActorIsolation::forMainActor(ctx);
+  }
+
   if (auto func = dyn_cast<AbstractFunctionDecl>(value)) {
     // A @Sendable function is assumed to be actor-independent.
     if (func->isSendable()) {
@@ -5520,6 +5526,9 @@ InferredActorIsolation ActorIsolationRequest::evaluate(
     overriddenIso = defaultIsolation;
   }
 
+  // NOTE: After this point, the default has been set. Only touch the default
+  // isolation above this point since code below assumes it is now constant.
+
   // Function used when returning an inferred isolation.
   auto inferredIsolation = [&](ActorIsolation inferred,
                                bool onlyGlobal = false) {
@@ -5633,6 +5642,32 @@ InferredActorIsolation ActorIsolationRequest::evaluate(
     }
   }
 
+  // If this is an actor, use the actor isolation of the actor.
+  if (ctx.LangOpts.hasFeature(Feature::UnspecifiedMeansMainActorIsolated)) {
+    // non-async inits and deinits need to be always nonisolated since we can
+    // run the deinit anywhere.
+    //
+    // TODO: We should add a check for if they are marked with global actor
+    // isolation.
+    if (auto *func = dyn_cast<AbstractFunctionDecl>(value)) {
+      if (isa<DestructorDecl>(func) && !func->isAsyncContext())
+        return {ActorIsolation::forNonisolated(false /*unsafe*/),
+                IsolationSource(func, IsolationSource::LexicalContext)};
+
+      if (isa<ConstructorDecl>(func) && !func->isAsyncContext())
+        return {ActorIsolation::forNonisolated(false /*unsafe*/),
+                IsolationSource(func, IsolationSource::LexicalContext)};
+    }
+
+    if (auto nominal = dyn_cast<NominalTypeDecl>(value)) {
+      if (nominal->isActor() && !nominal->isGlobalActor()) {
+        auto isolation = ActorIsolation::forActorInstanceSelf(value);
+        return {inferredIsolation(isolation),
+                IsolationSource(nominal, IsolationSource::LexicalContext)};
+      }
+    }
+  }
+
   // If this is an accessor, use the actor isolation of its storage
   // declaration.
   if (auto accessor = dyn_cast<AccessorDecl>(value)) {
diff --git a/test/Concurrency/assume_mainactor.swift b/test/Concurrency/assume_mainactor.swift
@@ -0,0 +1,199 @@
+// RUN: %target-swift-frontend -swift-version 6 -emit-silgen -enable-experimental-feature UnspecifiedMeansMainActorIsolated %s | %FileCheck %s
+// RUN: %target-swift-frontend -swift-version 6 -emit-sil -enable-experimental-feature UnspecifiedMeansMainActorIsolated %s -verify
+
+// READ THIS! This test is meant to FileCheck the specific isolation when
+// UnspecifiedMeansMainActorIsolated is enabled. Please do not put other types
+// of tests in here.
+
+class Klass {
+  // Implicit deinit
+  // CHECK: // Klass.deinit
+  // CHECK-NEXT: // Isolation: nonisolated
+  // CHECK-NEXT: sil hidden [ossa] @$s16assume_mainactor5KlassCfd : $@convention(method) (@guaranteed Klass) -> @owned Builtin.NativeObject {
+
+  // Implicit deallocating deinit
+  // CHECK: // Klass.__deallocating_deinit
+  // CHECK-NEXT: // Isolation: nonisolated
+  // CHECK-NEXT: sil hidden [ossa] @$s16assume_mainactor5KlassCfD : $@convention(method) (@owned Klass) -> () {
+
+  // Implicit allocating init
+  // CHECK: // Klass.__allocating_init()
+  // CHECK-NEXT: // Isolation: nonisolated
+  // CHECK-NEXT: sil hidden [exact_self_class] [ossa] @$s16assume_mainactor5KlassCACycfC : $@convention(method) (@thick Klass.Type) -> @owned Klass {
+
+  // Implicit designated init
+  // CHECK: // Klass.init()
+  // CHECK-NEXT: // Isolation: nonisolated
+  // CHECK-NEXT: sil hidden [ossa] @$s16assume_mainactor5KlassCACycfc : $@convention(method) (@owned Klass) -> @owned Klass {
+}
+
+struct StructContainingKlass {
+  // CHECK: // variable initialization expression of StructContainingKlass.k
+  // CHECK-NEXT: // Isolation: global_actor. type: MainActor
+  // CHECK-NEXT: sil hidden [transparent] [ossa] @$s16assume_mainactor21StructContainingKlassV1kAA0E0Cvpfi : $@convention(thin) () -> @owned Klass {
+
+  // CHECK: // StructContainingKlass.k.getter
+  // CHECK-NEXT: // Isolation: global_actor. type: MainActor
+  // CHECK-NEXT: sil hidden [transparent] [ossa] @$s16assume_mainactor21StructContainingKlassV1kAA0E0Cvg : $@convention(method) (@guaranteed StructContainingKlass) -> @owned Klass {
+
+  // CHECK: // StructContainingKlass.k.setter
+  // CHECK-NEXT: // Isolation: global_actor. type: MainActor
+  // CHECK-NEXT: sil hidden [transparent] [ossa] @$s16assume_mainactor21StructContainingKlassV1kAA0E0Cvs : $@convention(method) (@owned Klass, @inout StructContainingKlass) -> () {
+  var k = Klass()
+
+  // CHECK: // StructContainingKlass.init()
+  // CHECK-NEXT: // Isolation: nonisolated
+  // CHECK-NEXT: sil hidden [ossa] @$s16assume_mainactor21StructContainingKlassVACycfC : $@convention(method) (@thin StructContainingKlass.Type) -> @owned StructContainingKlass {
+}
+
+// TODO: Allow for nonisolated to be applied to structs.
+struct NonIsolatedStructContainingKlass {
+  // CHECK: // variable initialization expression of NonIsolatedStructContainingKlass.k
+  // CHECK-NEXT: // Isolation: global_actor. type: MainActor
+  // CHECK-NEXT: sil hidden [transparent] [ossa] @$s16assume_mainactor32NonIsolatedStructContainingKlassV1kAA0G0Cvpfi : $@convention(thin) () -> @owned Klass {
+
+  // CHECK: // NonIsolatedStructContainingKlass.k.getter
+  // CHECK-NEXT: // Isolation: global_actor. type: MainActor
+  // CHECK-NEXT: sil hidden [transparent] [ossa] @$s16assume_mainactor32NonIsolatedStructContainingKlassV1kAA0G0Cvg : $@convention(method) (@guaranteed NonIsolatedStructContainingKlass) -> @owned Klass {
+
+  // CHECK: // NonIsolatedStructContainingKlass.k.setter
+  // CHECK-NEXT: // Isolation: global_actor. type: MainActor
+  // CHECK-NEXT: sil hidden [transparent] [ossa] @$s16assume_mainactor32NonIsolatedStructContainingKlassV1kAA0G0Cvs : $@convention(method) (@owned Klass, @inout NonIsolatedStructContainingKlass) -> () {
+  var k = Klass()
+
+  // CHECK: // NonIsolatedStructContainingKlass.init()
+  // CHECK-NEXT: // Isolation: nonisolated
+  // CHECK-NEXT: sil hidden [ossa] @$s16assume_mainactor32NonIsolatedStructContainingKlassVACycfC : $@convention(method) (@thin NonIsolatedStructContainingKlass.Type) -> @owned NonIsolatedStructContainingKlass {
+}
+
+@globalActor
+actor CustomActor {
+  static let shared = CustomActor()
+}
+
+// CHECK: // unspecifiedAsync<A>(_:)
+// CHECK-NEXT: // Isolation: global_actor. type: MainActor
+// CHECK-NEXT: sil hidden [ossa] @$s16assume_mainactor16unspecifiedAsyncyyxYalF : $@convention(thin) @async <T> (@in_guaranteed T) -> () {
+func unspecifiedAsync<T>(_ t: T) async {}
+
+// CHECK: // nonisolatedAsync<A>(_:)
+// CHECK-NEXT: // Isolation: nonisolated
+// CHECK-NEXT: sil hidden [ossa] @$s16assume_mainactor16nonisolatedAsyncyyxYalF : $@convention(thin) @async <T> (@in_guaranteed T) -> () {
+nonisolated func nonisolatedAsync<T>(_ t: T) async {}
+
+// CHECK: // mainActorAsync<A>(_:)
+// CHECK-NEXT: // Isolation: global_actor. type: MainActor
+// CHECK-NEXT: sil hidden [ossa] @$s16assume_mainactor14mainActorAsyncyyxYalF : $@convention(thin) @async <T> (@in_guaranteed T) -> () {
+@MainActor func mainActorAsync<T>(_ t: T) async {}
+
+// CHECK: // customActorAsync<A>(_:)
+// CHECK-NEXT: // Isolation: global_actor. type: CustomActor
+// CHECK-NEXT: sil hidden [ossa] @$s16assume_mainactor16customActorAsyncyyxYalF : $@convention(thin) @async <T> (@in_guaranteed T) -> () {
+@CustomActor func customActorAsync<T>(_ t: T) async {}
+
+
+@CustomActor
+struct CustomActorStruct {
+  // Variable expression is custom actor... but since we have a nonisolated
+  // init, we should be fine?
+  //
+  // CHECK: // variable initialization expression of CustomActorStruct.k
+  // CHECK-NEXT: // Isolation: global_actor. type: CustomActor
+  // CHECK-NEXT: sil hidden [transparent] [ossa] @$s16assume_mainactor17CustomActorStructV1kAA5KlassCvpfi : $@convention(thin) () -> @owned Klass {
+
+  // CHECK: // CustomActorStruct.k.getter
+  // CHECK-NEXT: // Isolation: global_actor. type: CustomActor
+  // CHECK-NEXT: sil hidden [transparent] [ossa] @$s16assume_mainactor17CustomActorStructV1kAA5KlassCvg : $@convention(method) (@guaranteed CustomActorStruct) -> @owned Klass {
+
+  // CHECK: // CustomActorStruct.k.setter
+  // CHECK-NEXT: // Isolation: global_actor. type: CustomActor
+  // CHECK-NEXT: sil hidden [transparent] [ossa] @$s16assume_mainactor17CustomActorStructV1kAA5KlassCvs : $@convention(method) (@owned Klass, @inout CustomActorStruct) -> () {
+  var k = Klass()
+}
+
+// CHECK: // unspecifiedFunctionTest()
+// CHECK-NEXT: // Isolation: global_actor. type: MainActor
+// CHECK-NEXT: sil hidden [ossa] @$s16assume_mainactor23unspecifiedFunctionTestyyYaF : $@convention(thin) @async () -> () {
+func unspecifiedFunctionTest() async {
+  let k = Klass()
+  await unspecifiedAsync(k)
+  await nonisolatedAsync(k)
+  await mainActorAsync(k)
+}
+
+// CHECK: // unspecifiedFunctionTest2()
+// CHECK-NEXT: // Isolation: global_actor. type: MainActor
+// CHECK-NEXT: sil hidden [ossa] @$s16assume_mainactor24unspecifiedFunctionTest2yyYaF : $@convention(thin) @async () -> () {
+func unspecifiedFunctionTest2() async {
+  let k = StructContainingKlass()
+  await unspecifiedAsync(k)
+  await nonisolatedAsync(k)
+  await mainActorAsync(k)
+
+  await unspecifiedAsync(k.k)
+  await nonisolatedAsync(k.k)
+  await mainActorAsync(k.k)
+}
+
+// CHECK: // unspecifiedFunctionTest3()
+// CHECK-NEXT: // Isolation: global_actor. type: MainActor
+// CHECK-NEXT: sil hidden [ossa] @$s16assume_mainactor24unspecifiedFunctionTest3yyYaF : $@convention(thin) @async () -> () {
+func unspecifiedFunctionTest3() async {
+  let k = NonIsolatedStructContainingKlass()
+  await unspecifiedAsync(k)
+  await nonisolatedAsync(k)
+  await mainActorAsync(k)
+
+  await unspecifiedAsync(k.k)
+  await nonisolatedAsync(k.k)
+  await mainActorAsync(k.k)
+}
+
+// CHECK: // nonisolatedFunctionTest()
+// CHECK-NEXT: // Isolation: nonisolated
+// CHECK-NEXT: sil hidden [ossa] @$s16assume_mainactor23nonisolatedFunctionTestyyYaF : $@convention(thin) @async () -> () {
+nonisolated func nonisolatedFunctionTest() async {
+  let k = NonIsolatedStructContainingKlass()
+  await unspecifiedAsync(k.k)
+  await nonisolatedAsync(k.k)
+  await mainActorAsync(k.k)
+}
+
+actor MyActor {
+  // CHECK: // variable initialization expression of MyActor.k
+  // CHECK-NEXT: // Isolation: actor_instance
+  // CHECK-NEXT: sil hidden [transparent] [ossa] @$s16assume_mainactor7MyActorC1kAA5KlassCvpfi : $@convention(thin) () -> @owned Klass {
+
+  // CHECK: // MyActor.k.getter
+  // CHECK-NEXT: // Isolation: actor_instance. name: 'self'
+  // CHECK-NEXT: sil hidden [transparent] [ossa] @$s16assume_mainactor7MyActorC1kAA5KlassCvg : $@convention(method) (@sil_isolated @guaranteed MyActor) -> @owned Klass {
+
+  // CHECK: // MyActor.k.setter
+  // CHECK-NEXT: // Isolation: actor_instance. name: 'self'
+  // CHECK-NEXT: sil hidden [transparent] [ossa] @$s16assume_mainactor7MyActorC1kAA5KlassCvs : $@convention(method) (@owned Klass, @sil_isolated @guaranteed MyActor) -> () {
+  var k = Klass()
+
+  // Implicit deinit
+  // CHECK: // MyActor.deinit
+  // CHECK-NEXT: // Isolation: nonisolated
+  // CHECK-NEXT: sil hidden [ossa] @$s16assume_mainactor7MyActorCfd : $@convention(method) (@guaranteed MyActor) -> @owned Builtin.NativeObject {
+
+  // Non-async init should be nonisolated
+  // CHECK: // MyActor.init()
+  // CHECK-NEXT: // Isolation: nonisolated
+  // CHECK-NEXT: sil hidden [ossa] @$s16assume_mainactor7MyActorCACycfc : $@convention(method) (@owned MyActor) -> @owned MyActor {
+}
+
+actor MyActor2 {
+  // CHECK: // MyActor2.init()
+  // CHECK-NEXT: // Isolation: global_actor. type: MainActor
+  // CHECK-NEXT: sil hidden [ossa] @$s16assume_mainactor8MyActor2CACycfc : $@convention(method) (@owned MyActor2) -> @owned MyActor2 {
+  @MainActor
+  init() {}
+
+  // CHECK: // MyActor2.init(x:)
+  // CHECK-NEXT: // Isolation: global_actor. type: CustomActor
+  // CHECK-NEXT: sil hidden [ossa] @$s16assume_mainactor8MyActor2C1xACyt_tcfc : $@convention(method) (@owned MyActor2) -> @owned MyActor2 {
+  @CustomActor
+  init(x: ()) {}
+}
diff --git a/test/Concurrency/assume_mainactor_typechecker_errors.swift b/test/Concurrency/assume_mainactor_typechecker_errors.swift
@@ -0,0 +1,71 @@
+// RUN: %target-swift-frontend -swift-version 6 -emit-sil -enable-experimental-feature UnspecifiedMeansMainActorIsolated %s -verify
+
+// READ THIS! This test is meant to check the specific isolation when
+// UnspecifiedMeansMainActorIsolated is enabled in combination with validating
+// behavior around explicitly non-Sendable types that trigger type checker
+// specific errors. Please do not put other types of tests in here.
+
+// Fake Sendable Data
+class SendableData : @unchecked Sendable {}
+
+nonisolated func getDataFromSocket() -> SendableData { SendableData() }
+
+class Klass { // expected-note 3 {{}}
+  let s = SendableData()
+
+  init() { s = SendableData() }
+  init(_ s: SendableData) {}
+
+  func doSomething() {}
+}
+
+@available(*, unavailable)
+extension Klass : Sendable {}
+
+struct StructContainingKlass {
+  var k = Klass()
+}
+
+func unspecifiedAsync<T>(_ t: T) async {}
+nonisolated func nonisolatedAsync<T>(_ t: T) async {}
+@MainActor func mainActorAsync<T>(_ t: T) async {}
+
+func unspecifiedFunctionTest() async {
+  let k = Klass()
+  await unspecifiedAsync(k)
+  await nonisolatedAsync(k)
+  await mainActorAsync(k)
+}
+
+func unspecifiedFunctionTest2() async {
+  let k = StructContainingKlass()
+  await unspecifiedAsync(k)
+  await nonisolatedAsync(k)
+  await mainActorAsync(k)
+
+  await unspecifiedAsync(k.k)
+  await nonisolatedAsync(k.k)
+  await mainActorAsync(k.k)
+}
+
+nonisolated func nonisolatedFunctionTest() async {
+  let k = StructContainingKlass()
+  await unspecifiedAsync(k.k) // expected-error {{non-sendable type 'Klass' of property 'k' cannot exit main actor-isolated context}}
+  await nonisolatedAsync(k.k) // expected-error {{non-sendable type 'Klass' of property 'k' cannot exit main actor-isolated context}}
+  await mainActorAsync(k.k) // expected-error {{non-sendable type 'Klass' of property 'k' cannot exit main actor-isolated context}}
+}
+
+func testTask() async {
+  Task {
+    let k = Klass(getDataFromSocket())
+    k.doSomething()
+  }
+}
+
+func testTaskDetached() async {
+  Task.detached {
+    let k = Klass(getDataFromSocket())
+    // Have to pop back onto the main thread to do something.
+    await k.doSomething()
+  }
+}

Original file line number	Diff line number	Diff line change
`@@ -133,6 +133,8 @@ class ActorIsolation {`
`133`	`133`	`return ActorIsolation(GlobalActor, globalActor);`
`134`	`134`	`}`
`135`	`135`
	`136`	`+ static ActorIsolation forMainActor(ASTContext &ctx);`
	`137`	`+`
`136`	`138`	`static ActorIsolation forErased() {`
`137`	`139`	`return ActorIsolation(Erased);`
`138`	`140`	`}`