Merge pull request #83341 from DougGregor/default-isolation-extensions-6.2

[6.2] Extensions and members thereof can apply default isolation

Author: DougGregor

include/swift/Basic/Features.def

@@ -332,6 +332,10 @@ EXPERIMENTAL_FEATURE(LayoutPrespecialization, true)
 
 EXPERIMENTAL_FEATURE(AccessLevelOnImport, true)
 
+/// Disable the special behavior of of explicit 'nonisolated' vs. being
+/// implicitly nonisolated.
+EXPERIMENTAL_FEATURE(NoExplicitNonIsolated, true)
+
 /// Enables a module to allow non resilient access from other
 /// modules within a package if built from source.
 EXPERIMENTAL_FEATURE(AllowNonResilientAccessInPackage, false)

lib/AST/FeatureSet.cpp

@@ -127,6 +127,7 @@ UNINTERESTING_FEATURE(MacrosOnImports)
 UNINTERESTING_FEATURE(ExtensibleEnums)
 UNINTERESTING_FEATURE(NonisolatedNonsendingByDefault)
 UNINTERESTING_FEATURE(KeyPathWithMethodMembers)
+UNINTERESTING_FEATURE(NoExplicitNonIsolated)
 
 static bool usesFeatureNonescapableTypes(Decl *decl) {
   auto containsNonEscapable =

lib/Frontend/CompilerInvocation.cpp

@@ -1878,8 +1878,10 @@ static bool ParseLangArgs(LangOptions &Opts, ArgList &Args,
     Opts.DefaultIsolationBehavior = DefaultIsolation::Nonisolated;
   }
 
-  if (Opts.DefaultIsolationBehavior == DefaultIsolation::MainActor)
+  if (Opts.DefaultIsolationBehavior == DefaultIsolation::MainActor) {
     Opts.enableFeature(Feature::InferIsolatedConformances);
+    Opts.enableFeature(Feature::NoExplicitNonIsolated);
+  }
 
 #if !defined(NDEBUG) && SWIFT_ENABLE_EXPERIMENTAL_PARSER_VALIDATION
   /// Enable round trip parsing via the new swift parser unless it is disabled

lib/Sema/TypeCheckConcurrency.cpp

@@ -5301,6 +5301,33 @@ getIsolationFromAttributes(const Decl *decl, bool shouldDiagnose = true,
   llvm_unreachable("Forgot about an attribute?");
 }
 
+/// Determine the default isolation for the given declaration context.
+static DefaultIsolation getDefaultIsolationForContext(const DeclContext *dc) {
+  // Check whether there is a file-specific setting.
+  if (auto *sourceFile = dc->getParentSourceFile()) {
+    if (auto defaultIsolationInFile = sourceFile->getDefaultIsolation())
+      return defaultIsolationInFile.value();
+  }
+
+  // If we're in the main module, check the language option.
+  ASTContext &ctx = dc->getASTContext();
+  if (dc->getParentModule() == ctx.MainModule)
+    return ctx.LangOpts.DefaultIsolationBehavior;
+
+  // Otherwise, default to nonisolated.
+  return DefaultIsolation::Nonisolated;
+}
+
+/// Determines whether explicit 'nonisolated' is different from 'unspecified'
+/// in the given context.
+static bool explicitNonisolatedIsSpecial(const DeclContext *dc) {
+  ASTContext &ctx = dc->getASTContext();
+  if (ctx.LangOpts.hasFeature(Feature::NoExplicitNonIsolated))
+    return false;
+
+  return getDefaultIsolationForContext(dc) == DefaultIsolation::Nonisolated;
+}
+
 /// Infer isolation from witnessed protocol requirements.
 static std::optional<InferredActorIsolation>
 getIsolationFromWitnessedRequirements(ValueDecl *value) {
@@ -5317,11 +5344,13 @@ getIsolationFromWitnessedRequirements(ValueDecl *value) {
   if (dc->getSelfProtocolDecl())
     return std::nullopt;
 
-  // Prevent isolation inference from requirements if the conforming type
-  // has an explicit `nonisolated` attribute.
-  if (auto *NTD = dc->getSelfNominalTypeDecl()) {
-    if (NTD->getAttrs().hasAttribute<NonisolatedAttr>())
-      return std::nullopt;
+  if (explicitNonisolatedIsSpecial(dc)) {
+    // Prevent isolation inference from requirements if the conforming type
+    // has an explicit `nonisolated` attribute.
+    if (auto *NTD = dc->getSelfNominalTypeDecl()) {
+      if (NTD->getAttrs().hasAttribute<NonisolatedAttr>())
+        return std::nullopt;
+    }
   }
 
   // Walk through each of the conformances in this context, collecting any
@@ -5379,8 +5408,13 @@ getIsolationFromWitnessedRequirements(ValueDecl *value) {
       }
 
       case ActorIsolation::GlobalActor:
+        break;
+
       case ActorIsolation::Nonisolated:
       case ActorIsolation::NonisolatedUnsafe:
+        if (!explicitNonisolatedIsSpecial(requirement->getDeclContext()))
+          continue;
+
         break;
       }
 
@@ -5488,7 +5522,8 @@ getIsolationFromConformances(NominalTypeDecl *nominal) {
     case ActorIsolation::NonisolatedUnsafe:
       break;
     case ActorIsolation::Nonisolated:
-      if (inferredIsolation.source.kind == IsolationSource::Kind::Explicit) {
+      if (inferredIsolation.source.kind == IsolationSource::Kind::Explicit &&
+          explicitNonisolatedIsSpecial(nominal)) {
         if (!foundIsolation) {
           // We found an explicitly 'nonisolated' protocol.
           foundIsolation = {
@@ -6152,12 +6187,27 @@ computeDefaultInferredActorIsolation(ValueDecl *value) {
             return {};
 
           if (dc != dyn_cast<DeclContext>(value)) {
+            // If the nominal type is global-actor-isolated, there's nothing
+            // more to look for.
             if (getActorIsolation(nominal).isMainActor())
               break;
 
-            return {};
+            // If this is an extension of a nonisolated type, its isolation
+            // is independent of the type.
+            if (auto ext = dyn_cast<ExtensionDecl>(dc)) {
+              // If there were isolation attributes on the extension, respect
+              // them.
+              if (getIsolationFromAttributes(ext).has_value())
+                return {};
+
+              // Keep looking.
+            } else {
+              // The type is nonisolated, so its members are nonisolated.
+              return {};
+            }
           }
         }
+
         dc = dc->getParent();
       }
 
@@ -6187,12 +6237,8 @@ computeDefaultInferredActorIsolation(ValueDecl *value) {
       return {};
     };
 
-    DefaultIsolation defaultIsolation = ctx.LangOpts.DefaultIsolationBehavior;
-    if (auto *SF = value->getDeclContext()->getParentSourceFile()) {
-      if (auto defaultIsolationInFile = SF->getDefaultIsolation())
-        defaultIsolation = defaultIsolationInFile.value();
-    }
-
+    DefaultIsolation defaultIsolation =
+        getDefaultIsolationForContext(value->getDeclContext());
     // If we are required to use main actor... just use that.
     if (defaultIsolation == DefaultIsolation::MainActor)
       if (auto result =
@@ -6269,6 +6315,36 @@ computeDefaultInferredActorIsolation(ValueDecl *value) {
   return {{ActorIsolation::forUnspecified(), {}}, nullptr, {}};
 }
 
+/// Determines when the given "self" isolation should override default
+/// isolation.
+static bool shouldSelfIsolationOverrideDefault(
+    ASTContext &ctx, const DeclContext *dc,
+    const ActorIsolation &selfIsolation) {
+  switch (selfIsolation) {
+  case ActorIsolation::ActorInstance:
+  case ActorIsolation::Erased:
+  case ActorIsolation::GlobalActor:
+    // Actor isolation always overrides.
+    return true;
+
+  case ActorIsolation::Unspecified:
+    // Unspecified isolation never overrides.
+    return false;
+
+  case ActorIsolation::Nonisolated:
+  case ActorIsolation::NonisolatedUnsafe:
+  case ActorIsolation::CallerIsolationInheriting:
+    // Explicit nonisolated used to overwrite default isolation all the time,
+    // but under NoExplicitNonIsolated it doesn't affect extensions.
+    if (isa<NominalTypeDecl>(dc))
+      return true;
+
+    /// Only allow explicit nonisolated to override the default when it's
+    /// treated as special.
+    return explicitNonisolatedIsSpecial(dc);
+  }
+}
+
 static InferredActorIsolation computeActorIsolation(Evaluator &evaluator,
                                                     ValueDecl *value) {
   // If this declaration has actor-isolated "self", it's isolated to that
@@ -6601,7 +6677,8 @@ static InferredActorIsolation computeActorIsolation(Evaluator &evaluator,
     // has isolation, use that.
     if (auto selfTypeDecl = value->getDeclContext()->getSelfNominalTypeDecl()) {
       auto selfTypeIsolation = getInferredActorIsolation(selfTypeDecl);
-      if (selfTypeIsolation.isolation) {
+      if (shouldSelfIsolationOverrideDefault(
+              ctx, value->getDeclContext(), selfTypeIsolation.isolation)) {
         auto isolation = selfTypeIsolation.isolation;
 
         if (ctx.LangOpts.hasFeature(Feature::NonisolatedNonsendingByDefault) &&
@@ -8439,6 +8516,19 @@ ActorIsolation swift::inferConformanceIsolation(
   // isolated to a global actor, we may use the conforming type's isolation.
   auto nominalIsolation = getActorIsolation(nominal);
   if (!nominalIsolation.isGlobalActor()) {
+    // If we are in an extension of the type, we might still infer an
+    // isolated conformance depending on default isolation and on the extension
+    // itself.
+    if (auto ext = dyn_cast<ExtensionDecl>(dc)) {
+      // If there's an isolation-related attribute on the extension, use it.
+      if (auto attrIsolation = getIsolationFromAttributes(ext))
+        return *attrIsolation;
+
+      // If we're defaulting to main-actor isolation, use that.
+      if (getDefaultIsolationForContext(dc) == DefaultIsolation::MainActor)
+        return ActorIsolation::forMainActor(ctx);
+    }
+
     return ActorIsolation::forNonisolated(false);
   }
 

test/Concurrency/assume_mainactor_typechecker_errors.swift

@@ -79,3 +79,70 @@ func testTaskDetached() async {
     await k.doSomething()
   }
 }
+
+// @MainActor
+extension Int {
+  func memberOfInt() { } // expected-note 2{{calls to instance method 'memberOfInt()' from outside of its actor context are implicitly asynchronous}}
+}
+
+nonisolated func testMemberOfInt(i: Int) {
+  i.memberOfInt() // expected-swift5-warning{{call to main actor-isolated instance method 'memberOfInt()' in a synchronous nonisolated context}}
+  // expected-swift6-error@-1{{call to main actor-isolated instance method 'memberOfInt()' in a synchronous nonisolated context}}
+}
+
+protocol SendableProto: Sendable { }
+
+struct MyStruct: SendableProto { }
+
+extension MyStruct: CustomStringConvertible {
+  var description: String {
+    17.memberOfInt() // okay, on main actor
+    return "hello"
+  }
+}
+
+nonisolated struct MyOtherStruct { }
+
+extension MyOtherStruct {
+  func f() {
+    17.memberOfInt() // okay, on main actor
+  }
+}
+
+nonisolated
+extension MyOtherStruct {
+  func g() {
+    17.memberOfInt() // expected-swift5-warning{{call to main actor-isolated instance method 'memberOfInt()' in a synchronous nonisolated context}}
+  // expected-swift6-error@-1{{call to main actor-isolated instance method 'memberOfInt()' in a synchronous nonisolated context}}
+  }
+}
+
+nonisolated protocol P {
+  func g()
+}
+
+struct MyP: P {
+  func g() {
+    17.memberOfInt() // okay, on main actor
+  }
+}
+
+// https://github.com/swiftlang/swift/issues/82168 -
+nonisolated protocol OtherP {
+  associatedtype AT
+  static var at: AT { get }
+}
+
+nonisolated struct KP<R: OtherP, V> {
+  init(keyPath: KeyPath<R, V>) {}
+}
+
+struct S: OtherP {
+  let p: Int
+  struct AT {
+    let kp = KP(keyPath: \S.p)
+  }
+
+  // FIXME: This should not be an error.
+  static let at = AT() // expected-swift6-error{{'S.AT' cannot be constructed because it has no accessible initializers}}
+}

test/Concurrency/isolated_conformance_default_actor.swift

@@ -2,6 +2,8 @@
 
 // REQUIRES: concurrency
 
+@MainActor func onMain() { }
+
 nonisolated
 protocol P {
   func f()
@@ -12,8 +14,14 @@ protocol Q {
   func g()
 }
 
+// expected-note@+4{{turn data races into runtime errors with '@preconcurrency'}}
+// expected-note@+3{{isolate this conformance to the main actor with '@MainActor'}}
+// expected-note@+2{{mark all declarations used in the conformance 'nonisolated'}}
+// expected-error@+1{{conformance of 'CImplicitMainActorNonisolatedConformance' to protocol 'P' crosses into main actor-isolated code and can cause data races}}
 class CImplicitMainActorNonisolatedConformance: nonisolated P {
-  func f() { } // error: explicitly nonisolated conformance
+  func f() { // expected-note{{main actor-isolated instance method 'f()' cannot satisfy nonisolated requirement}}
+    onMain() // okay, f is on @MainActor
+  }
 }
 
 
@@ -73,9 +81,6 @@ func acceptSendablePMeta<T: Sendable & P>(_: T.Type) { }
 func acceptSendableQMeta<T: Sendable & Q>(_: T.Type) { }
 
 nonisolated func testConformancesFromNonisolated() {
-  let _: any P = CExplicitMainActor() // okay
-  let _: any P = CImplicitMainActor() // okay
-
   let _: any P = CNonIsolated()
   let _: any P = CImplicitMainActorNonisolatedConformance()
 
@@ -84,6 +89,10 @@ nonisolated func testConformancesFromNonisolated() {
   let _: any Q = CImplicitMainActor()
 
   // Error, these are main-actor-isolated conformances
+  let _: any P = CExplicitMainActor() // expected-error{{main actor-isolated conformance of 'CExplicitMainActor' to 'P' cannot be used in nonisolated context}}
+  let _: any P = CImplicitMainActor() // expected-error{{main actor-isolated conformance of 'CImplicitMainActor' to 'P' cannot be used in nonisolated context}}
+
+
   let _: any Equatable.Type = EquatableStruct.self // expected-error{{main actor-isolated conformance of 'EquatableStruct' to 'Equatable' cannot be used in nonisolated context}}
   let _: any Hashable.Type = HashableStruct.self // expected-error{{main actor-isolated conformance of 'HashableStruct' to 'Hashable' cannot be used in nonisolated context}}
   let _: any RawRepresentable.Type = RawRepresentableEnum.self