Merge pull request #42299 from ktoso/pick-cache-checking-dist-func

🍒[5.7][Distributed] Cache checking distributed func

Author: ktoso

include/swift/AST/TypeCheckRequests.h

@@ -1083,6 +1083,33 @@ class GetDistributedActorImplicitCodableRequest :
   bool isCached() const { return true; }
 };
 
+/// Check a distributed function declaration and cache if it was valid or not.
+///
+/// This is used because we not only type-check to emit errors, but also use
+/// the information to potentially avoid emitting the distributed thunk for
+/// methods which are invalid (e.g. their parameters dont conform to
+/// SerializationRequirement), as otherwise we'd be causing errors in synthesized
+/// code which looks very confusing to end-users.
+///
+class CheckDistributedFunctionRequest :
+    public SimpleRequest<CheckDistributedFunctionRequest,
+                         bool(AbstractFunctionDecl *),
+                         RequestFlags::Cached> {
+public:
+  using SimpleRequest::SimpleRequest;
+
+private:
+  friend SimpleRequest;
+
+  /// \returns \c true if there was a problem with the function declaration,
+  /// \c false otherwise.
+  bool evaluate(Evaluator &evaluator, AbstractFunctionDecl *) const;
+
+public:
+  // Caching
+  bool isCached() const { return true; }
+};
+
 /// Obtain the 'remoteCall' function of a 'DistributedActorSystem'.
 class GetDistributedActorSystemRemoteCallFunctionRequest :
     public SimpleRequest<GetDistributedActorSystemRemoteCallFunctionRequest,

include/swift/AST/TypeCheckerTypeIDZone.def

@@ -110,6 +110,9 @@ SWIFT_REQUEST(TypeChecker, IsActorRequest, bool(NominalTypeDecl *),
 SWIFT_REQUEST(TypeChecker, IsDefaultActorRequest,
               bool(ClassDecl *, ModuleDecl *, ResilienceExpansion),
               Cached, NoLocationInfo)
+SWIFT_REQUEST(TypeChecker, CheckDistributedFunctionRequest,
+              bool(AbstractFunctionDecl *),
+              Cached, NoLocationInfo)
 SWIFT_REQUEST(TypeChecker, IsDistributedActorRequest, bool(NominalTypeDecl *),
               Cached, NoLocationInfo)
 SWIFT_REQUEST(TypeChecker, GetDistributedActorImplicitCodableRequest,

lib/Sema/CodeSynthesisDistributedActor.cpp

@@ -696,10 +696,12 @@ FuncDecl *GetDistributedThunkRequest::evaluate(
     return nullptr;
   }
 
-  // Force type-checking the original function, so we can avoid synthesizing
-  // the thunks (which would have many of the same errors, if they are caused
-  // by a bad source function signature, e.g. missing conformances etc).
-  if (distributedTarget->getInterfaceType()->hasError()) {
+  // If the target function signature has errors, or if it is illegal in other
+  // ways, such as e.g. parameters not conforming to SerializationRequirement,
+  // we must avoid synthesis of the thunk because it'd also have errors,
+  // giving an ugly user experience (errors in implicit code).
+  if (distributedTarget->getInterfaceType()->hasError() ||
+      checkDistributedFunction(distributedTarget)) {
     return nullptr;
   }
 

lib/Sema/TypeCheckConcurrency.cpp

@@ -3256,7 +3256,7 @@ void swift::checkFunctionActorIsolation(AbstractFunctionDecl *decl) {
   }
   if (decl->getAttrs().hasAttribute<DistributedActorAttr>()) {
     if (auto func = dyn_cast<FuncDecl>(decl)) {
-      checkDistributedFunction(func, /*diagnose=*/true);
+      checkDistributedFunction(func);
     }
   }
 }

lib/Sema/TypeCheckDistributed.cpp

@@ -16,7 +16,6 @@
 #include "TypeCheckConcurrency.h"
 #include "TypeCheckDistributed.h"
 #include "TypeChecker.h"
-#include "TypeCheckType.h"
 #include "swift/Strings.h"
 #include "swift/AST/ASTWalker.h"
 #include "swift/AST/Initializer.h"
@@ -434,11 +433,17 @@ static bool checkDistributedTargetResultType(
 
 /// Check whether the function is a proper distributed function
 ///
-/// \param diagnose Whether to emit a diagnostic when a problem is encountered.
-///
 /// \returns \c true if there was a problem with adding the attribute, \c false
 /// otherwise.
-bool swift::checkDistributedFunction(FuncDecl *func, bool diagnose) {
+bool swift::checkDistributedFunction(AbstractFunctionDecl *func) {
+  auto &C = func->getASTContext();
+  return evaluateOrDefault(C.evaluator,
+                           CheckDistributedFunctionRequest{func},
+                           false); // no error if cycle
+}
+
+bool CheckDistributedFunctionRequest::evaluate(
+    Evaluator &evaluator, AbstractFunctionDecl *func) const {
   assert(func->isDistributed());
 
   auto &C = func->getASTContext();
@@ -456,9 +461,8 @@ bool swift::checkDistributedFunction(FuncDecl *func, bool diagnose) {
     serializationRequirements = extractDistributedSerializationRequirements(
         C, extension->getGenericRequirements());
   } else if (auto actor = dyn_cast<ClassDecl>(DC)) {
-    auto systemProp = actor->getDistributedActorSystemProperty();
     serializationRequirements = getDistributedSerializationRequirementProtocols(
-        systemProp->getInterfaceType()->getAnyNominal(),
+        getDistributedActorSystemType(actor)->getAnyNominal(),
         C.getProtocol(KnownProtocolKind::DistributedActorSystem));
   } else {
     llvm_unreachable("Cannot handle types other than extensions and actor "
@@ -476,18 +480,16 @@ bool swift::checkDistributedFunction(FuncDecl *func, bool diagnose) {
 
     for (auto req : serializationRequirements) {
       if (TypeChecker::conformsToProtocol(paramTy, req, module).isInvalid()) {
-        if (diagnose) {
-          auto diag = func->diagnose(
-              diag::distributed_actor_func_param_not_codable,
-              param->getArgumentName().str(), param->getInterfaceType(),
-              func->getDescriptiveKind(),
-              serializationRequirementIsCodable ? "Codable"
-                                                : req->getNameStr());
-
-          if (auto paramNominalTy = paramTy->getAnyNominal()) {
-            addCodableFixIt(paramNominalTy, diag);
-          } // else, no nominal type to suggest the fixit for, e.g. a closure
-        }
+        auto diag = func->diagnose(
+            diag::distributed_actor_func_param_not_codable,
+            param->getArgumentName().str(), param->getInterfaceType(),
+            func->getDescriptiveKind(),
+            serializationRequirementIsCodable ? "Codable"
+                                              : req->getNameStr());
+
+        if (auto paramNominalTy = paramTy->getAnyNominal()) {
+          addCodableFixIt(paramNominalTy, diag);
+        } // else, no nominal type to suggest the fixit for, e.g. a closure
         return true;
       }
     }
@@ -513,7 +515,8 @@ bool swift::checkDistributedFunction(FuncDecl *func, bool diagnose) {
   }
 
   // --- Result type must be either void or a codable type
-  if (checkDistributedTargetResultType(module, func, serializationRequirements, diagnose)) {
+  if (checkDistributedTargetResultType(module, func, serializationRequirements,
+                                       /*diagnose=*/true)) {
     return true;
   }
 

lib/Sema/TypeCheckDistributed.h

@@ -51,7 +51,7 @@ bool checkDistributedActorSystemAdHocProtocolRequirements(
     bool diagnose);
 
 /// Typecheck a distributed method declaration
-bool checkDistributedFunction(FuncDecl *decl, bool diagnose);
+bool checkDistributedFunction(AbstractFunctionDecl *decl);
 
 /// Typecheck a distributed computed (get-only) property declaration.
 /// They are effectively checked the same way as argument-less methods.