[Distributed] improve ad-hoc requirement typechecking #41179

ktoso · 2022-02-03T10:22:39Z

Improves the matching of ad-hoc protocol requirements remoteCall / remoteCallVoid.

Dealing with the generics there is a pain in the neck but we're almost there...
This isn't a complete impl still, it has to check the param of the Act and the SerializationRequirements still

ktoso · 2022-02-03T10:26:22Z

@swift-ci please smoke test and merge

xedin · 2022-02-04T05:41:35Z

lib/AST/Decl.cpp

+
+  size_t expectedRequirementsNum = 3;
+  if (!isVoidReturn) {
+    // TODO(distributed): support alternative SerializationRequirements here


Here is an alternative way to check this:

Lookup declaring type - should be an implementation of DistributedActorSystem

Find a concrete serialization requirement associated with it e.g. Codable

Find a concrete InvocationDecoder type

Extract either a single protocol type or a set of protocols (if requirement is a ProtocolCompositionType)

Structural checks:

async + throws

Has 3 generic parameters

Has required number of parameters,

labels match (!)

result type is represented by a generic parameter or Void

Check that parameter at index 0 is a generic parameter and it conforms to DistributedActor

Check that parameter at index 1 is a known type - RemoteCallTarget (you'd have to make that type known to the compiler)

Check that parameter at index 2 is equal to concrete type of InvocationDecoder and parameter is inout

Check that parameter at index 3 is a metatype of a generic parameter or Void and if it's former that it "covers" all of the protocols from step 3 (you can get a set of requirements associated with a given generic parameter via GenericSignature::RequiredProtocols getRequiredProtocols(Type type) const;).

Check that parameter at index 4 is a metatype of generic parameter and generic parameter has a requirement for Error (same getRequiredProtocols call).

You could either check the label list of equivalence or do per-parameter label checks.

Thanks -- getting back to this now :)

Thanks for the hints this makes it much cleaner; Only halfway through so far since i implemented dealing with the serialization req a bit more. Shaping up well tho

Done and more cleanups along the way.

ktoso · 2022-02-10T14:19:05Z

Made good progress here -- lots of cleanup and moving around utilities to work with the associated types (serialization req) and desugaring it etc... I think I should have this complete tomorrow.

DistributedTargetInvocationEncoder

ktoso · 2022-02-11T10:29:15Z

@swift-ci please smoke test

ktoso · 2022-02-11T10:30:01Z

Did a good next pass here, all checks on remote calls are done I think.

Will continue with the other ones which will be pretty simple.

There is still a bit code duplication around dealing with types... I'll be cleaning up more and more as follow up as I implement the rest -- maybe tomorrow

lib/AST/DistributedDecl.cpp

xedin

Looks good to me so far! Please don't forgot to remove all of the fprintf debug logging. Also FYI you can use llvm::errs() to make it simpler for yourself since it works the same way as std::cout and doesn't require format strings :)

ktoso · 2022-02-14T13:07:08Z

Thanks -- done a lot here and going to try to get this step in; there's one more protocol to handle the result handler with its onReturn but I'll do this in a small PR

ktoso · 2022-02-14T13:07:25Z

@swift-ci please smoke test

ktoso · 2022-02-14T13:09:10Z

lib/AST/ASTContext.cpp

-      return fd;
-  }
-
-  return nullptr;


moved all these to requests and implemented properly

ktoso · 2022-02-14T13:09:23Z

lib/AST/CMakeLists.txt

@@ -43,6 +43,7 @@ add_swift_host_library(swiftAST STATIC
  DiagnosticConsumer.cpp
  DiagnosticEngine.cpp
  DiagnosticList.cpp
+  DistributedDecl.cpp


new common place for all our "get this, get that"

ktoso · 2022-02-14T13:09:36Z

@swift-ci please smoke test

ktoso · 2022-02-14T13:10:30Z

test/Distributed/Runtime/distributed_actor_self_calls.swift

-//          Act.ID == ActorID
-  {
+          Act.ID == ActorID,
+          Err: Error {


uncommented and requiring those properly now

ktoso · 2022-02-15T13:57:36Z

Had some issues with the mutable checking but will revisit this later

ktoso · 2022-02-15T13:57:42Z

@swift-ci please smoke test and merge

fixup

ktoso · 2022-02-15T23:45:16Z

@swift-ci please smoke test and merge

ktoso · 2022-02-16T03:18:10Z

@swift-ci please smoke test and merge

ktoso · 2022-02-16T04:40:38Z

Flaky lldb tests:


  lldb-api :: lang/swift/implementation_only_imports/library_resilient/TestLibraryResilient.py
  lldb-shell :: SwiftREPL/BreakpointSimple.test

ktoso · 2022-02-16T04:40:45Z