Merge pull request #37619 from apple/revert-37462-remove-actor-independent

Revert "Remove `@actorIndependent` attribute."

Author: lorentey

include/swift/AST/Attr.def

@@ -574,7 +574,12 @@ CONTEXTUAL_SIMPLE_DECL_ATTR(actor, Actor,
   APIBreakingToAdd | APIBreakingToRemove,
   102)
 
-// Unused attribute 103
+DECL_ATTR(actorIndependent, ActorIndependent,
+  OnClass | OnStruct | OnEnum | OnExtension | OnFunc | OnConstructor |
+  OnVar | OnSubscript | ConcurrencyOnly |
+  ABIBreakingToAdd | ABIBreakingToRemove |
+  APIBreakingToAdd | APIBreakingToRemove,
+  103)
 
 SIMPLE_DECL_ATTR(globalActor, GlobalActor,
   OnClass | OnStruct | OnEnum | ConcurrencyOnly |

include/swift/AST/Attr.h

@@ -162,6 +162,10 @@ class DeclAttribute : public AttributeBase {
       kind : NumInlineKindBits
     );
 
+    SWIFT_INLINE_BITFIELD(ActorIndependentAttr, DeclAttribute, NumActorIndependentKindBits,
+      kind : NumActorIndependentKindBits
+    );
+
     SWIFT_INLINE_BITFIELD(OptimizeAttr, DeclAttribute, NumOptimizationModeBits,
       mode : NumOptimizationModeBits
     );
@@ -1212,6 +1216,25 @@ class ReferenceOwnershipAttr : public DeclAttribute {
   }
 };
 
+/// Represents an actorIndependent/actorIndependent(unsafe) decl attribute.
+class ActorIndependentAttr : public DeclAttribute {
+public:
+  ActorIndependentAttr(SourceLoc atLoc, SourceRange range, ActorIndependentKind kind)
+      : DeclAttribute(DAK_ActorIndependent, atLoc, range, /*Implicit=*/false) {
+    Bits.ActorIndependentAttr.kind = unsigned(kind);
+  }
+
+  ActorIndependentAttr(ActorIndependentKind kind, bool IsImplicit=false)
+    : ActorIndependentAttr(SourceLoc(), SourceRange(), kind) {
+      setImplicit(IsImplicit);
+    }
+
+  ActorIndependentKind getKind() const { return ActorIndependentKind(Bits.ActorIndependentAttr.kind); }
+  static bool classof(const DeclAttribute *DA) {
+    return DA->getKind() == DAK_ActorIndependent;
+  }
+};
+
 /// Defines the attribute that we use to model documentation comments.
 class RawDocCommentAttr : public DeclAttribute {
   /// Source range of the attached comment.  This comment is located before

include/swift/AST/AttrKind.h

@@ -80,6 +80,16 @@ enum : unsigned { NumInlineKindBits =
   countBitsUsed(static_cast<unsigned>(InlineKind::Last_InlineKind)) };
 
 
+/// Indicates whether an actorIndependent decl is unsafe or not
+enum class ActorIndependentKind : uint8_t {
+  Safe = 0,
+  Unsafe = 1,
+  Last_InlineKind = Unsafe
+};
+
+enum : unsigned { NumActorIndependentKindBits =
+  countBitsUsed(static_cast<unsigned>(ActorIndependentKind::Last_InlineKind)) };
+
 /// This enum represents the possible values of the @_effects attribute.
 /// These values are ordered from the strongest guarantee to the weakest,
 /// so please do not reorder existing values.

include/swift/AST/DiagnosticsSema.def

@@ -4489,12 +4489,23 @@ ERROR(concurrent_value_inherit,none,
       "%select{| other than 'NSObject'}0",
       (bool, DeclName))
 
+ERROR(actorindependent_let,none,
+      "'@actorIndependent' is meaningless on 'let' declarations because "
+      "they are immutable",
+      ())
+ERROR(actorindependent_mutable_storage,none,
+      "'@actorIndependent' can not be applied to stored properties",
+      ())
+ERROR(actorindependent_local_var,none,
+      "'@actorIndependent' can not be applied to local variables",
+      ())
+
 ERROR(nonisolated_let,none,
       "'nonisolated' is meaningless on 'let' declarations because "
       "they are immutable",
       ())
 ERROR(nonisolated_mutable_storage,none,
-      "'nonisolated' can not be applied to stored properties",
+      "nonisolated' can not be applied to stored properties",
       ())
 ERROR(nonisolated_local_var,none,
       "'nonisolated' can not be applied to local variables",

lib/AST/Attr.cpp

@@ -790,6 +790,7 @@ bool DeclAttribute::printImpl(ASTPrinter &Printer, const PrintOptions &Options,
   case DAK_ReferenceOwnership:
   case DAK_Effects:
   case DAK_Optimize:
+  case DAK_ActorIndependent:
     if (DeclAttribute::isDeclModifier(getKind())) {
       Printer.printKeyword(getAttrName(), Options);
     } else if (Options.IsForSwiftInterface && getKind() == DAK_ResultBuilder) {
@@ -1177,6 +1178,15 @@ StringRef DeclAttribute::getAttrName() const {
     }
     llvm_unreachable("Invalid inline kind");
   }
+  case DAK_ActorIndependent: {
+    switch (cast<ActorIndependentAttr>(this)->getKind()) {
+    case ActorIndependentKind::Safe:
+      return "actorIndependent";
+    case ActorIndependentKind::Unsafe:
+      return "actorIndependent(unsafe)";
+    }
+    llvm_unreachable("Invalid actorIndependent kind");
+  }
   case DAK_Optimize: {
     switch (cast<OptimizeAttr>(this)->getMode()) {
     case OptimizationMode::NoOptimization:

lib/ClangImporter/ImportDecl.cpp

@@ -8706,14 +8706,6 @@ void ClangImporter::Implementation::importAttributes(
         continue;
       }
 
-      // Hard-code @actorIndependent, until Objective-C clients start
-      // using nonisolated.
-      if (swiftAttr->getAttribute() == "@actorIndependent") {
-        auto attr = new (SwiftContext) NonisolatedAttr(/*isImplicit=*/true);
-        MappedDecl->getAttrs().add(attr);
-        continue;
-      }
-
       // Dig out a buffer with the attribute text.
       unsigned bufferID = getClangSwiftAttrSourceBuffer(
           swiftAttr->getAttribute());
@@ -9638,8 +9630,9 @@ ClangImporter::Implementation::createConstant(Identifier name, DeclContext *dc,
 
   // Mark the function transparent so that we inline it away completely.
   func->getAttrs().add(new (C) TransparentAttr(/*implicit*/ true));
-  auto nonisolatedAttr = new (C) NonisolatedAttr(/*IsImplicit=*/true);
-  var->getAttrs().add(nonisolatedAttr);
+  auto actorIndependentAttr = new (C) ActorIndependentAttr(
+      ActorIndependentKind::Unsafe, /*IsImplicit=*/true);
+  var->getAttrs().add(actorIndependentAttr);
 
   // Set the function up as the getter.
   makeComputed(var, func, nullptr);

lib/Parse/ParseDecl.cpp

@@ -1784,6 +1784,45 @@ bool Parser::parseNewDeclAttribute(DeclAttributes &Attributes, SourceLoc AtLoc,
     break;
   }
 
+  case DAK_ActorIndependent: {
+    // if no option is provided, then it's the 'safe' version.
+    if (!consumeIf(tok::l_paren)) {
+      if (!DiscardAttribute) {
+        AttrRange = SourceRange(Loc, Tok.getRange().getStart());
+        Attributes.add(new (Context) ActorIndependentAttr(AtLoc, AttrRange, 
+                                                  ActorIndependentKind::Safe));
+      }
+      break;
+    }
+
+    // otherwise, make sure it looks like an identifier.
+    if (Tok.isNot(tok::identifier)) {
+      diagnose(Loc, diag::attr_expected_option_such_as, AttrName, "unsafe");
+      return false;
+    }
+
+    // make sure the identifier is 'unsafe'
+    if (Tok.getText() != "unsafe") {
+      diagnose(Loc, diag::attr_unknown_option, Tok.getText(), AttrName);
+      return false;
+    }
+
+    consumeToken(tok::identifier);
+    AttrRange = SourceRange(Loc, Tok.getRange().getStart());
+    
+    if (!consumeIf(tok::r_paren)) {
+      diagnose(Loc, diag::attr_expected_rparen, AttrName,
+               DeclAttribute::isDeclModifier(DK));
+      return false;
+    }
+
+    if (!DiscardAttribute)
+      Attributes.add(new (Context) ActorIndependentAttr(AtLoc, AttrRange, 
+                                                ActorIndependentKind::Unsafe));
+
+    break;
+  }
+
   case DAK_Optimize: {
     if (!consumeIf(tok::l_paren)) {
       diagnose(Loc, diag::attr_expected_lparen, AttrName,

lib/Sema/TypeCheckAttr.cpp

@@ -251,6 +251,7 @@ class AttributeChecker : public AttributeVisitor<AttributeChecker> {
   void visitTransposeAttr(TransposeAttr *attr);
 
   void visitActorAttr(ActorAttr *attr);
+  void visitActorIndependentAttr(ActorIndependentAttr *attr);
   void visitGlobalActorAttr(GlobalActorAttr *attr);
   void visitAsyncAttr(AsyncAttr *attr);
   void visitSpawnAttr(SpawnAttr *attr);
@@ -5381,6 +5382,48 @@ void AttributeChecker::visitActorAttr(ActorAttr *attr) {
   (void)classDecl->isActor();
 }
 
+void AttributeChecker::visitActorIndependentAttr(ActorIndependentAttr *attr) {
+  // @actorIndependent can be applied to global and static/class variables
+  // that do not have storage.
+  auto dc = D->getDeclContext();
+  if (auto var = dyn_cast<VarDecl>(D)) {
+    // @actorIndependent is meaningless on a `let`.
+    if (var->isLet()) {
+      diagnoseAndRemoveAttr(attr, diag::actorindependent_let);
+      return;
+    }
+
+    // @actorIndependent can not be applied to stored properties, unless if
+    // the 'unsafe' option was specified
+    if (var->hasStorage()) {
+      switch (attr->getKind()) {
+        case ActorIndependentKind::Safe:
+          diagnoseAndRemoveAttr(attr, diag::actorindependent_mutable_storage);
+          return;
+
+        case ActorIndependentKind::Unsafe:
+          break;
+      }
+    }
+
+    // @actorIndependent can not be applied to local properties.
+    if (dc->isLocalContext()) {
+      diagnoseAndRemoveAttr(attr, diag::actorindependent_local_var);
+      return;
+    }
+
+    // If this is a static or global variable, we're all set.
+    if (dc->isModuleScopeContext() ||
+        (dc->isTypeContext() && var->isStatic())) {
+      return;
+    }
+  }
+
+  if (auto VD = dyn_cast<ValueDecl>(D)) {
+    (void)getActorIsolation(VD);
+  }
+}
+
 void AttributeChecker::visitNonisolatedAttr(NonisolatedAttr *attr) {
   // 'nonisolated' can be applied to global and static/class variables
   // that do not have storage.
@@ -5398,7 +5441,7 @@ void AttributeChecker::visitNonisolatedAttr(NonisolatedAttr *attr) {
       return;
     }
 
-    // nonisolated can not be applied to local properties.
+    // @actorIndependent can not be applied to local properties.
     if (dc->isLocalContext()) {
       diagnoseAndRemoveAttr(attr, diag::nonisolated_local_var);
       return;
@@ -5551,6 +5594,10 @@ class ClosureAttributeChecker
     // Nothing else to check.
   }
 
+  void visitActorIndependentAttr(ActorIndependentAttr *attr) {
+    // Nothing else to check.
+  }
+
   void visitCustomAttr(CustomAttr *attr) {
     // Check whether this custom attribute is the global actor attribute.
     auto globalActorAttr = evaluateOrDefault(

lib/Sema/TypeCheckConcurrency.cpp

@@ -2115,6 +2115,9 @@ namespace {
         AbstractClosureExpr *closure) {
       // If the closure specifies a global actor, use it.
       if (auto explicitClosure = dyn_cast<ClosureExpr>(closure)) {
+        if (explicitClosure->getAttrs().hasAttribute<ActorIndependentAttr>())
+          return ClosureActorIsolation::forIndependent();
+
         if (Type globalActorType = resolveGlobalActorType(explicitClosure))
           return ClosureActorIsolation::forGlobalActor(globalActorType);
 
@@ -2246,19 +2249,23 @@ static Optional<ActorIsolation> getIsolationFromAttributes(
     const Decl *decl, bool shouldDiagnose = true, bool onlyExplicit = false) {
   // Look up attributes on the declaration that can affect its actor isolation.
   // If any of them are present, use that attribute.
+  auto independentAttr = decl->getAttrs().getAttribute<ActorIndependentAttr>();
   auto nonisolatedAttr = decl->getAttrs().getAttribute<NonisolatedAttr>();
   auto globalActorAttr = decl->getGlobalActorAttr();
 
   // Remove implicit attributes if we only care about explicit ones.
   if (onlyExplicit) {
+    if (independentAttr && independentAttr->isImplicit())
+      independentAttr = nullptr;
     if (nonisolatedAttr && nonisolatedAttr->isImplicit())
       nonisolatedAttr = nullptr;
     if (globalActorAttr && globalActorAttr->first->isImplicit())
       globalActorAttr = None;
   }
 
   unsigned numIsolationAttrs =
-    (nonisolatedAttr ? 1 : 0) + (globalActorAttr ? 1 : 0);
+    (nonisolatedAttr ? 1 : 0) + (independentAttr ? 1 : 0) +
+    (globalActorAttr ? 1 : 0);
   if (numIsolationAttrs == 0)
     return None;
 
@@ -2274,12 +2281,22 @@ static Optional<ActorIsolation> getIsolationFromAttributes(
     }
 
     if (globalActorAttr) {
+      StringRef nonisolatedAttrName;
+      SourceRange nonisolatedRange;
+      if (independentAttr) {
+        nonisolatedAttrName = independentAttr->getAttrName();
+        nonisolatedRange = independentAttr->getRangeWithAt();
+      } else {
+        nonisolatedAttrName = nonisolatedAttr->getAttrName();
+        nonisolatedRange = nonisolatedAttr->getRangeWithAt();
+      }
+
       if (shouldDiagnose) {
         decl->diagnose(
             diag::actor_isolation_multiple_attr, decl->getDescriptiveKind(),
-            name, nonisolatedAttr->getAttrName(),
+            name, nonisolatedAttrName,
             globalActorAttr->second->getName().str())
-          .highlight(nonisolatedAttr->getRangeWithAt())
+          .highlight(nonisolatedRange)
           .highlight(globalActorAttr->first->getRangeWithAt());
       }
     }
@@ -2291,6 +2308,12 @@ static Optional<ActorIsolation> getIsolationFromAttributes(
     return ActorIsolation::forIndependent();
   }
 
+  // If the declaration is explicitly marked @actorIndependent, report it as
+  // independent.
+  if (independentAttr) {
+    return ActorIsolation::forIndependent();
+  }
+
   // If the declaration is marked with a global actor, report it as being
   // part of that global actor.
   if (globalActorAttr) {
@@ -2380,7 +2403,7 @@ static Optional<ActorIsolation> getIsolationFromWitnessedRequirements(
         llvm_unreachable("protocol requirements cannot be actor instances");
 
       case ActorIsolation::Independent:
-        // We only need one nonisolated.
+        // We only need one @actorIndependent.
         if (sawActorIndependent)
           return true;
 
@@ -2593,7 +2616,8 @@ ActorIsolation ActorIsolationRequest::evaluate(
       if (onlyGlobal)
         return ActorIsolation::forUnspecified();
 
-      value->getAttrs().add(new (ctx) NonisolatedAttr(/*IsImplicit=*/true));
+      value->getAttrs().add(new (ctx) ActorIndependentAttr(
+                              ActorIndependentKind::Safe, /*IsImplicit=*/true));
       break;
 
     case ActorIsolation::GlobalActorUnsafe:
@@ -2842,9 +2866,12 @@ void swift::checkOverrideActorIsolation(ValueDecl *value) {
 bool swift::contextUsesConcurrencyFeatures(const DeclContext *dc) {
   while (!dc->isModuleScopeContext()) {
     if (auto closure = dyn_cast<AbstractClosureExpr>(dc)) {
-      // A closure with an explicit global actor or nonindependent
+      // A closure with an explicit global actor or @actorIndependent
       // uses concurrency features.
       if (auto explicitClosure = dyn_cast<ClosureExpr>(closure)) {
+        if (explicitClosure->getAttrs().hasAttribute<ActorIndependentAttr>())
+          return true;
+
         if (getExplicitGlobalActor(const_cast<ClosureExpr *>(explicitClosure)))
           return true;
       }

lib/Sema/TypeCheckDeclOverride.cpp

@@ -1533,6 +1533,7 @@ namespace  {
     UNINTERESTING_ATTR(ProjectedValueProperty)
     UNINTERESTING_ATTR(OriginallyDefinedIn)
     UNINTERESTING_ATTR(Actor)
+    UNINTERESTING_ATTR(ActorIndependent)
     UNINTERESTING_ATTR(GlobalActor)
     UNINTERESTING_ATTR(Async)
     UNINTERESTING_ATTR(Spawn)

lib/Serialization/Deserialization.cpp

@@ -4284,6 +4284,14 @@ llvm::Error DeclDeserializer::deserializeDeclCommon() {
         break;
       }
 
+      case decls_block::ActorIndependent_DECL_ATTR: {
+        unsigned kind;
+        serialization::decls_block::ActorIndependentDeclAttrLayout::readRecord(
+            scratch, kind);
+        Attr = new (ctx) ActorIndependentAttr((ActorIndependentKind)kind);
+        break;
+      }
+
       case decls_block::Optimize_DECL_ATTR: {
         unsigned kind;
         serialization::decls_block::OptimizeDeclAttrLayout::readRecord(