Put worker protocol items inside a WorkerProto struct

See API docs on that struct for why. The pasing as as template argument
doesn't yet happen in that commit, but will instead happen in later
commit.

Also make `WorkerOp` (now `Op`) and enum struct. This led us to catch
that two operations were not handled!

Co-authored-by: Robert Hensing <roberth@users.noreply.github.com>

src/libstore/worker-protocol.hh

@@ -14,57 +14,6 @@ namespace nix {
 #define GET_PROTOCOL_MINOR(x) ((x) & 0x00ff)
 
 
-/**
- * Enumeration of all the request types for the "worker protocol", used
- * by unix:// and ssh-ng:// stores.
- */
-typedef enum {
-    wopIsValidPath = 1,
-    wopHasSubstitutes = 3,
-    wopQueryPathHash = 4, // obsolete
-    wopQueryReferences = 5, // obsolete
-    wopQueryReferrers = 6,
-    wopAddToStore = 7,
-    wopAddTextToStore = 8, // obsolete since 1.25, Nix 3.0. Use wopAddToStore
-    wopBuildPaths = 9,
-    wopEnsurePath = 10,
-    wopAddTempRoot = 11,
-    wopAddIndirectRoot = 12,
-    wopSyncWithGC = 13,
-    wopFindRoots = 14,
-    wopExportPath = 16, // obsolete
-    wopQueryDeriver = 18, // obsolete
-    wopSetOptions = 19,
-    wopCollectGarbage = 20,
-    wopQuerySubstitutablePathInfo = 21,
-    wopQueryDerivationOutputs = 22, // obsolete
-    wopQueryAllValidPaths = 23,
-    wopQueryFailedPaths = 24,
-    wopClearFailedPaths = 25,
-    wopQueryPathInfo = 26,
-    wopImportPaths = 27, // obsolete
-    wopQueryDerivationOutputNames = 28, // obsolete
-    wopQueryPathFromHashPart = 29,
-    wopQuerySubstitutablePathInfos = 30,
-    wopQueryValidPaths = 31,
-    wopQuerySubstitutablePaths = 32,
-    wopQueryValidDerivers = 33,
-    wopOptimiseStore = 34,
-    wopVerifyStore = 35,
-    wopBuildDerivation = 36,
-    wopAddSignatures = 37,
-    wopNarFromPath = 38,
-    wopAddToStoreNar = 39,
-    wopQueryMissing = 40,
-    wopQueryDerivationOutputMap = 41,
-    wopRegisterDrvOutput = 42,
-    wopQueryRealisation = 43,
-    wopAddMultipleToStore = 44,
-    wopAddBuildLog = 45,
-    wopBuildPathsWithResults = 46,
-} WorkerOp;
-
-
 #define STDERR_NEXT  0x6f6c6d67
 #define STDERR_READ  0x64617461 // data needed from source
 #define STDERR_WRITE 0x64617416 // data for sink
@@ -78,7 +27,7 @@ typedef enum {
 class Store;
 struct Source;
 
-// items being serialized
+// items being serialised
 struct DerivedPath;
 struct DrvOutput;
 struct Realisation;
@@ -88,31 +37,132 @@ enum TrustedFlag : bool;
 
 
 /**
- * Data type for canonical pairs of serializers for the worker protocol.
+ * The "worker protocol", used by unix:// and ssh-ng:// stores.
  *
- * See https://en.cppreference.com/w/cpp/language/adl for the broader
- * concept of what is going on here.
+ * This `struct` is basically just a `namespace`; We use a type rather
+ * than a namespace just so we can use it as a template argument.
  */
-template<typename T>
-struct WorkerProto {
-    static T read(const Store & store, Source & from);
-    static void write(const Store & store, Sink & out, const T & t);
+struct WorkerProto
+{
+    /**
+     * Enumeration of all the request types for the protocol.
+     */
+    enum struct Op : uint64_t;
+
+    /**
+     * Data type for canonical pairs of serialisers for the worker protocol.
+     *
+     * See https://en.cppreference.com/w/cpp/language/adl for the broader
+     * concept of what is going on here.
+     */
+    template<typename T>
+    struct Serialise;
+    // This is the definition of `Serialise` we *want* to put here, but
+    // do not do so.
+    //
+    // The problem is that if we do so, C++ will think we have
+    // seralisers for *all* types. We don't, of course, but that won't
+    // cause an error until link time. That makes for long debug cycles
+    // when there is a missing serialiser.
+    //
+    // By not defining it globally, and instead letting individual
+    // serialisers specialise the type, we get back the compile-time
+    // errors we would like. When no serialiser exists, C++ sees an
+    // abstract "incomplete" type with no definition, and any attempt to
+    // use `to` or `from` static methods is a compile-time error because
+    // they don't exist on an incomplete type.
+    //
+    // This makes for a quicker debug cycle, as desired.
+#if 0
+    {
+        static T read(const Store & store, Source & from);
+        static void write(const Store & store, Sink & out, const T & t);
+    };
+#endif
+
+    /**
+     * Wrapper function around `WorkerProto::Serialise<T>::write` that allows us to
+     * infer the type instead of having to write it down explicitly.
+     */
+    template<typename T>
+    static void write(const Store & store, Sink & out, const T & t)
+    {
+        WorkerProto::Serialise<T>::write(store, out, t);
+    }
+};
+
+enum struct WorkerProto::Op : uint64_t
+{
+    IsValidPath = 1,
+    HasSubstitutes = 3,
+    QueryPathHash = 4, // obsolete
+    QueryReferences = 5, // obsolete
+    QueryReferrers = 6,
+    AddToStore = 7,
+    AddTextToStore = 8, // obsolete since 1.25, Nix 3.0. Use WorkerProto::Op::AddToStore
+    BuildPaths = 9,
+    EnsurePath = 10,
+    AddTempRoot = 11,
+    AddIndirectRoot = 12,
+    SyncWithGC = 13,
+    FindRoots = 14,
+    ExportPath = 16, // obsolete
+    QueryDeriver = 18, // obsolete
+    SetOptions = 19,
+    CollectGarbage = 20,
+    QuerySubstitutablePathInfo = 21,
+    QueryDerivationOutputs = 22, // obsolete
+    QueryAllValidPaths = 23,
+    QueryFailedPaths = 24,
+    ClearFailedPaths = 25,
+    QueryPathInfo = 26,
+    ImportPaths = 27, // obsolete
+    QueryDerivationOutputNames = 28, // obsolete
+    QueryPathFromHashPart = 29,
+    QuerySubstitutablePathInfos = 30,
+    QueryValidPaths = 31,
+    QuerySubstitutablePaths = 32,
+    QueryValidDerivers = 33,
+    OptimiseStore = 34,
+    VerifyStore = 35,
+    BuildDerivation = 36,
+    AddSignatures = 37,
+    NarFromPath = 38,
+    AddToStoreNar = 39,
+    QueryMissing = 40,
+    QueryDerivationOutputMap = 41,
+    RegisterDrvOutput = 42,
+    QueryRealisation = 43,
+    AddMultipleToStore = 44,
+    AddBuildLog = 45,
+    BuildPathsWithResults = 46,
 };
 
 /**
- * Wrapper function around `WorkerProto<T>::write` that allows us to
- * infer the type instead of having to write it down explicitly.
+ * Convenience for sending operation codes.
+ *
+ * @todo Switch to using `WorkerProto::Serialise` instead probably. But
+ * this was not done at this time so there would be less churn.
  */
-template<typename T>
-void workerProtoWrite(const Store & store, Sink & out, const T & t)
+inline Sink & operator << (Sink & sink, WorkerProto::Op op)
 {
-    WorkerProto<T>::write(store, out, t);
+    return sink << (uint64_t) op;
 }
 
 /**
- * Declare a canonical serializer pair for the worker protocol.
+ * Convenience for debugging.
  *
- * We specialize the struct merely to indicate that we are implementing
+ * @todo Perhaps render known opcodes more nicely.
+ */
+inline std::ostream & operator << (std::ostream & s, WorkerProto::Op op)
+{
+    return s << (uint64_t) op;
+}
+
+/**
+ * Declare a canonical serialiser pair for the worker protocol.
+ *
+ * We specialise the struct merely to indicate that we are implementing
  * the function for the given type.
  *
  * Some sort of `template<...>` must be used with the caller for this to
@@ -120,7 +170,7 @@ void workerProtoWrite(const Store & store, Sink & out, const T & t)
  * practice.
  */
 #define MAKE_WORKER_PROTO(T) \
-    struct WorkerProto< T > { \
+    struct WorkerProto::Serialise< T > { \
         static T read(const Store & store, Source & from); \
         static void write(const Store & store, Sink & out, const T & t); \
     };
@@ -156,7 +206,7 @@ MAKE_WORKER_PROTO(X_);
 
 /**
  * These use the empty string for the null case, relying on the fact
- * that the underlying types never serialize to the empty string.
+ * that the underlying types never serialise to the empty string.
  *
  * We do this instead of a generic std::optional<T> instance because
  * ordinal tags (0 or 1, here) are a bit of a compatability hazard. For