Merge pull request #69219 from xedin/rdar-99281333

[TypeChecker] Disallow use of global actor attributes of setters, add…

Author: xedin

include/swift/AST/DiagnosticsSema.def

@@ -5670,6 +5670,8 @@ ERROR(global_actor_access,none,
 ERROR(global_actor_not_usable_from_inline,none,
       "global actor for %kind0 must be '@usableFromInline' or public",
       (const ValueDecl *))
+NOTE(move_global_actor_attr_to_storage_decl,none,
+     "move global actor attribute to %kind0", (const ValueDecl *))
 
 ERROR(actor_isolation_multiple_attr,none,
       "%kind0 has multiple actor-isolation attributes ('%1' and '%2')",

lib/Sema/TypeCheckConcurrency.cpp

@@ -373,6 +373,15 @@ GlobalActorAttributeRequest::evaluate(
   if (decl->getDeclContext()->getParentSourceFile() == nullptr)
     return result;
 
+  auto isStoredInstancePropertyOfStruct = [](VarDecl *var) {
+    if (var->isStatic() || !var->isOrdinaryStoredProperty())
+      return false;
+
+    auto *nominal = var->getDeclContext()->getSelfNominalTypeDecl();
+    return isa_and_nonnull<StructDecl>(nominal) &&
+           !isWrappedValueOfPropWrapper(var);
+  };
+
   auto globalActorAttr = result->first;
   if (auto nominal = dyn_cast<NominalTypeDecl>(decl)) {
     // Nominal types are okay...
@@ -406,25 +415,53 @@ GlobalActorAttributeRequest::evaluate(
       }
 
       // ... and not if it's the instance storage of a struct
-      if (!var->isStatic() && var->isOrdinaryStoredProperty()) {
-        if (auto *nominal = var->getDeclContext()->getSelfNominalTypeDecl()) {
-          if (isa<StructDecl>(nominal) && !isWrappedValueOfPropWrapper(var)) {
-
-            var->diagnose(diag::global_actor_on_storage_of_value_type,
-                          var->getName())
-              .highlight(globalActorAttr->getRangeWithAt())
-              .warnUntilSwiftVersion(6);
-
-            // In Swift 6, once the diag above is an error, it is disallowed.
-            if (var->getASTContext().isSwiftVersionAtLeast(6))
-              return llvm::None;
-          }
-        }
+      if (isStoredInstancePropertyOfStruct(var)) {
+        var->diagnose(diag::global_actor_on_storage_of_value_type,
+                      var->getName())
+            .highlight(globalActorAttr->getRangeWithAt())
+            .warnUntilSwiftVersion(6);
+
+        // In Swift 6, once the diag above is an error, it is disallowed.
+        if (var->getASTContext().isSwiftVersionAtLeast(6))
+          return llvm::None;
       }
     }
   } else if (isa<ExtensionDecl>(decl)) {
     // Extensions are okay.
   } else if (isa<ConstructorDecl>(decl) || isa<FuncDecl>(decl)) {
+    // None of the accessors/addressors besides a getter are allowed
+    // to have a global actor attribute.
+    if (auto *accessor = dyn_cast<AccessorDecl>(decl)) {
+      if (!accessor->isGetter()) {
+        decl->diagnose(diag::global_actor_disallowed,
+                       decl->getDescriptiveKind())
+            .fixItRemove(globalActorAttr->getRangeWithAt());
+
+        auto *storage = accessor->getStorage();
+        // Let's suggest to move the attribute to the storage if
+        // this is an accessor/addressor of a property of subscript.
+        if (storage->getDeclContext()->isTypeContext()) {
+          // If enclosing declaration has a global actor,
+          // skip the suggestion.
+          if (storage->getGlobalActorAttr())
+            return llvm::None;
+
+          // Global actor attribute cannot be applied to
+          // an instance stored property of a struct.
+          if (auto *var = dyn_cast<VarDecl>(storage)) {
+            if (isStoredInstancePropertyOfStruct(var))
+              return llvm::None;
+          }
+
+          decl->diagnose(diag::move_global_actor_attr_to_storage_decl, storage)
+              .fixItInsert(
+                  storage->getAttributeInsertionLoc(/*forModifier=*/false),
+                  llvm::Twine("@", result->second->getNameStr()).str());
+        }
+
+        return llvm::None;
+      }
+    }
     // Functions are okay.
   } else {
     // Everything else is disallowed.

test/attr/global_actor.swift

@@ -149,3 +149,60 @@ public struct PublicGA {
 @InternalGA open class OpenClassInternalGA {} // expected-error {{open class 'OpenClassInternalGA' cannot have internal global actor 'InternalGA'}}
 @PackageGA open class OpenClassPackageGA {} // expected-error {{open class 'OpenClassPackageGA' cannot have package global actor 'PackageGA'}}
 @PublicGA open class OpenClassPublicGA {}
+
+// rdar://99281333 - no accessors/addressors/observers expect to 'get' are allowed to have a global actor attribute
+do {
+  class TestInvalidAccessors {
+    var test1: Int {
+      get { 42 }
+      @GA1
+      set { } // expected-error {{setter cannot have a global actor}} {{158:7-11=}}
+      // expected-note@-1 {{move global actor attribute to property 'test1'}} {{156:5-5=@GA1}}
+
+      @GA1 _modify { fatalError() } // expected-error {{_modify accessor cannot have a global actor}} {{7-12=}}
+      // expected-note@-1 {{move global actor attribute to property 'test1'}} {{156:5-5=@GA1}}
+    }
+
+    func local() {
+      var test: Bool {
+        get { false }
+
+        @GA1
+        set { } // expected-error {{setter cannot have a global actor}}
+      }
+    }
+
+    @GA1 var testAlreadyWithGlobal: String {
+      get { "" }
+      @GA1 set { } // expected-error {{setter cannot have a global actor}} {{7-12=}}
+    }
+  }
+
+  struct TestStruct {
+    var test1: Int {
+      get { 42 }
+      @GA1
+      set { } // expected-error {{setter cannot have a global actor}} {{184:7-11=}}
+      // expected-note@-1 {{move global actor attribute to property 'test1'}} {{182:5-5=@GA1}}
+      @GA1 _modify { fatalError() } // expected-error {{_modify accessor cannot have a global actor}} {{7-12=}}
+      // expected-note@-1 {{move global actor attribute to property 'test1'}} {{182:5-5=@GA1}}
+    }
+
+    var test2: Int {
+      @GA1 willSet { // expected-error {{willSet observer cannot have a global actor}} {{7-12=}}
+        // expected-note@-1 {{move global actor attribute to property 'test2'}} {{191:5-5=@GA1}}
+      }
+    }
+
+    subscript(x: Int) -> Bool {
+      get { true }
+      @GA1 set { } // expected-error {{setter cannot have a global actor}} {{7-12=}}
+      // expected-note@-1 {{move global actor attribute to subscript 'subscript(_:)'}} {{197:5-5=@GA1}}
+    }
+
+    @GA1 subscript(y: Bool) -> String {
+      get { "" }
+      @GA1 set { } // expected-error {{setter cannot have a global actor}} {{7-12=}}
+    }
+  }
+}