[Sema] Fix and cleanup distributed id/actorSystem synthesis (#85245)

Author: hamishknight

include/swift/AST/Decl.h

@@ -374,6 +374,12 @@ bool conflicting(ASTContext &ctx,
                  bool *wouldConflictInSwift5 = nullptr,
                  bool skipProtocolExtensionCheck = false);
 
+/// The kind of special compiler synthesized property in a \c distributed actor,
+/// currently this includes \c id and \c actorSystem.
+enum class SpecialDistributedProperty {
+  Id, ActorSystem
+};
+
 /// The kind of artificial main to generate.
 enum class ArtificialMainKind : uint8_t {
   NSApplicationMain,
@@ -3056,6 +3062,12 @@ class ValueDecl : public Decl {
   /// `distributed var get { }` accessors.
   bool isDistributedGetAccessor() const;
 
+  /// Whether this is the special synthesized 'id' or 'actorSystem' property
+  /// of a distributed actor. If \p onlyCheckName is set, then any
+  /// matching user-defined property with the name is also considered.
+  std::optional<SpecialDistributedProperty>
+  isSpecialDistributedProperty(bool onlyCheckName = false) const;
+
   bool hasName() const { return bool(Name); }
   bool isOperator() const { return Name.isOperator(); }
 
@@ -5430,7 +5442,6 @@ enum class KnownDerivableProtocolKind : uint8_t {
   Decodable,
   AdditiveArithmetic,
   Differentiable,
-  Identifiable,
   Actor,
   DistributedActor,
   DistributedActorSystem,

include/swift/AST/DiagnosticsSema.def

@@ -5710,6 +5710,8 @@ ERROR(actor_cannot_inherit_distributed_actor_protocol,none,
       (DeclName))
 ERROR(broken_distributed_actor_requirement,none,
       "DistributedActor protocol is broken: unexpected requirement", ())
+ERROR(broken_distributed_actor_system_requirement,none,
+      "DistributedActorSystem protocol is broken: unexpected requirement", ())
 ERROR(distributed_actor_system_conformance_missing_adhoc_requirement,none,
       "%kind0 is missing witness for protocol requirement %1",
       (const ValueDecl *, DeclName))

include/swift/AST/TypeCheckRequests.h

@@ -2978,9 +2978,6 @@ enum class ImplicitMemberAction : uint8_t {
   ResolveCodingKeys,
   ResolveEncodable,
   ResolveDecodable,
-  ResolveDistributedActor,
-  ResolveDistributedActorID,
-  ResolveDistributedActorSystem,
 };
 
 class ResolveImplicitMemberRequest

lib/AST/Decl.cpp

@@ -6449,6 +6449,16 @@ void NominalTypeDecl::synthesizeSemanticMembersIfNeeded(DeclName member) {
   if (isa<ProtocolDecl>(this))
     return;
 
+  auto baseName = member.getBaseName();
+  auto &Context = getASTContext();
+
+  // For a distributed actor `id` and `actorSystem` can be synthesized without
+  // causing cycles so do them above the cycle guard.
+  if (member.isSimpleName(Context.Id_id))
+    (void)getDistributedActorIDProperty();
+  if (member.isSimpleName(Context.Id_actorSystem))
+    (void)getDistributedActorSystemProperty();
+
   // Silently break cycles here because we can't be sure when and where a
   // request to synthesize will come from yet.
   // FIXME: rdar://56844567
@@ -6458,14 +6468,12 @@ void NominalTypeDecl::synthesizeSemanticMembersIfNeeded(DeclName member) {
   Bits.NominalTypeDecl.IsComputingSemanticMembers = true;
   SWIFT_DEFER { Bits.NominalTypeDecl.IsComputingSemanticMembers = false; };
 
-  auto baseName = member.getBaseName();
-  auto &Context = getASTContext();
   std::optional<ImplicitMemberAction> action = std::nullopt;
   if (baseName.isConstructor())
     action.emplace(ImplicitMemberAction::ResolveImplicitInit);
 
   if (member.isSimpleName() && !baseName.isSpecial()) {
-    if (baseName.getIdentifier() == getASTContext().Id_CodingKeys) {
+    if (baseName.getIdentifier() == Context.Id_CodingKeys) {
       action.emplace(ImplicitMemberAction::ResolveCodingKeys);
     }
   } else {
@@ -6474,22 +6482,13 @@ void NominalTypeDecl::synthesizeSemanticMembersIfNeeded(DeclName member) {
       if (baseName.isConstructor()) {
         if ((member.isSimpleName() || argumentNames.front() == Context.Id_from)) {
           action.emplace(ImplicitMemberAction::ResolveDecodable);
-        } else if (argumentNames.front() == Context.Id_system) {
-          action.emplace(ImplicitMemberAction::ResolveDistributedActorSystem);
         }
       } else if (!baseName.isSpecial() &&
                  baseName.getIdentifier() == Context.Id_encode &&
                  (member.isSimpleName() ||
                   argumentNames.front() == Context.Id_to)) {
         action.emplace(ImplicitMemberAction::ResolveEncodable);
       }
-    } else if (member.isSimpleName() || argumentNames.size() == 2) {
-      if (baseName.isConstructor()) {
-        if (argumentNames[0] == Context.Id_resolve &&
-            argumentNames[1] == Context.Id_using) {
-          action.emplace(ImplicitMemberAction::ResolveDistributedActor);
-        }
-      }
     }
   }
 
@@ -7554,8 +7553,6 @@ ProtocolDecl::getKnownDerivableProtocolKind() const {
     return KnownDerivableProtocolKind::AdditiveArithmetic;
   case KnownProtocolKind::Differentiable:
     return KnownDerivableProtocolKind::Differentiable;
-  case KnownProtocolKind::Identifiable:
-    return KnownDerivableProtocolKind::Identifiable;
   case KnownProtocolKind::Actor:
     return KnownDerivableProtocolKind::Actor;
   case KnownProtocolKind::DistributedActor:

lib/AST/DistributedDecl.cpp

@@ -15,12 +15,13 @@
 //===----------------------------------------------------------------------===//
 
 #include "swift/AST/DistributedDecl.h"
-#include "swift/AST/AccessRequests.h"
-#include "swift/AST/AccessScope.h"
 #include "swift/AST/ASTContext.h"
-#include "swift/AST/ASTWalker.h"
 #include "swift/AST/ASTMangler.h"
+#include "swift/AST/ASTWalker.h"
+#include "swift/AST/AccessRequests.h"
+#include "swift/AST/AccessScope.h"
 #include "swift/AST/ConformanceLookup.h"
+#include "swift/AST/DiagnosticsSema.h"
 #include "swift/AST/ExistentialLayout.h"
 #include "swift/AST/Expr.h"
 #include "swift/AST/ForeignAsyncConvention.h"
@@ -29,7 +30,6 @@
 #include "swift/AST/GenericSignature.h"
 #include "swift/AST/Initializer.h"
 #include "swift/AST/LazyResolver.h"
-#include "swift/AST/ASTMangler.h"
 #include "swift/AST/Module.h"
 #include "swift/AST/NameLookup.h"
 #include "swift/AST/NameLookupRequests.h"
@@ -39,9 +39,10 @@
 #include "swift/AST/ResilienceExpansion.h"
 #include "swift/AST/SourceFile.h"
 #include "swift/AST/Stmt.h"
-#include "swift/AST/TypeCheckRequests.h"
 #include "swift/AST/SwiftNameTranslation.h"
+#include "swift/AST/TypeCheckRequests.h"
 #include "swift/Basic/Assertions.h"
+#include "swift/Basic/StringExtras.h"
 #include "swift/ClangImporter/ClangModule.h"
 #include "swift/Parse/Lexer.h" // FIXME: Bad dependency
 #include "clang/Lex/MacroInfo.h"
@@ -51,7 +52,6 @@
 #include "llvm/ADT/Statistic.h"
 #include "llvm/Support/Compiler.h"
 #include "llvm/Support/raw_ostream.h"
-#include "swift/Basic/StringExtras.h"
 
 #include "clang/Basic/CharInfo.h"
 #include "clang/Basic/Module.h"
@@ -198,7 +198,7 @@ Type swift::getDistributedActorSystemType(NominalTypeDecl *actor) {
 
   auto DA = C.getDistributedActorDecl();
   if (!DA)
-    return ErrorType::get(C); // FIXME(distributed): just use Type()
+    return ErrorType::get(C);
 
   // Dig out the actor system type.
   Type selfType = actor->getSelfInterfaceType();
@@ -229,17 +229,16 @@ Type swift::getDistributedActorSerializationType(
   auto resultTy = getAssociatedTypeOfDistributedSystemOfActor(
       actorOrExtension,
       ctx.Id_SerializationRequirement);
+  if (resultTy->hasError())
+    return resultTy;
 
   // Protocols are allowed to either not provide a `SerializationRequirement`
   // at all or provide it in a conformance requirement.
-  if ((!resultTy || resultTy->hasDependentMember()) &&
+  if (resultTy->hasDependentMember() &&
       actorOrExtension->getSelfProtocolDecl()) {
     auto sig = actorOrExtension->getGenericSignatureOfContext();
 
     auto actorProtocol = ctx.getProtocol(KnownProtocolKind::DistributedActor);
-    if (!actorProtocol)
-      return Type();
-
     auto serializationTy =
         actorProtocol->getAssociatedType(ctx.Id_SerializationRequirement)
             ->getDeclaredInterfaceType();
@@ -331,29 +330,40 @@ swift::getAssociatedDistributedInvocationDecoderDecodeNextArgumentFunction(
 Type swift::getAssociatedTypeOfDistributedSystemOfActor(
     DeclContext *actorOrExtension, Identifier member) {
   auto &ctx = actorOrExtension->getASTContext();
+  auto getLoc = [&]() { return extractNearestSourceLoc(actorOrExtension); };
 
   auto actorProtocol = ctx.getProtocol(KnownProtocolKind::DistributedActor);
-  if (!actorProtocol)
-    return Type();
+  if (!actorProtocol) {
+    ctx.Diags.diagnose(getLoc(), diag::broken_stdlib_type, "DistributedActor");
+    return ErrorType::get(ctx);
+  }
 
   AssociatedTypeDecl *actorSystemDecl =
       actorProtocol->getAssociatedType(ctx.Id_ActorSystem);
-  if (!actorSystemDecl)
-    return Type();
+  if (!actorSystemDecl) {
+    ctx.Diags.diagnose(getLoc(), diag::broken_distributed_actor_requirement);
+    return ErrorType::get(ctx);
+  }
 
   auto actorSystemProtocol = ctx.getDistributedActorSystemDecl();
-  if (!actorSystemProtocol)
-    return Type();
+  if (!actorSystemProtocol) {
+    ctx.Diags.diagnose(getLoc(), diag::broken_stdlib_type,
+                       "DistributedActorSystem");
+    return ErrorType::get(ctx);
+  }
 
   AssociatedTypeDecl *memberTypeDecl =
       actorSystemProtocol->getAssociatedType(member);
-  if (!memberTypeDecl)
-    return Type();
+  if (!memberTypeDecl) {
+    ctx.Diags.diagnose(getLoc(),
+                       diag::broken_distributed_actor_system_requirement);
+    return ErrorType::get(ctx);
+  }
 
   Type memberTy = DependentMemberType::get(
-    DependentMemberType::get(actorProtocol->getSelfInterfaceType(),
-                             actorSystemDecl),
-    memberTypeDecl);
+      DependentMemberType::get(actorProtocol->getSelfInterfaceType(),
+                               actorSystemDecl),
+      memberTypeDecl);
 
   auto sig = actorOrExtension->getGenericSignatureOfContext();
 
@@ -365,7 +375,7 @@ Type swift::getAssociatedTypeOfDistributedSystemOfActor(
       lookupConformance(
           actorType->getDeclaredInterfaceType(), actorProtocol);
   if (actorConformance.isInvalid())
-    return Type();
+    return ErrorType::get(ctx);
 
   auto subs = SubstitutionMap::getProtocolSubstitutions(actorConformance);
 
@@ -432,11 +442,7 @@ swift::getDistributedSerializationRequirements(
 }
 
 bool swift::checkDistributedSerializationRequirementIsExactlyCodable(
-    ASTContext &C,
-    Type type) {
-  if (!type)
-    return false;
-
+    ASTContext &C, Type type) {
   if (type->hasError())
     return false;
 
@@ -1352,6 +1358,42 @@ bool ValueDecl::isDistributedGetAccessor() const {
   return false;
 }
 
+std::optional<SpecialDistributedProperty>
+ValueDecl::isSpecialDistributedProperty(bool onlyCheckName) const {
+  if (!isa<VarDecl>(this))
+    return std::nullopt;
+
+  auto *DC = getDeclContext();
+  auto &ctx = DC->getASTContext();
+
+  auto kind = [&]() -> std::optional<SpecialDistributedProperty> {
+    auto name = getName();
+    if (name.isSimpleName(ctx.Id_id))
+      return SpecialDistributedProperty::Id;
+    if (name.isSimpleName(ctx.Id_actorSystem))
+      return SpecialDistributedProperty::ActorSystem;
+
+    return std::nullopt;
+  }();
+  if (!kind || onlyCheckName)
+    return kind;
+
+  // The properties can only be synthesized in the nominal itself.
+  auto *CD = dyn_cast<ClassDecl>(DC);
+  if (!CD || !CD->isDistributedActor())
+    return std::nullopt;
+
+  // The synthesized bit doesn't get preserved by serialization or module
+  // interfaces, we only need to check it when compiling a SourceFile though
+  // since we'll diagnose any conflicting user-defined versions.
+  if (!isSynthesized() && DC->getParentSourceFile() &&
+      !DC->isInSwiftinterface()) {
+    return std::nullopt;
+  }
+
+  return kind;
+}
+
 ConstructorDecl *
 NominalTypeDecl::getDistributedRemoteCallTargetInitFunction() const {
   auto mutableThis = const_cast<NominalTypeDecl *>(this);

lib/AST/TypeCheckRequests.cpp

@@ -1310,15 +1310,6 @@ void swift::simple_display(llvm::raw_ostream &out,
   case ImplicitMemberAction::ResolveDecodable:
     out << "resolve Decodable.init(from:)";
     break;
-  case ImplicitMemberAction::ResolveDistributedActor:
-    out << "resolve DistributedActor";
-    break;
-  case ImplicitMemberAction::ResolveDistributedActorID:
-    out << "resolve DistributedActor.id";
-    break;
-  case ImplicitMemberAction::ResolveDistributedActorSystem:
-    out << "resolve DistributedActor.actorSystem";
-    break;
   }
 }
 

lib/SILGen/SILGenDestructor.cpp

@@ -74,10 +74,8 @@ void SILGenFunction::emitDistributedRemoteActorDeinit(
           continue;
 
         // Just to double-check, we only want to destroy `id` and `actorSystem`
-        if (vd->getBaseIdentifier() == C.Id_id ||
-            vd->getBaseIdentifier() == C.Id_actorSystem) {
+        if (vd->isSpecialDistributedProperty())
           destroyClassMember(cleanupLoc, borrowedSelf, vd);
-        }
       }
 
       if (cd->isRootDefaultActor()) {