Sema: Ignore global actor attr on accessors in Swift 6

Accesses to storage declarations are checked relative to the storage's
isolation, not the accessor's, whereas accessors are checked relative to
their own isolation. This inconsistency exposes a data race safety hole
because global actor attributes are allowed on accessors in Swift 5 and
even in Swift 6 on getters.

This fixes the bug by inferring accessor isolation from the storage in
Swift 6 and onward.

Expected source compatibility impact is negligible. First, an accessor
with a global actor attribute is not a common pattern. Second, this
change will only affect clients that depend on resilient dynamic
libraries that declare such an accessor as inlinable.

Author: AnthonyLatsis

include/swift/AST/DiagnosticsSema.def

@@ -6016,6 +6016,9 @@ ERROR(multiple_global_actors,none,
       (Identifier, Identifier))
 ERROR(global_actor_disallowed,none,
       "%kindonly0 cannot have a global actor", (const Decl *))
+NOTE(global_actor_attr_is_ignored,none,
+     "global actor attribute is ignored", ())
+
 ERROR(global_actor_on_actor_class,none,
       "actor %0 cannot have a global actor", (Identifier))
 ERROR(global_actor_on_local_variable,none,

lib/Sema/TypeCheckConcurrency.cpp

@@ -348,6 +348,10 @@ swift::checkGlobalActorAttributes(SourceLoc loc, DeclContext *dc,
   return std::make_pair(globalActorAttr, globalActorNominal);
 }
 
+static bool shouldIgnoreGlobalActorOnAccessor(const ASTContext &ctx) {
+  return ctx.isSwiftVersionAtLeast(6);
+}
+
 std::optional<std::pair<CustomAttr *, NominalTypeDecl *>>
 GlobalActorAttributeRequest::evaluate(
     Evaluator &evaluator,
@@ -452,12 +456,18 @@ GlobalActorAttributeRequest::evaluate(
     auto &ctx = accessor->getASTContext();
     const auto attrRange = globalActorAttr->getRangeWithAt();
     const unsigned langModeForError = accessor->isGetter() ? 7 : 6;
+    const bool isError = ctx.isSwiftVersionAtLeast(langModeForError);
 
     // Complain about a global actor attribute on an accessor.
     accessor->diagnose(diag::global_actor_disallowed, decl)
         .highlight(attrRange)
         .warnUntilSwiftVersion(langModeForError);
 
+    if (!isError && shouldIgnoreGlobalActorOnAccessor(ctx)) {
+      ctx.Diags.diagnose(attrRange.Start, diag::global_actor_attr_is_ignored)
+          .highlight(attrRange);
+    }
+
     auto *storage = accessor->getStorage();
 
     // Suggest to move the attribute to the storage if it is legal and the
@@ -476,7 +486,7 @@ GlobalActorAttributeRequest::evaluate(
     }
 
     // Fail if we emitted an error.
-    if (ctx.isSwiftVersionAtLeast(langModeForError))
+    if (isError)
       return std::nullopt;
 
   } else if (isa<ExtensionDecl>(decl) || isa<AbstractFunctionDecl>(decl)) {
@@ -5458,9 +5468,11 @@ getActorIsolationForMainFuncDecl(FuncDecl *fnDecl) {
 /// Check rules related to global actor attributes on a class declaration.
 ///
 /// \returns true if an error occurred.
-static bool checkClassGlobalActorIsolation(
-    ClassDecl *classDecl, ActorIsolation isolation) {
-  assert(isolation.isGlobalActor());
+static bool checkClassIsolation(ClassDecl *classDecl,
+                                ActorIsolation isolation) {
+  if (!isolation.isGlobalActor()) {
+    return false;
+  }
 
   // A class can only be annotated with a global actor if it has no
   // superclass, the superclass is annotated with the same global actor, or
@@ -5900,8 +5912,7 @@ static InferredActorIsolation computeActorIsolation(Evaluator &evaluator,
       if (isolationFromAttr && isolationFromAttr->isGlobalActor()) {
         if (!areTypesEqual(isolationFromAttr->getGlobalActor(),
                            mainIsolation->getGlobalActor())) {
-          fd->getASTContext().Diags.diagnose(
-              fd->getLoc(), diag::main_function_must_be_mainActor);
+          fd->diagnose(diag::main_function_must_be_mainActor);
         }
       }
       return {
@@ -5911,17 +5922,29 @@ static InferredActorIsolation computeActorIsolation(Evaluator &evaluator,
     }
   }
 
-  // If this declaration has one of the actor isolation attributes, report
-  // that.
+  // If this declaration has one of the actor isolation attributes, consider
+  // reporting that.
   if (isolationFromAttr) {
-    // Classes with global actors have additional rules regarding inheritance.
-    if (isolationFromAttr->isGlobalActor()) {
-      if (auto classDecl = dyn_cast<ClassDecl>(value))
-        checkClassGlobalActorIsolation(classDecl, *isolationFromAttr);
+    if (isa<AccessorDecl>(value) && shouldIgnoreGlobalActorOnAccessor(ctx)) {
+      // Global actor attributes on accessors are deprecated, whereas other
+      // isolation attributes are unconditionally unapplicable to accessors.
+      //
+      // In Swift 6 mode and onward, disregard the global actor attribute and
+      // proceed to infer the isolation from the storage declaration.
+    } else {
+      // Classes with global actors have additional rules regarding inheritance.
+      if (auto classDecl = dyn_cast<ClassDecl>(value)) {
+        checkClassIsolation(classDecl, *isolationFromAttr);
+      }
+
+      return {*isolationFromAttr,
+              IsolationSource(/*source*/ nullptr, IsolationSource::Explicit)};
     }
+  }
 
-    return {*isolationFromAttr,
-            IsolationSource(/*source*/ nullptr, IsolationSource::Explicit)};
+  // For an accessor, use the actor isolation of its storage declaration.
+  if (auto *accessor = dyn_cast<AccessorDecl>(value)) {
+    return getInferredActorIsolation(accessor->getStorage());
   }
 
   InferredActorIsolation defaultIsolation;
@@ -6037,12 +6060,6 @@ static InferredActorIsolation computeActorIsolation(Evaluator &evaluator,
     }
   }
 
-  // If this is an accessor, use the actor isolation of its storage
-  // declaration.
-  if (auto accessor = dyn_cast<AccessorDecl>(value)) {
-    return getInferredActorIsolation(accessor->getStorage());
-  }
-
   if (auto var = dyn_cast<VarDecl>(value)) {
     auto &ctx = var->getASTContext();
     if (!ctx.LangOpts.isConcurrencyModelTaskToThread() &&

test/Concurrency/ActorIsolationInference/Inputs/global_actor_accessors.swift

@@ -0,0 +1,154 @@
+actor SomeActor {}
+
+@globalActor
+struct GA1 {
+  static let shared = SomeActor()
+}
+@globalActor
+struct GA2 {
+  static let shared = SomeActor()
+}
+
+// expected-swift6-note@+1 * {{calls to global function 'ga2()' from outside of its actor context are implicitly asynchronous}}
+@GA2 func ga2() {}
+
+// Global actor on global var accessors.
+
+@GA1 var globalObserved: Int = 0 {
+  // expected-swift6-error@+2 {{cannot have a global actor}}
+  // expected-swift5-warning@+1 {{cannot have a global actor}}
+  @GA2 willSet {
+    ga2()
+    // expected-swift6-error@-1 {{call to global actor 'GA2'-isolated global function 'ga2()' in a synchronous global actor 'GA1'-isolated context}}
+  }
+  // expected-swift6-error@+2 {{cannot have a global actor}}
+  // expected-swift5-warning@+1 {{cannot have a global actor}}
+  @GA2 didSet {
+    ga2()
+    // expected-swift6-error@-1 {{call to global actor 'GA2'-isolated global function 'ga2()' in a synchronous global actor 'GA1'-isolated context}}
+  }
+}
+
+@GA1 var globalComputed: Int {
+  // expected-swift6-note@+3 {{global actor attribute is ignored}}
+  // expected-swift6-warning@+2 {{cannot have a global actor}}
+  // expected-swift5-warning@+1 {{cannot have a global actor}}
+  @GA2 get {
+    ga2()
+    // expected-swift6-error@-1 {{call to global actor 'GA2'-isolated global function 'ga2()' in a synchronous global actor 'GA1'-isolated context}}
+
+    return 0
+  }
+  // expected-swift6-error@+2 {{cannot have a global actor}}
+  // expected-swift5-warning@+1 {{cannot have a global actor}}
+  @GA2 set {
+    ga2()
+    // expected-swift6-error@-1 {{call to global actor 'GA2'-isolated global function 'ga2()' in a synchronous global actor 'GA1'-isolated context}}
+  }
+  // expected-swift6-error@+2 {{cannot have a global actor}}
+  // expected-swift5-warning@+1 {{cannot have a global actor}}
+  @GA2 _modify {
+    ga2()
+    // expected-swift6-error@-1 {{call to global actor 'GA2'-isolated global function 'ga2()' in a synchronous global actor 'GA1'-isolated context}}
+  }
+}
+
+@GA1 var globalComputedRead: Int {
+  // expected-swift6-error@+2 {{cannot have a global actor}}
+  // expected-swift5-warning@+1 {{cannot have a global actor}}
+  @GA2 _read {
+    ga2()
+    // expected-swift6-error@+-1 {{call to global actor 'GA2'-isolated global function 'ga2()' in a synchronous global actor 'GA1'-isolated context}}
+  }
+}
+
+@GA1 func testGlobalStorageDeclActorIsolation() {
+  let _ = globalObserved
+  globalObserved = 0
+
+  let _ = globalComputed
+  globalComputed = 0
+
+  let _ = globalComputedRead
+}
+
+// Global actor on property/subscript accessors.
+
+@GA1 struct TypeContext {
+  nonisolated init() {}
+
+  var observed: Int {
+    // expected-swift6-error@+3 {{cannot have a global actor}}
+    // expected-swift5-warning@+2 {{cannot have a global actor}}
+    // expected-note@+1 {{move global actor}}
+    @GA2 willSet {
+      ga2()
+      // expected-swift6-error@-1 {{call to global actor 'GA2'-isolated global function 'ga2()' in a synchronous global actor 'GA1'-isolated context}}
+    }
+    // expected-swift6-error@+3 {{cannot have a global actor}}
+    // expected-swift5-warning@+2 {{cannot have a global actor}}
+    // expected-note@+1 {{move global actor}}
+    @GA2 didSet {
+      ga2()
+      // expected-swift6-error@-1 {{call to global actor 'GA2'-isolated global function 'ga2()' in a synchronous global actor 'GA1'-isolated context}}
+    }
+  }
+
+  var computed: Int {
+    // expected-swift6-note@+4 {{global actor attribute is ignored}}
+    // expected-swift6-warning@+3 {{cannot have a global actor}}
+    // expected-swift5-warning@+2 {{cannot have a global actor}}
+    // expected-note@+1 {{move global actor}}
+    @GA2 get {
+      ga2()
+      // expected-swift6-error@-1 {{call to global actor 'GA2'-isolated global function 'ga2()' in a synchronous global actor 'GA1'-isolated context}}
+
+      return 0
+    }
+    // expected-swift6-error@+3 {{cannot have a global actor}}
+    // expected-swift5-warning@+2 {{cannot have a global actor}}
+    // expected-note@+1 {{move global actor}}
+    @GA2 set {
+      ga2()
+      // expected-swift6-error@-1 {{call to global actor 'GA2'-isolated global function 'ga2()' in a synchronous global actor 'GA1'-isolated context}}
+    }
+    // expected-swift6-error@+3 {{cannot have a global actor}}
+    // expected-swift5-warning@+2 {{cannot have a global actor}}
+    // expected-note@+1 {{move global actor}}
+    @GA2 _modify {
+      ga2()
+      // expected-swift6-error@-1 {{call to global actor 'GA2'-isolated global function 'ga2()' in a synchronous global actor 'GA1'-isolated context}}
+    }
+    // expected-swift6-error@+3 {{cannot have a global actor}}
+    // expected-swift5-warning@+2 {{cannot have a global actor}}
+    // expected-note@+1 {{move global actor}}
+    @GA2 init {
+      ga2()
+      // expected-swift6-error@-1 {{call to global actor 'GA2'-isolated global function 'ga2()' in a synchronous global actor 'GA1'-isolated context}}
+    }
+  }
+
+  var computedRead: Int {
+    // expected-swift6-error@+3 {{cannot have a global actor}}
+    // expected-swift5-warning@+2 {{cannot have a global actor}}
+    // expected-note@+1 {{move global actor}}
+    @GA2 _read {
+      ga2()
+      // expected-swift6-error@-1 {{call to global actor 'GA2'-isolated global function 'ga2()' in a synchronous global actor 'GA1'-isolated context}}
+    }
+  }
+}
+
+@GA1 func testMemberStorageDeclActorIsolation() {
+  var c = TypeContext()
+
+  let _ = c.observed
+  c.observed = 0
+
+  let _ = c.computed
+  c.computed = 0
+
+  let _ = c.computedRead
+}
+
+

test/Concurrency/ActorIsolationInference/global_actor_accessors_swift5.swift

@@ -0,0 +1,3 @@
+// RUN: %target-swift-frontend -typecheck -verify -swift-version 5 -parse-as-library -verify-additional-prefix swift5- %S/Inputs/global_actor_accessors.swift
+
+// REQUIRES: concurrency

test/Concurrency/ActorIsolationInference/global_actor_accessors_swift6.swift

@@ -0,0 +1,3 @@
+// RUN: %target-swift-frontend -typecheck -verify -swift-version 6 -parse-as-library -verify-additional-prefix swift6- -verify-additional-prefix swift6+- %S/Inputs/global_actor_accessors.swift
+
+// REQUIRES: concurrency