[6.3][Concurrency] A few fixes for default isolation inference

lib/Sema/TypeCheckConcurrency.cpp

@@ -23,6 +23,7 @@
 #include "TypeCheckType.h"
 #include "TypeChecker.h"
 #include "swift/AST/ASTWalker.h"
+#include "swift/AST/Attr.h"
 #include "swift/AST/Concurrency.h"
 #include "swift/AST/ConformanceLookup.h"
 #include "swift/AST/DiagnosticGroups.h"
@@ -6083,6 +6084,21 @@ static void checkDeclWithIsolatedParameter(ValueDecl *value) {
   }
 }
 
+/// If `@preconcurrency` attribute can be used on this declaration, apply it.
+static void markAsPreconcurrencyIfApplicable(ValueDecl *value) {
+  // If there is one already, nothing more to do.
+  if (value->getAttrs().hasAttribute<PreconcurrencyAttr>())
+    return;
+
+  if (!DeclAttribute::canAttributeAppearOnDecl(DeclAttrKind::Preconcurrency,
+                                               value))
+    return;
+
+  auto *preconcurrency =
+      new (value->getASTContext()) PreconcurrencyAttr(/*IsImplicit=*/true);
+  value->addAttribute(preconcurrency);
+}
+
 static void addAttributesForActorIsolation(ValueDecl *value,
                                            ActorIsolation isolation) {
   ASTContext &ctx = value->getASTContext();
@@ -6106,11 +6122,9 @@ static void addAttributesForActorIsolation(ValueDecl *value,
                                    /*implicit=*/true);
     value->addAttribute(attr);
 
-    if (isolation.preconcurrency() &&
-        !value->getAttrs().hasAttribute<PreconcurrencyAttr>()) {
-      auto preconcurrency = new (ctx) PreconcurrencyAttr(/*isImplicit*/ true);
-      value->addAttribute(preconcurrency);
-    }
+    if (isolation.preconcurrency())
+      markAsPreconcurrencyIfApplicable(value);
+
     break;
   }
     case ActorIsolation::Erased:
@@ -6424,11 +6438,9 @@ static InferredActorIsolation computeActorIsolation(Evaluator &evaluator,
 
   auto isolationFromAttr = getIsolationFromAttributes(value);
   ASTContext &ctx = value->getASTContext();
-  if (isolationFromAttr && isolationFromAttr->preconcurrency() &&
-      !value->getAttrs().hasAttribute<PreconcurrencyAttr>()) {
-    auto preconcurrency =
-        new (ctx) PreconcurrencyAttr(/*isImplicit*/true);
-    value->addAttribute(preconcurrency);
+
+  if (isolationFromAttr && isolationFromAttr->preconcurrency()) {
+    markAsPreconcurrencyIfApplicable(value);
   }
 
   // Check if we inferred CallerIsolationInheriting from our isolation attr, but
@@ -6767,6 +6779,18 @@ static InferredActorIsolation computeActorIsolation(Evaluator &evaluator,
     // default.
     addAttributesForActorIsolation(value, defaultIsolation.isolation);
   }
+
+  // In `@MainActor` by default mode, let's inject the attributes when the
+  // isolation was inferred. This would make sure that the declaration is
+  // going to retain the isolation when printed in swift interface file and
+  // serialized.
+  if (value->getModuleContext() == ctx.MainModule &&
+      getDefaultIsolationForContext(value->getDeclContext()) ==
+          DefaultIsolation::MainActor &&
+      defaultIsolation.isolation.isMainActor()) {
+    addAttributesForActorIsolation(value, defaultIsolation.isolation);
+  }
+
   return defaultIsolation;
 }
 

test/Concurrency/approachable_concurrency.swift

@@ -16,7 +16,8 @@ func testSendableInference(s: S) {
   takesSendable(s.test)
 }
 
-// CHECK-LABEL: sil hidden [ossa] @$s24approachable_concurrency25testNonisolatedNonSendingyyyyYaYCXEYaF : $@convention(thin) @async (@guaranteed @noescape @async @callee_guaranteed (@sil_isolated @sil_implicit_leading_param @guaranteed Builtin.ImplicitActor) -> ()) -> ()
+// The declaration is @MainActor @preconcurrency because language mode is less than 6
+// CHECK-LABEL: sil hidden [ossa] @$s24approachable_concurrency25testNonisolatedNonSendingyyyyYaXEYaF : $@convention(thin) @async (@guaranteed @noescape @async @callee_guaranteed (@sil_isolated @sil_implicit_leading_param @guaranteed Builtin.ImplicitActor) -> ()) -> ()
 func testNonisolatedNonSending(_: () async -> Void) async {
 }
 

test/Concurrency/default_isolation_MainActor_cross_module.swift

@@ -0,0 +1,103 @@
+// RUN: %empty-directory(%t/src)
+// RUN: split-file %s %t/src
+
+/// Build the library A
+// RUN: %target-swift-frontend -emit-module %t/src/A.swift \
+// RUN:   -module-name A -swift-version 5 \
+// RUN:   -default-isolation MainActor \
+// RUN:   -enable-library-evolution \
+// RUN:     -emit-module-path %t/A.swiftmodule \
+// RUN:     -emit-module-interface-path %t/A.swiftinterface
+
+// RUN: %FileCheck %t/src/A.swift < %t/A.swiftinterface
+
+// Build the client using module
+// RUN: %target-swift-frontend -typecheck -verify -verify-ignore-unrelated -I %t -primary-file %t/src/Client.swift
+
+// RUN: rm %t/A.swiftmodule
+
+// Re-build the client using interface
+// RUN: %target-swift-frontend -typecheck -verify -verify-ignore-unrelated -I %t -primary-file %t/src/Client.swift
+
+// REQUIRES: concurrency
+
+//--- A.swift
+
+// CHECK: @_Concurrency.MainActor @preconcurrency public protocol P
+public protocol P {
+}
+
+// CHECK: nonisolated public protocol Q : Swift.Sendable
+nonisolated public protocol Q: Sendable {
+}
+
+// CHECK: @_Concurrency.MainActor @preconcurrency public struct S : A.P {
+public struct S: P {
+  // CHECK:   @_Concurrency.MainActor @preconcurrency public func f()
+  public func f() {}
+
+  // CHECK: @_Concurrency.MainActor @preconcurrency public struct Inner {
+  public struct Inner {
+    // CHECK: @_Concurrency.MainActor @preconcurrency public init()
+    public init() {}
+  }
+  // CHECK: }
+}
+// CHECK: }
+
+// CHECK: public struct R : A.Q {
+public struct R: Q {
+  // CHECK: public struct Inner {
+  public struct Inner {
+    // CHECK: public init()
+    public init() {}
+  }
+  // CHECK: }
+
+  // CHECK: @_Concurrency.MainActor @preconcurrency public struct InnerIsolated : A.P {
+  // CHECK: }
+  public struct InnerIsolated: P {}
+}
+// CHECK: }
+
+// CHECK: @_Concurrency.MainActor @preconcurrency public func testGlobal()
+public func testGlobal() {}
+
+// CHECK: @_Concurrency.MainActor @preconcurrency public class C {
+public class C {
+  // CHECK: @_Concurrency.MainActor @preconcurrency public init()
+  public init() {}
+
+  // CHECK: @_Concurrency.MainActor @preconcurrency public static var value: Swift.Int
+  public static var value = 42
+
+  // CHECK: {{(@objc )?}} @_Concurrency.MainActor deinit
+}
+// CHECK: }
+
+// CHECK: @_Concurrency.MainActor @preconcurrency open class IsolatedDeinitTest {
+open class IsolatedDeinitTest {
+  // CHECK:   {{(@objc )?}} isolated deinit
+  isolated deinit {}
+}
+// CHECK: }
+
+
+//--- Client.swift
+import A
+
+final class IsolatedDeinitChild: IsolatedDeinitTest {} // Ok
+
+nonisolated func testIsolation() {
+  testGlobal() // expected-warning {{call to main actor-isolated global function 'testGlobal()' in a synchronous nonisolated context}}
+
+  func test(s: S) {
+    s.f() // expected-warning {{call to main actor-isolated instance method 'f()' in a synchronous nonisolated context}}
+  }
+
+  _ = S.Inner() // expected-warning {{call to main actor-isolated initializer 'init()' in a synchronous nonisolated context}}
+  _ = R.Inner() // Ok
+
+  _ = C() // expected-warning {{call to main actor-isolated initializer 'init()' in a synchronous nonisolated context}}
+  _ = C.value // expected-warning {{main actor-isolated class property 'value' can not be referenced from a nonisolated context}}
+}

test/Concurrency/deinit_isolation_import/test.swift

@@ -259,7 +259,7 @@ class ProbeDefault_BaseIsolatedClass: BaseIsolatedClass {
 }
 
 // CHECK-LABEL: @objc @_inheritsConvenienceInitializers @MainActor @preconcurrency class ProbeIsolated_BaseIsolatedClass : BaseIsolatedClass {
-// CHECK: @objc @preconcurrency isolated deinit
+// CHECK: @objc isolated deinit
 // CHECK: }
 // CHECK-SYMB: ProbeIsolated_BaseIsolatedClass.__isolated_deallocating_deinit
 // CHECK-SYMB-NEXT: // Isolation: global_actor. type: MainActor
@@ -319,7 +319,7 @@ class ProbeDefault_DerivedIsolatedClass: DerivedIsolatedClass {
 }
 
 // CHECK-LABEL: @objc @_inheritsConvenienceInitializers @MainActor @preconcurrency class ProbeIsolated_DerivedIsolatedClass : DerivedIsolatedClass {
-// CHECK: @objc @preconcurrency isolated deinit
+// CHECK: @objc isolated deinit
 // CHECK: }
 // CHECK-SYMB: ProbeIsolated_DerivedIsolatedClass.__isolated_deallocating_deinit
 // CHECK-SYMB-NEXT: // Isolation: global_actor. type: MainActor

test/ModuleInterface/isolated_deinit.swift

@@ -0,0 +1,33 @@
+// RUN: %target-swift-frontend -emit-module %s \
+// RUN:   -module-name A -swift-version 5 \
+// RUN:   -disable-availability-checking \
+// RUN:   -enable-library-evolution \
+// RUN:     -emit-module-path %t/A.swiftmodule \
+// RUN:     -emit-module-interface-path %t/A.swiftinterface
+
+// RUN: %FileCheck %s < %t/A.swiftinterface
+
+// RUN: %target-swift-typecheck-module-from-interface(%t/A.swiftinterface)
+
+// REQUIRES: concurrency
+
+// CHECK: @_Concurrency.MainActor @preconcurrency public class C1 {
+// CHECK:   {{(@objc )?}} isolated deinit
+// CHECK: }
+
+@MainActor
+@preconcurrency
+public class C1 {
+  isolated deinit {
+  }
+}
+
+// CHECK: @preconcurrency public class C2 {
+// CHECK:   {{(@objc )?}} @_Concurrency.MainActor deinit
+// CHECK: }
+
+@preconcurrency
+public class C2 {
+  @MainActor deinit {
+  }
+}