Merge pull request swiftlang#33982 from DougGregor/concurrency-actor-conformance

[Concurrency] Actor-isolated members cannot satisfy protocol requirements

Author: DougGregor

include/swift/AST/DiagnosticsSema.def

@@ -4167,6 +4167,13 @@ NOTE(actor_mutable_state,none,
 WARNING(shared_mutable_state_access,none,
         "reference to %0 %1 is not concurrency-safe because it involves "
         "shared mutable state", (DescriptiveDeclKind, DeclName))
+NOTE(actor_isolated_witness,none,
+     "actor-isolated %0 %1 cannot be used to satisfy a protocol requirement",
+     (DescriptiveDeclKind, DeclName))
+NOTE(actor_isolated_witness_could_be_async_handler,none,
+     "actor-isolated %0 %1 cannot be used to satisfy a protocol requirement; "
+     "did you mean to make it an asychronous handler?",
+     (DescriptiveDeclKind, DeclName))
 
 //------------------------------------------------------------------------------
 // MARK: Type Check Types

lib/Sema/MiscDiagnostics.cpp

@@ -17,6 +17,7 @@
 #include "MiscDiagnostics.h"
 #include "ConstraintSystem.h"
 #include "TypeCheckAvailability.h"
+#include "TypeCheckConcurrency.h"
 #include "TypeChecker.h"
 #include "swift/AST/ASTWalker.h"
 #include "swift/AST/NameLookup.h"

lib/Sema/TypeCheckConcurrency.cpp

@@ -13,6 +13,7 @@
 // This file implements type checking support for Swift's concurrency model.
 //
 //===----------------------------------------------------------------------===//
+#include "TypeCheckConcurrency.h"
 #include "TypeChecker.h"
 #include "swift/AST/ASTWalker.h"
 #include "swift/AST/ParameterList.h"
@@ -21,14 +22,7 @@
 
 using namespace swift;
 
-/// Check whether the @asyncHandler attribute can be applied to the given
-/// function declaration.
-///
-/// \param diagnose Whether to emit a diagnostic when a problem is encountered.
-///
-/// \returns \c true if there was a problem with adding the attribute, \c false
-/// otherwise.
-static bool checkAsyncHandler(FuncDecl *func, bool diagnose) {
+bool swift::checkAsyncHandler(FuncDecl *func, bool diagnose) {
   if (!func->getResultInterfaceType()->isVoid()) {
     if (diagnose) {
       func->diagnose(diag::asynchandler_returns_value)
@@ -233,30 +227,6 @@ class IsolationRestriction {
 
   explicit IsolationRestriction(Kind kind) : kind(kind) { }
 
-  /// Determine whether the given value is an instance member of an actor
-  /// class that is isolated to the current actor instance.
-  ///
-  /// \returns the type of the actor.
-  static ClassDecl *getActorIsolatingInstanceMember(ValueDecl *value) {
-    // Only instance members are isolated.
-    if (!value->isInstanceMember())
-      return nullptr;
-
-    // Are we within an actor class?
-    auto classDecl = value->getDeclContext()->getSelfClassDecl();
-    if (!classDecl || !classDecl->isActor())
-      return nullptr;
-
-    // Functions that are an asynchronous context can be accessed from anywhere.
-    if (auto func = dyn_cast<AbstractFunctionDecl>(value)) {
-      if (func->isAsyncContext())
-        return nullptr;
-    }
-
-    // This member is part of the isolated state.
-    return classDecl;
-  }
-
 public:
   Kind getKind() const { return kind; }
 
@@ -354,8 +324,7 @@ class IsolationRestriction {
         return forLocalCapture(decl->getDeclContext());
 
       // Protected actor instance members can only be accessed on 'self'.
-      if (auto actorClass = getActorIsolatingInstanceMember(
-              cast<ValueDecl>(decl)))
+      if (auto actorClass = getActorIsolatingMember(cast<ValueDecl>(decl)))
         return forActorSelf(actorClass);
 
       // All other accesses are unsafe.
@@ -635,3 +604,23 @@ void swift::checkActorIsolation(const Expr *expr, const DeclContext *dc) {
   ActorIsolationWalker walker(dc);
   const_cast<Expr *>(expr)->walk(walker);
 }
+
+ClassDecl *swift::getActorIsolatingMember(ValueDecl *value) {
+  // Only instance members are isolated.
+  if (!value->isInstanceMember())
+    return nullptr;
+
+  // Are we within an actor class?
+  auto classDecl = value->getDeclContext()->getSelfClassDecl();
+  if (!classDecl || !classDecl->isActor())
+    return nullptr;
+
+  // Functions that are an asynchronous context can be accessed from anywhere.
+  if (auto func = dyn_cast<AbstractFunctionDecl>(value)) {
+    if (func->isAsyncContext())
+      return nullptr;
+  }
+
+  // This member is part of the isolated state.
+  return classDecl;
+}

lib/Sema/TypeCheckConcurrency.h

@@ -0,0 +1,52 @@
+//===--- TypeCheckConcurrency.h - Concurrency -------------------*- C++ -*-===//
+//
+// This source file is part of the Swift.org open source project
+//
+// Copyright (c) 2014 - 2020 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
+//
+//===----------------------------------------------------------------------===//
+//
+// This file provides type checking support for Swift's concurrency model.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef SWIFT_SEMA_TYPECHECKCONCURRENCY_H
+#define SWIFT_SEMA_TYPECHECKCONCURRENCY_H
+
+namespace swift {
+
+class ClassDecl;
+class DeclContext;
+class Expr;
+class FuncDecl;
+class ValueDecl;
+
+/// Check whether the @asyncHandler attribute can be applied to the given
+/// function declaration.
+///
+/// \param diagnose Whether to emit a diagnostic when a problem is encountered.
+///
+/// \returns \c true if there was a problem with adding the attribute, \c false
+/// otherwise.
+bool checkAsyncHandler(FuncDecl *func, bool diagnose);
+
+/// Add notes suggesting the addition of 'async' or '@asyncHandler', as
+/// appropriate, to a diagnostic for a function that isn't an async context.
+void addAsyncNotes(FuncDecl *func);
+
+/// Check actor isolation rules.
+void checkActorIsolation(const Expr *expr, const DeclContext *dc);
+
+/// Determine whether the given value is an instance member of an actor
+/// class that is isolated to the current actor instance.
+///
+/// \returns the type of the actor.
+ClassDecl *getActorIsolatingMember(ValueDecl *value);
+
+} // end namespace swift
+
+#endif /* SWIFT_SEMA_TYPECHECKCONCURRENCY_H */

lib/Sema/TypeCheckDecl.cpp

@@ -19,8 +19,9 @@
 #include "DerivedConformances.h"
 #include "TypeChecker.h"
 #include "TypeCheckAccess.h"
-#include "TypeCheckDecl.h"
 #include "TypeCheckAvailability.h"
+#include "TypeCheckConcurrency.h"
+#include "TypeCheckDecl.h"
 #include "TypeCheckObjC.h"
 #include "TypeCheckType.h"
 #include "MiscDiagnostics.h"

lib/Sema/TypeCheckEffects.cpp

@@ -16,6 +16,7 @@
 //===----------------------------------------------------------------------===//
 
 #include "TypeChecker.h"
+#include "TypeCheckConcurrency.h"
 #include "swift/AST/ASTWalker.h"
 #include "swift/AST/DiagnosticsSema.h"
 #include "swift/AST/Initializer.h"

lib/Sema/TypeCheckProtocol.cpp

@@ -21,6 +21,7 @@
 #include "TypeAccessScopeChecker.h"
 #include "TypeCheckAccess.h"
 #include "TypeCheckAvailability.h"
+#include "TypeCheckConcurrency.h"
 #include "TypeCheckObjC.h"
 #include "swift/AST/ASTContext.h"
 #include "swift/AST/ASTMangler.h"
@@ -706,7 +707,6 @@ swift::matchWitness(
         !reqFnType->getExtInfo().isThrowing()) {
       return RequirementMatch(witness, MatchKind::ThrowsConflict);
     }
-
   } else {
     auto reqTypeIsIUO = req->isImplicitlyUnwrappedOptional();
     auto witnessTypeIsIUO = witness->isImplicitlyUnwrappedOptional();
@@ -727,6 +727,10 @@ swift::matchWitness(
     }
   }
 
+  // Actor-isolated witnesses cannot conform to protocol requirements.
+  if (getActorIsolatingMember(witness))
+    return RequirementMatch(witness, MatchKind::ActorIsolatedWitness);
+
   // Now finalize the match.
   auto result = finalize(anyRenaming, optionalAdjustments);
   // Check if the requirement's `@differentiable` attributes are satisfied by
@@ -2429,6 +2433,24 @@ diagnoseMatch(ModuleDecl *module, NormalProtocolConformance *conformance,
   case MatchKind::NonObjC:
     diags.diagnose(match.Witness, diag::protocol_witness_not_objc);
     break;
+  case MatchKind::ActorIsolatedWitness: {
+    bool canBeAsyncHandler = false;
+    if (auto witnessFunc = dyn_cast<FuncDecl>(match.Witness)) {
+      canBeAsyncHandler = !witnessFunc->isAsyncHandler() &&
+          !checkAsyncHandler(witnessFunc, /*diagnose=*/false);
+    }
+    auto diag = match.Witness->diagnose(
+        canBeAsyncHandler ? diag::actor_isolated_witness_could_be_async_handler
+                          : diag::actor_isolated_witness,
+        match.Witness->getDescriptiveKind(), match.Witness->getName());
+
+    if (canBeAsyncHandler) {
+      diag.fixItInsert(
+          match.Witness->getAttributeInsertionLoc(false), "@asyncHandler ");
+    }
+    break;
+  }
+
   case MatchKind::MissingDifferentiableAttr: {
     auto *witness = match.Witness;
     // Emit a note and fix-it showing the missing requirement `@differentiable`

lib/Sema/TypeCheckProtocol.h

@@ -268,6 +268,9 @@ enum class MatchKind : uint8_t {
   /// The witness is explicitly @nonobjc but the requirement is @objc.
   NonObjC,
 
+  /// The witness is part of actor-isolated state.
+  ActorIsolatedWitness,
+
   /// The witness is missing a `@differentiable` attribute from the requirement.
   MissingDifferentiableAttr,
 
@@ -500,6 +503,7 @@ struct RequirementMatch {
     case MatchKind::AsyncConflict:
     case MatchKind::ThrowsConflict:
     case MatchKind::NonObjC:
+    case MatchKind::ActorIsolatedWitness:
     case MatchKind::MissingDifferentiableAttr:
     case MatchKind::EnumCaseWithAssociatedValues:
       return false;
@@ -532,6 +536,7 @@ struct RequirementMatch {
     case MatchKind::AsyncConflict:
     case MatchKind::ThrowsConflict:
     case MatchKind::NonObjC:
+    case MatchKind::ActorIsolatedWitness:
     case MatchKind::MissingDifferentiableAttr:
     case MatchKind::EnumCaseWithAssociatedValues:
       return false;

lib/Sema/TypeChecker.h

@@ -1384,13 +1384,6 @@ void checkUnknownAttrRestrictions(
 /// it to later stages.
 void bindSwitchCasePatternVars(DeclContext *dc, CaseStmt *stmt);
 
-/// Add notes suggesting the addition of 'async' or '@asyncHandler', as
-/// appropriate, to a diagnostic for a function that isn't an async context.
-void addAsyncNotes(FuncDecl *func);
-
-/// Check actor isolation rules.
-void checkActorIsolation(const Expr *expr, const DeclContext *dc);
-
 } // end namespace swift
 
 #endif

test/decl/class/actor/conformance.swift

@@ -0,0 +1,55 @@
+// RUN: %target-typecheck-verify-swift -enable-experimental-concurrency
+
+protocol AsyncProtocol {
+  func asyncMethod() async -> Int
+}
+
+actor class MyActor {
+}
+
+// Actors conforming to asynchronous program.
+extension MyActor: AsyncProtocol {
+  func asyncMethod() async -> Int { return 0 }
+}
+
+// FIXME: "Do you want to add a stub?" diagnostics should be suppressed here.
+protocol SyncProtocol {
+  var propertyA: Int { get }
+  // expected-note@-1{{do you want to add a stub}}
+  var propertyB: Int { get set }
+  // expected-note@-1{{do you want to add a stub}}
+
+  func syncMethodA()
+  // expected-note@-1{{do you want to add a stub}}
+
+  func syncMethodB()
+
+  subscript (index: Int) -> String { get }
+  // expected-note@-1{{do you want to add a stub}}
+
+  static func staticMethod()
+  static var staticProperty: Int { get }
+}
+
+
+actor class OtherActor: SyncProtocol { // expected-error{{type 'OtherActor' does not conform to protocol 'SyncProtocol'}}
+  var propertyB: Int = 17
+  // expected-note@-1{{actor-isolated property 'propertyB' cannot be used to satisfy a protocol requirement}}
+
+  var propertyA: Int { 17 }
+  // expected-note@-1{{actor-isolated property 'propertyA' cannot be used to satisfy a protocol requirement}}
+
+  func syncMethodA() { }
+  // expected-note@-1{{actor-isolated instance method 'syncMethodA()' cannot be used to satisfy a protocol requirement; did you mean to make it an asychronous handler?}}{{3-3=@asyncHandler }}
+
+  // Async handlers are okay.
+  @asyncHandler
+  func syncMethodB() { }
+
+  subscript (index: Int) -> String { "\(index)" }
+  // expected-note@-1{{actor-isolated subscript 'subscript(_:)' cannot be used to satisfy a protocol requirement}}
+
+  // Static methods and properties are okay.
+  static func staticMethod() { }
+  static var staticProperty: Int = 17
+}