[Concurrency] Check actor isolation consistency for overrides & subclasses.

Both overriding declarations and subclasses must have the actor
isolation as their overridden declarations or superclasses,
respectively. Enforce this, ensuring that we're also doing the
appropriate substitutions.

Author: DougGregor

include/swift/AST/ActorIsolation.h

@@ -25,6 +25,7 @@ class raw_ostream;
 
 namespace swift {
 class ClassDecl;
+class SubstitutionMap;
 class Type;
 
 /// Determine whether the given types are (canonically) equal, declared here
@@ -97,6 +98,13 @@ class ActorIsolation {
     return globalActor;
   }
 
+  /// Determine whether this isolation will require substitution to be
+  /// evaluated.
+  bool requiresSubstitution() const;
+
+  /// Substitute into types within the actor isolation.
+  ActorIsolation subst(SubstitutionMap subs) const;
+
   friend bool operator==(const ActorIsolation &lhs,
                          const ActorIsolation &rhs) {
     if (lhs.kind != rhs.kind)

include/swift/AST/DiagnosticsSema.def

@@ -4262,6 +4262,13 @@ ERROR(actor_isolation_multiple_attr,none,
       "%0 %1 has multiple actor-isolation attributes ('%2' and '%3')",
       (DescriptiveDeclKind, DeclName, StringRef, StringRef))
 
+ERROR(actor_isolation_override_mismatch,none,
+      "%0 %1 %2 has different actor isolation from %3 overridden declaration",
+      (ActorIsolation, DescriptiveDeclKind, DeclName, ActorIsolation))
+ERROR(actor_isolation_superclass_mismatch,none,
+      "%0 class %1 has different actor isolation from %2 superclass %3",
+      (ActorIsolation, Identifier, ActorIsolation, Identifier))
+
 //------------------------------------------------------------------------------
 // MARK: Type Check Types
 //------------------------------------------------------------------------------

lib/AST/TypeCheckRequests.cpp

@@ -1447,6 +1447,29 @@ void CustomAttrTypeRequest::cacheResult(Type value) const {
   attr->setType(value);
 }
 
+bool ActorIsolation::requiresSubstitution() const {
+  switch (kind) {
+  case ActorInstance:
+  case Independent:
+  case Unspecified:
+    return false;
+
+  case GlobalActor:
+    return getGlobalActor()->hasTypeParameter();
+  }
+}
+
+ActorIsolation ActorIsolation::subst(SubstitutionMap subs) const {
+  switch (kind) {
+  case ActorInstance:
+  case Independent:
+  case Unspecified:
+    return *this;
+
+  case GlobalActor:
+    return forGlobalActor(getGlobalActor().subst(subs));
+  }
+}
 
 void swift::simple_display(
     llvm::raw_ostream &out, const ActorIsolation &state) {

lib/Sema/TypeCheckConcurrency.cpp

@@ -1155,8 +1155,16 @@ ActorIsolation ActorIsolationRequest::evaluate(
   // If the declaration overrides another declaration, it must have the same
   // actor isolation.
   if (auto overriddenValue = value->getOverriddenDecl()) {
-    if (auto isolation = getActorIsolation(overriddenValue))
-      return inferredIsolation(isolation);
+    if (auto isolation = getActorIsolation(overriddenValue)) {
+      SubstitutionMap subs;
+      if (auto env = value->getInnermostDeclContext()
+              ->getGenericEnvironmentOfContext()) {
+        subs = SubstitutionMap::getOverrideSubstitutions(
+            overriddenValue, value, subs);
+      }
+
+      return inferredIsolation(isolation.subst(subs));
+    }
   }
 
   // If the declaration witnesses a protocol requirement that is isolated,
@@ -1210,3 +1218,65 @@ ActorIsolation swift::getActorIsolation(ValueDecl *value) {
       ctx.evaluator, ActorIsolationRequest{value},
       ActorIsolation::forUnspecified());
 }
+
+void swift::checkOverrideActorIsolation(ValueDecl *value) {
+  if (isa<TypeDecl>(value))
+    return;
+
+  auto overridden = value->getOverriddenDecl();
+  if (!overridden)
+    return;
+
+  // Determine the actor isolation of this declaration.
+  auto isolation = getActorIsolation(value);
+
+  // Determine the actor isolation of the overridden function.=
+  auto overriddenIsolation = getActorIsolation(overridden);
+
+  if (overriddenIsolation.requiresSubstitution()) {
+    SubstitutionMap subs;
+    if (auto env = value->getInnermostDeclContext()
+            ->getGenericEnvironmentOfContext()) {
+      subs = SubstitutionMap::getOverrideSubstitutions(overridden, value, subs);
+      overriddenIsolation = overriddenIsolation.subst(subs);
+    }
+  }
+
+  // If the isolation matches, we're done.
+  if (isolation == overriddenIsolation)
+    return;
+
+  // Isolation mismatch. Diagnose it.
+  value->diagnose(
+      diag::actor_isolation_override_mismatch, isolation,
+      value->getDescriptiveKind(), value->getName(), overriddenIsolation);
+  overridden->diagnose(diag::overridden_here);
+}
+
+void swift::checkSubclassActorIsolation(ClassDecl *classDecl) {
+  auto superclassDecl = classDecl->getSuperclassDecl();
+  if (!superclassDecl)
+    return;
+
+  auto isolation = getActorIsolation(classDecl);
+  auto superclassIsolation = getActorIsolation(superclassDecl);
+
+  if (superclassIsolation.requiresSubstitution()) {
+    Type superclassType = classDecl->getSuperclass();
+    if (!superclassType)
+      return;
+
+    SubstitutionMap subs = superclassType->getMemberSubstitutionMap(
+        classDecl->getModuleContext(), classDecl);
+    superclassIsolation = superclassIsolation.subst(subs);
+  }
+
+  if (isolation == superclassIsolation)
+    return;
+
+  // Diagnose mismatch.
+  classDecl->diagnose(
+      diag::actor_isolation_superclass_mismatch, isolation,
+      classDecl->getName(), superclassIsolation, superclassDecl->getName());
+  superclassDecl->diagnose(diag::decl_declared_here, superclassDecl->getName());
+}

lib/Sema/TypeCheckConcurrency.h

@@ -144,6 +144,13 @@ class ActorIsolationRestriction {
   operator Kind() const { return kind; };
 };
 
+/// Check that the actor isolation of an override matches that of its
+/// overridden declaration.
+void checkOverrideActorIsolation(ValueDecl *value);
+
+/// Check that the actor isolation of a class matches that of its superclass.
+void checkSubclassActorIsolation(ClassDecl *classDecl);
+
 } // end namespace swift
 
 #endif /* SWIFT_SEMA_TYPECHECKCONCURRENCY_H */

lib/Sema/TypeCheckDeclPrimary.cpp

@@ -21,6 +21,7 @@
 #include "DerivedConformances.h"
 #include "TypeChecker.h"
 #include "TypeCheckAccess.h"
+#include "TypeCheckConcurrency.h"
 #include "TypeCheckDecl.h"
 #include "TypeCheckAvailability.h"
 #include "TypeCheckObjC.h"
@@ -1382,12 +1383,17 @@ class DeclChecker : public DeclVisitor<DeclChecker> {
       (void) VD->getFormalAccess();
 
       // Compute overrides.
-      (void) VD->getOverriddenDecls();
+      if (!VD->getOverriddenDecls().empty())
+        checkOverrideActorIsolation(VD);
 
       // Check whether the member is @objc or dynamic.
       (void) VD->isObjC();
       (void) VD->isDynamic();
 
+      // For a class, check actor isolation.
+      if (auto classDecl = dyn_cast<ClassDecl>(VD))
+        checkSubclassActorIsolation(classDecl);
+
       // If this is a member of a nominal type, don't allow it to have a name of
       // "Type" or "Protocol" since we reserve the X.Type and X.Protocol
       // expressions to mean something builtin to the language.  We *do* allow

test/Concurrency/global_actor_inference.swift

@@ -15,6 +15,11 @@ struct OtherGlobalActor {
   static let shared = SomeActor()
 }
 
+@globalActor
+struct GenericGlobalActor<T> {
+  static var shared: SomeActor { SomeActor() }
+}
+
 // ----------------------------------------------------------------------
 // Global actor inference for protocols
 // ----------------------------------------------------------------------
@@ -25,11 +30,10 @@ protocol P1 {
 }
 
 protocol P2 {
-  @SomeGlobalActor func method1()
+  @SomeGlobalActor func method1() // expected-note {{'method1()' declared here}}
   func method2()
 }
 
-
 class C1: P1 {
   func method() { } // expected-note{{only asynchronous methods can be used outside the actor instance}}
 
@@ -88,3 +92,55 @@ class C5 {
   c5.method1() // OK: no propagation
   c5.method2() // expected-error{{instance method 'method2()' isolated to global actor 'SomeGlobalActor' can not be referenced from different global actor 'OtherGlobalActor'}}
 }
+
+protocol P3 {
+  @OtherGlobalActor func method1() // expected-note{{'method1()' declared here}}
+  func method2()
+}
+
+class C6: P2, P3 {
+  func method1() { }
+    // expected-error@-1{{instance method 'method1()' must be isolated to the global actor 'SomeGlobalActor' to satisfy corresponding requirement from protocol 'P2'}}
+    // expected-error@-2{{instance method 'method1()' must be isolated to the global actor 'OtherGlobalActor' to satisfy corresponding requirement from protocol 'P3'}}
+  func method2() { }
+
+  func testMethod() {
+    method1() // okay: no inference
+    method2() // okay: no inference
+  }
+}
+
+// ----------------------------------------------------------------------
+// Global actor checking for overrides
+// ----------------------------------------------------------------------
+actor class GenericSuper<T> {
+  @GenericGlobalActor<T> func method() { }
+
+  @GenericGlobalActor<T> func method2() { } // expected-note {{overridden declaration is here}}
+  @GenericGlobalActor<T> func method3() { } // expected-note {{overridden declaration is here}}
+  @GenericGlobalActor<T> func method4() { }
+  @GenericGlobalActor<T> func method5() { }
+}
+
+actor class GenericSub<T> : GenericSuper<[T]> {
+  override func method() { } // expected-note{{only asynchronous methods can be used outside the actor instance; do you want to add 'async'?}}
+
+  @GenericGlobalActor<T> override func method2() { } // expected-error{{global actor 'GenericGlobalActor<T>'-isolated instance method 'method2()' has different actor isolation from global actor 'GenericGlobalActor<[T]>'-isolated overridden declaration}}
+  @actorIndependent override func method3() { } // expected-error{{actor-independent instance method 'method3()' has different actor isolation from global actor 'GenericGlobalActor<[T]>'-isolated overridden declaration}}
+
+  @OtherGlobalActor func testMethod() {
+    method() // expected-error{{instance method 'method()' isolated to global actor 'GenericGlobalActor<[T]>' can not be referenced from different global actor 'OtherGlobalActor'}}
+  }
+}
+
+// ----------------------------------------------------------------------
+// Global actor checking for supeclasses
+// ----------------------------------------------------------------------
+struct Container<T> {
+  @GenericGlobalActor<T> class Superclass { } // expected-note{{'Superclass' declared here}}
+}
+
+struct OtherContainer<U> {
+  @GenericGlobalActor<[U]> class Subclass1 : Container<[U]>.Superclass { }
+  @GenericGlobalActor<U> class Subclass2 : Container<[U]>.Superclass { } // expected-error{{global actor 'GenericGlobalActor<U>'-isolated class 'Subclass2' has different actor isolation from global actor 'GenericGlobalActor<[U]>'-isolated superclass 'Superclass'}}
+}