🍒[5.7][Concurrency] Settable prop reqs cannot be witnessed by actors

include/swift/AST/DiagnosticsSema.def

@@ -4682,6 +4682,9 @@ ERROR(actor_isolated_witness,none,
      "%select{|distributed }0%1 %2 %3 cannot be used to satisfy %4 protocol "
      "requirement",
      (bool, ActorIsolation, DescriptiveDeclKind, DeclName, ActorIsolation))
+ERROR(actor_isolated_mutable_property_witness,none,
+     "%0 %1 %2 cannot be used to satisfy settable property protocol requirement",
+     (ActorIsolation, DescriptiveDeclKind, DeclName))
 ERROR(actor_cannot_conform_to_global_actor_protocol,none,
       "actor %0 cannot conform to global actor isolated protocol %1",
       (Type, Type))

lib/Sema/TypeCheckProtocol.cpp

@@ -2961,6 +2961,21 @@ Optional<ActorIsolation> ConformanceChecker::checkActorIsolation(
   auto refResult = ActorReferenceResult::forReference(
       getConcreteWitness(), witness->getLoc(), DC, None, None,
       None, requirementIsolation);
+
+  // Limit the behavior of the diagnostic based on context.
+  // If we're working with requirements imported from Clang, or with global
+  // actor isolation in general, use the default diagnostic behavior based
+  // on the conformance context.
+  DiagnosticBehavior behavior = DiagnosticBehavior::Unspecified;
+  if (requirement->hasClangNode() ||
+      refResult.isolation.isGlobalActor() ||
+      requirementIsolation.isGlobalActor()) {
+    // If the witness or requirement has global actor isolation, downgrade
+    // based on context.
+    behavior = SendableCheckContext(
+                   Conformance->getDeclContext()).defaultDiagnosticBehavior();
+  }
+
   bool sameConcurrencyDomain = false;
   switch (refResult) {
   case ActorReferenceResult::SameConcurrencyDomain:
@@ -2971,6 +2986,31 @@ Optional<ActorIsolation> ConformanceChecker::checkActorIsolation(
       break;
     }
 
+    if (refResult.isolation) {
+      /// A property inside a class/actor may be unable to be witness to the
+      /// requirement, if it has a setter, since those cannot be async. Structs
+      /// are except from this since they don't need isolation.
+      auto classDecl = dyn_cast<ClassDecl>(witness->getDeclContext());
+      auto var = dyn_cast<VarDecl>(witness);
+      if (classDecl && var) {
+        // An immutable property may be as witness for
+        // an actor-isolated protocol requirement.
+        if (var->isLet() || var->getWriteImpl() == WriteImplKind::Immutable) {
+          return None;
+        }
+
+        // We're trying to witness an actor-isolated get/set requirement with an
+        // actor which is impossible since we cannot express the async setter.
+        witness
+            ->diagnose(diag::actor_isolated_mutable_property_witness,
+                       refResult.isolation, witness->getDescriptiveKind(),
+                       witness->getName())
+            .limitBehavior(behavior);
+
+        return None;
+      }
+    }
+
     // Otherwise, we're done.
     return None;
 
@@ -3068,20 +3108,6 @@ Optional<ActorIsolation> ConformanceChecker::checkActorIsolation(
     return None;
   }
 
-  // Limit the behavior of the diagnostic based on context.
-  // If we're working with requirements imported from Clang, or with global
-  // actor isolation in general, use the default diagnostic behavior based
-  // on the conformance context.
-  DiagnosticBehavior behavior = DiagnosticBehavior::Unspecified;
-  if (requirement->hasClangNode() ||
-      refResult.isolation.isGlobalActor() ||
-      requirementIsolation.isGlobalActor()) {
-    // If the witness or requirement has global actor isolation, downgrade
-    // based on context.
-    behavior = SendableCheckContext(
-        Conformance->getDeclContext()).defaultDiagnosticBehavior();
-  }
-
   // Complain that this witness cannot conform to the requirement due to
   // actor isolation.
   witness->diagnose(diag::actor_isolated_witness,
@@ -5076,10 +5102,17 @@ void ConformanceChecker::resolveValueWitnesses() {
       // Check actor isolation. If we need to enter into the actor's
       // isolation within the witness thunk, record that.
       if (auto enteringIsolation = checkActorIsolation(requirement, witness)) {
-        Conformance->overrideWitness(
-            requirement,
-            Conformance->getWitnessUncached(requirement)
-              .withEnterIsolation(*enteringIsolation));
+
+        if (auto var = dyn_cast<VarDecl>(witness)) {
+          if (var->getWriteImpl() != WriteImplKind::Immutable) {
+            Conformance->setInvalid();
+          }
+        } else {
+
+          Conformance->overrideWitness(
+              requirement, Conformance->getWitnessUncached(requirement)
+                               .withEnterIsolation(*enteringIsolation));
+        }
       }
 
       // Objective-C checking for @objc requirements.

test/Concurrency/actor_isolation.swift

@@ -118,7 +118,17 @@ protocol MainCounter {
 
 struct InferredFromConformance: MainCounter {
   var counter = 0
+  var ticker: Int { // this is ok, only because this is in a struct
+    get { 1 }
+    set {}
+  }
+}
+
+class InferredFromConformanceClass: MainCounter {
+  var counter = 0
+  // expected-warning@-1{{actor-isolated property 'counter' cannot be used to satisfy settable property protocol requirement}}
   var ticker: Int {
+    // expected-warning@-1{{actor-isolated property 'ticker' cannot be used to satisfy settable property protocol requirement}}
     get { 1 }
     set {}
   }

test/Concurrency/actor_isolation_settable_property_via_protocol.swift

@@ -0,0 +1,38 @@
+// RUN: %empty-directory(%t)
+// RUN: %target-swift-frontend -emit-module -emit-module-path %t/OtherActors.swiftmodule -module-name OtherActors %S/Inputs/OtherActors.swift -disable-availability-checking
+// RUN: %target-typecheck-verify-swift -I %t  -disable-availability-checking -warn-concurrency -parse-as-library
+// REQUIRES: concurrency
+
+actor A1 {
+  var pa = 0
+  // expected-error@-1{{actor-isolated property 'pa' cannot be used to satisfy settable property protocol requirement}}
+  // expected-note@-2{{mutation of this property is only permitted within the actor}}
+}
+
+actor A2 {
+  var pa: Int {
+    // expected-error@-1{{actor-isolated property 'pa' cannot be used to satisfy settable property protocol requirement}}
+    get async { 1 }
+    set { _ = newValue }
+    // expected-error@-1{{'set' accessor is not allowed on property with 'get' accessor that is 'async' or 'throws'}}
+  }
+}
+
+protocol PA: Actor {
+  var pa: Int { get set }
+}
+extension A1: PA {}
+extension A2: PA {}
+
+func test_a1(act: A1) async {
+  await act.pa = 222
+  // expected-error@-1{{actor-isolated property 'pa' can not be mutated from a non-isolated context}}
+  // expected-warning@-2{{no 'async' operations occur within 'await' expression}}
+
+  // ==== Access through protocol ----------------------------------------------
+
+  await (act as any PA).pa = 42 // Not allowed since act does not even conform to PA to begin with
+
+  let anyPA: any PA = act
+  await anyPA.pa = 777 // Not allowed since act does not even conform to PA to begin with
+}