Generate Save functions from LCP as top level

Summary:
This should allow us to more easily decouple the code which should be
open sourced. Unfortunately, the downside of this approach is that we
cannot rely on virtual calls to dispatch the serialization to correct
type. Another downside is that members need to be publicly accessible
for serialization.

Reviewers: mtomic, msantl

Reviewed By: mtomic

Subscribers: pullbot

Differential Revision: https://phabricator.memgraph.io/D1596

docs/dev/lcp.md

@@ -387,7 +387,7 @@ rpc_server.Register<QueryResultRpc>(
       request.Load(req_reader);
       // process the request and send the response
       QueryResultRes response(values_for_response);
-      response.Save(res_builder);
+      Save(response, res_builder);
     });
 
 
@@ -428,14 +428,19 @@ For example:
 
 `:serialize` option will generate a Cap'n Proto schema of the class and store
 it in the `.capnp` file. C++ code will be generated for saving and loading
-members and the class will get 2 public methods:
+members:
 
 ```cpp
-void Save(capnp::MyClass::Builder *builder) const;
+// Top level function
+void Save(const MyClass &instance, capnp::MyClass::Builder *builder);
 
-void Load(const capnp::MyClass::Reader &reader);
+// Member function
+void MyClass::Load(const capnp::MyClass::Reader &reader);
 ```
 
+Since we use top level functions, the class needs to have some sort of public
+access to its members.
+
 The schema file will be namespaced in `capnp`. To change add a prefix
 namespace use `lcp:capnp-namespace` function. For example, if we use
 `(lcp:capnp-namespace "my_namespace")` then the reader and builder would be in
@@ -463,11 +468,13 @@ base class. And a `Construct` function is added which will instantiate a
 concrete type from a base reader.
 
 ```cpp
-virtual void Save(capnp::Base *builder) const;
+void Save(const Derived &derived, capnp::Base *builder);
 
-static std::unique_ptr<Base> Construct(const capnp::Base &reader);
+class Derived {
+  ...
+  static std::unique_ptr<Base> Construct(const capnp::Base &reader);
 
-virtual void Load(const capnp::Base &reader);
+  virtual void Load(const capnp::Base &reader);
 ```
 
 With polymorphic types, you need to call `Base::Construct` followed by `Load`.
@@ -596,9 +603,17 @@ you to delay the initialization to your custom save code. You rarely want to
 set `:capnp-init nil`.
 
 Custom save code is added as a value of `:capnp-save`. It should be a function
-which takes 2 arguments: builder and member. Both are character strings which
-represent the variable names that will be in generated for C++ code. The
-result of the function needs to be a C++ code block.
+which takes 3 arguments.
+
+  1. Name of builder variable.
+  2. Name of the class (or struct) member.
+  3. Name of the member in Cap'n Proto schema.
+
+The result of the function needs to be a C++ code block.
+
+You will rarely need to use the 3rd argument, so it should be ignored in most
+cases. It is usually needed when you set `:capnp-init nil`, so that you can
+correctly initialize the builder.
 
 Similarly, `:capnp-load` expects a function taking a reader and a member, then
 returns a C++ block.
@@ -609,10 +624,10 @@ Example:
 (lcp:define-class my-class ()
   ((my-member "ComplexType"
               :capnp-init nil
-              :capnp-save (lambda (builder member)
+              :capnp-save (lambda (builder member capnp-name)
                             #>cpp
                             auto data = ${member}.GetSaveData();
-                            auto my_builder = ${builder}.initMyMember();
+                            auto my_builder = ${builder}.init${capnp-name}();
                             my_builder.setData(data);
                             cpp<#)
               :capnp-load (lambda (reader member)
@@ -667,7 +682,7 @@ The custom serialization code will now have access to `save_helper` and
 list of pairs, e.g.
 
 ```lisp
-:capnp-save '((first-helper "SomeType *") (second-helper "OtherType *") ...)
+:save-args '((first-helper "SomeType *") (second-helper "OtherType *") ...)
 ```
 
 #### Custom Serialization Helper Functions

src/communication/rpc/client.hpp

@@ -60,7 +60,7 @@ class Client {
       auto req_builder =
           data_builder
               .template initAs<typename TRequestResponse::Request::Capnp>();
-      request.Save(&req_builder);
+      Save(request, &req_builder);
     }
     auto response = Send(&req_msg);
     auto res_msg = response.getRoot<capnp::Message>();

src/communication/rpc/messages.hpp

@@ -43,7 +43,7 @@ inline bool operator>=(const MessageType &a, const MessageType &b) {
 /// and `TResponse` are required to define a nested `Capnp` type, which
 /// corresponds to the Cap'n Proto schema type, as well as defined the following
 /// serialization functions:
-///   * void Save(Capnp::Builder *, ...) const
+///   * void Save(const TRequest|TResponse &, Capnp::Builder *, ...)
 ///   * void Load(const Capnp::Reader &, ...)
 template <typename TRequest, typename TResponse>
 struct RequestResponse {

src/database/distributed_counters.cpp

@@ -11,7 +11,7 @@ MasterCounters::MasterCounters(communication::rpc::Server *server)
   rpc_server_->Register<CountersGetRpc>(
       [this](const auto &req_reader, auto *res_builder) {
         CountersGetRes res(Get(req_reader.getName()));
-        res.Save(res_builder);
+        Save(res, res_builder);
       });
   rpc_server_->Register<CountersSetRpc>(
       [this](const auto &req_reader, auto *res_builder) {

src/database/state_delta.lcp

@@ -53,7 +53,8 @@ cpp<#
    (value "PropertyValue" :initval "PropertyValue::Null"
           :capnp-type "Storage.PropertyValue"
           :capnp-save
-          (lambda (builder member)
+          (lambda (builder member capnp-name)
+            (declare (ignore capnp-name))
             #>cpp
             storage::SaveCapnpPropertyValue(${member}, &${builder});
             cpp<#)

src/distributed/bfs_rpc_messages.lcp

@@ -52,7 +52,8 @@ cpp<#
     (:request ((subcursor-ids "std::unordered_map<int16_t, int64_t>"
                               :capnp-type "Utils.Map(Utils.BoxInt16, Utils.BoxInt64)"
                               :capnp-save
-                              (lambda (builder member)
+                              (lambda (builder member capnp-name)
+                                (declare (ignore capnp-name))
                                 #>cpp
                                 utils::SaveMap<utils::capnp::BoxInt16, utils::capnp::BoxInt64>(
                                     ${member}, &${builder},

src/distributed/bfs_rpc_server.hpp

@@ -27,7 +27,7 @@ class BfsRpcServer {
           req.Load(req_reader);
           CreateBfsSubcursorRes res(subcursor_storage_->Create(
               req.tx_id, req.direction, req.edge_types, req.graph_view));
-          res.Save(res_builder);
+          Save(res, res_builder);
         });
 
     server_->Register<RegisterSubcursorsRpc>(
@@ -37,7 +37,7 @@ class BfsRpcServer {
           subcursor_storage_->Get(req.subcursor_ids.at(db_->WorkerId()))
               ->RegisterSubcursors(req.subcursor_ids);
           RegisterSubcursorsRes res;
-          res.Save(res_builder);
+          Save(res, res_builder);
         });
 
     server_->Register<ResetSubcursorRpc>(
@@ -46,7 +46,7 @@ class BfsRpcServer {
           req.Load(req_reader);
           subcursor_storage_->Get(req.subcursor_id)->Reset();
           ResetSubcursorRes res;
-          res.Save(res_builder);
+          Save(res, res_builder);
         });
 
     server_->Register<RemoveBfsSubcursorRpc>(
@@ -55,7 +55,7 @@ class BfsRpcServer {
           req.Load(req_reader);
           subcursor_storage_->Erase(req.member);
           RemoveBfsSubcursorRes res;
-          res.Save(res_builder);
+          Save(res, res_builder);
         });
 
     server_->Register<SetSourceRpc>(
@@ -64,7 +64,7 @@ class BfsRpcServer {
           req.Load(req_reader);
           subcursor_storage_->Get(req.subcursor_id)->SetSource(req.source);
           SetSourceRes res;
-          res.Save(res_builder);
+          Save(res, res_builder);
         });
 
     server_->Register<ExpandLevelRpc>([this](const auto &req_reader,
@@ -72,7 +72,7 @@ class BfsRpcServer {
       ExpandLevelReq req;
       req.Load(req_reader);
       ExpandLevelRes res(subcursor_storage_->Get(req.member)->ExpandLevel());
-      res.Save(res_builder);
+      Save(res, res_builder);
     });
 
     server_->Register<SubcursorPullRpc>(
@@ -81,7 +81,7 @@ class BfsRpcServer {
           req.Load(req_reader);
           auto vertex = subcursor_storage_->Get(req.member)->Pull();
           SubcursorPullRes res(vertex);
-          res.Save(res_builder, db_->WorkerId());
+          Save(res, res_builder, db_->WorkerId());
         });
 
     server_->Register<ExpandToRemoteVertexRpc>(
@@ -91,7 +91,7 @@ class BfsRpcServer {
           ExpandToRemoteVertexRes res(
               subcursor_storage_->Get(req.subcursor_id)
                   ->ExpandToLocalVertex(req.edge, req.vertex));
-          res.Save(res_builder);
+          Save(res, res_builder);
         });
 
     server_->Register<ReconstructPathRpc>([this](const auto &req_reader,
@@ -108,7 +108,7 @@ class BfsRpcServer {
         LOG(FATAL) << "`edge` or `vertex` should be set in ReconstructPathReq";
       }
       ReconstructPathRes res(result.edges, result.next_vertex, result.next_edge);
-      res.Save(res_builder, db_->WorkerId());
+      Save(res, res_builder, db_->WorkerId());
     });
 
     server_->Register<PrepareForExpandRpc>([this](const auto &req_reader,
@@ -117,7 +117,7 @@ class BfsRpcServer {
       req.Load(req_reader);
       subcursor_storage_->Get(req.subcursor_id)->PrepareForExpand(req.clear);
       PrepareForExpandRes res;
-      res.Save(res_builder);
+      Save(res, res_builder);
     });
   }
 

src/distributed/cluster_discovery_master.cpp

@@ -77,7 +77,7 @@ ClusterDiscoveryMaster::ClusterDiscoveryMaster(
     RegisterWorkerRes res(registration_successful, durability_error,
                           coordination_->RecoveredSnapshotTx(),
                           coordination_->GetWorkers());
-    res.Save(res_builder);
+    Save(res, res_builder);
   });
 
   server_->Register<NotifyWorkerRecoveredRpc>([this](const auto &req_reader,

src/distributed/coordination_rpc_messages.lcp

@@ -29,7 +29,8 @@ cpp<#
     (snapshot-to-recover "std::experimental::optional<std::pair<int64_t, tx::TransactionId>>"
                          :capnp-type "Utils.Optional(Utils.Pair(Utils.BoxUInt64, Utils.BoxUInt64))"
                          :capnp-save
-                         (lambda (builder member)
+                         (lambda (builder member capnp-name)
+                           (declare (ignore capnp-name))
                            #>cpp
                            utils::SaveOptional<utils::capnp::Pair<utils::capnp::BoxUInt64, utils::capnp::BoxUInt64>, std::pair<int64_t, tx::TransactionId>>(
                                ${member}, &${builder},
@@ -51,15 +52,16 @@ cpp<#
     (workers "std::unordered_map<int, io::network::Endpoint>"
              :capnp-type "Utils.Map(Utils.BoxInt16, Io.Endpoint)"
              :capnp-save
-             (lambda (builder member)
+             (lambda (builder member capnp-name)
+               (declare (ignore capnp-name))
                #>cpp
-               utils::SaveMap<utils::capnp::BoxInt16, io::network::capnp::Endpoint>(${member}, &${builder},
-                              [](auto *builder, const auto &entry) {
-                                  auto key_builder = builder->initKey();
-                                  key_builder.setValue(entry.first);
-                                  auto value_builder = builder->initValue();
-                                  entry.second.Save(&value_builder);
-                              });
+               utils::SaveMap<utils::capnp::BoxInt16, io::network::capnp::Endpoint>(
+                   ${member}, &${builder}, [](auto *builder, const auto &entry) {
+                     auto key_builder = builder->initKey();
+                     key_builder.setValue(entry.first);
+                     auto value_builder = builder->initValue();
+                     Save(entry.second, &value_builder);
+                   });
                cpp<#)
              :capnp-load
              (lambda (reader member)

src/distributed/data_rpc_messages.lcp

@@ -32,9 +32,10 @@ cpp<#
     (vertex-input "const Vertex *"
                   :capnp-type "Storage.Vertex"
                   :capnp-save
-                  (lambda (builder member)
+                  (lambda (builder member capnp-name)
+                    (declare (ignore capnp-name))
                     #>cpp
-                    storage::SaveVertex(*${member}, &${builder}, worker_id);
+                    storage::SaveVertex(*${member}, &${builder}, self.worker_id);
                     cpp<#)
                   :capnp-load
                   (lambda (reader member)
@@ -53,9 +54,10 @@ cpp<#
     (edge-input "const Edge *"
                 :capnp-type "Storage.Edge"
                 :capnp-save
-                (lambda (builder member)
+                (lambda (builder member capnp-name)
+                  (declare (ignore capnp-name))
                   #>cpp
-                  storage::SaveEdge(*${member}, &${builder}, worker_id);
+                  storage::SaveEdge(*${member}, &${builder}, self.worker_id);
                   cpp<#)
                 :capnp-load
                 (lambda (reader member)

src/distributed/data_rpc_server.cpp

@@ -18,7 +18,7 @@ DataRpcServer::DataRpcServer(database::DistributedGraphDb *db,
         CHECK(vertex.GetOld())
             << "Old record must exist when sending vertex by RPC";
         VertexRes response(vertex.CypherId(), vertex.GetOld(), db_->WorkerId());
-        response.Save(res_builder);
+        Save(response, res_builder);
       });
 
   rpc_server_->Register<EdgeRpc>([this](const auto &req_reader,
@@ -27,7 +27,7 @@ DataRpcServer::DataRpcServer(database::DistributedGraphDb *db,
     auto edge = dba->FindEdge(req_reader.getMember().getGid(), false);
     CHECK(edge.GetOld()) << "Old record must exist when sending edge by RPC";
     EdgeRes response(edge.CypherId(), edge.GetOld(), db_->WorkerId());
-    response.Save(res_builder);
+    Save(response, res_builder);
   });
 
   rpc_server_->Register<VertexCountRpc>(
@@ -38,7 +38,7 @@ DataRpcServer::DataRpcServer(database::DistributedGraphDb *db,
         int64_t size = 0;
         for (auto vertex : dba->Vertices(false)) ++size;
         VertexCountRes res(size);
-        res.Save(res_builder);
+        Save(res, res_builder);
       });
 }
 

src/distributed/durability_rpc_worker.cpp

@@ -13,7 +13,7 @@ DurabilityRpcWorker::DurabilityRpcWorker(database::Worker *db,
       [this](const auto &req_reader, auto *res_builder) {
         auto dba = db_->Access(req_reader.getMember());
         MakeSnapshotRes res(db_->MakeSnapshot(*dba));
-        res.Save(res_builder);
+        Save(res, res_builder);
       });
 
   rpc_server_->Register<RecoverWalAndIndexesRpc>(

src/distributed/dynamic_worker.cpp

@@ -15,7 +15,7 @@ DynamicWorkerAddition::DynamicWorkerAddition(database::DistributedGraphDb *db,
         DynamicWorkerReq req;
         req.Load(req_reader);
         DynamicWorkerRes res(this->GetIndicesToCreate());
-        res.Save(res_builder);
+        Save(res, res_builder);
       });
 }
 

src/distributed/dynamic_worker_rpc_messages.lcp

@@ -20,7 +20,8 @@ cpp<#
      ((recover-indices "std::vector<std::pair<std::string, std::string>>"
                       :capnp-type "List(Utils.Pair(Text, Text))"
                       :capnp-save
-                      (lambda (builder member)
+                      (lambda (builder member capnp-name)
+                        (declare (ignore capnp-name))
                         #>cpp
                         utils::SaveVector<utils::capnp::Pair<::capnp::Text, ::capnp::Text>,
                                           std::pair<std::string, std::string>>(

src/distributed/plan_consumer.cpp

@@ -12,7 +12,7 @@ PlanConsumer::PlanConsumer(communication::rpc::Server &server)
             req.plan_id, std::make_unique<PlanPack>(req.plan, req.symbol_table,
                                                     std::move(req.storage)));
         DispatchPlanRes res;
-        res.Save(res_builder);
+        Save(res, res_builder);
       });
 
   server_.Register<RemovePlanRpc>(

src/distributed/plan_rpc_messages.lcp

@@ -29,13 +29,14 @@ cpp<#
   storage = std::move(helper.ast_storage);
   cpp<#)
 
-(defun save-plan (builder member)
+(defun save-plan (builder member capnp-name)
+  (declare (ignore capnp-name))
   #>cpp
   query::plan::LogicalOperator::SaveHelper helper;
   utils::SaveSharedPtr<query::plan::capnp::LogicalOperator, query::plan::LogicalOperator>(
       ${member}, &${builder},
       [&helper](auto *builder, const auto &val) {
-        val.Save(builder, &helper);
+        Save(val, builder, &helper);
       }, &helper.saved_ops);
   cpp<#)
 

src/distributed/produce_rpc_server.cpp

@@ -112,7 +112,7 @@ ProduceRpcServer::ProduceRpcServer(database::Worker *db,
         PullReq req;
         req.Load(req_reader);
         PullRes res(Pull(req));
-        res.Save(res_builder);
+        Save(res, res_builder);
       });
 
   produce_rpc_server_.Register<ResetCursorRpc>(
@@ -121,7 +121,7 @@ ProduceRpcServer::ProduceRpcServer(database::Worker *db,
         req.Load(req_reader);
         Reset(req);
         ResetCursorRes res;
-        res.Save(res_builder);
+        Save(res, res_builder);
       });
 
   CHECK(data_manager);
@@ -133,7 +133,7 @@ ProduceRpcServer::ProduceRpcServer(database::Worker *db,
         tx_engine_->UpdateCommand(req.member);
         data_manager->ClearCacheForSingleTransaction(req.member);
         TransactionCommandAdvancedRes res;
-        res.Save(res_builder);
+        Save(res, res_builder);
       });
 }
 

src/distributed/pull_produce_rpc_messages.lcp

@@ -72,7 +72,8 @@ the relevant parts of the response, ready for use."))
    (frames "std::vector<std::vector<query::TypedValue>>"
            :capnp-type "List(List(Query.TypedValue))"
            :capnp-save
-           (lambda (builder member)
+           (lambda (builder member capnp-name)
+             (declare (ignore capnp-name))
              #>cpp
              for (size_t frame_i = 0; frame_i < ${member}.size(); ++frame_i) {
                const auto &frame = ${member}[frame_i];
@@ -80,7 +81,7 @@ the relevant parts of the response, ready for use."))
                for (size_t val_i = 0; val_i < frame.size(); ++val_i) {
                  const auto &value = frame[val_i];
                  auto value_builder = frame_builder[val_i];
-                 query::SaveCapnpTypedValue(value, &value_builder, send_versions, worker_id);
+                 query::SaveCapnpTypedValue(value, &value_builder, self.send_versions, self.worker_id);
                }
              }
              cpp<#)
@@ -188,7 +189,8 @@ to the appropriate value. Not used on side that generates the response.")
       (evaluation-context "query::EvaluationContext"
                           :capnp-type "Query.EvaluationContext"
                           :capnp-save
-                          (lambda (builder member)
+                          (lambda (builder member capnp-name)
+                            (declare (ignore capnp-name))
                             #>cpp
 			    ${builder}.setTimestamp(${member}.timestamp);
 			    auto params_builder = ${builder}.initParams().initEntries(${member}.parameters.size());

src/distributed/updates_rpc_messages.lcp

@@ -59,13 +59,14 @@ cpp<#
    (properties "std::unordered_map<storage::Property, PropertyValue>"
                :capnp-type "Utils.Map(Storage.Common, Storage.PropertyValue)"
                :capnp-save
-               (lambda (builder member)
+               (lambda (builder member capnp-name)
+                 (declare (ignore capnp-name))
                  #>cpp
                  utils::SaveMap<storage::capnp::Common, storage::capnp::PropertyValue>(
                      ${member}, &${builder},
                      [](auto *builder, const auto &entry) {
                        auto key_builder = builder->initKey();
-                       entry.first.Save(&key_builder);
+                       Save(entry.first, &key_builder);
                        auto value_builder = builder->initValue();
                        storage::SaveCapnpPropertyValue(entry.second, &value_builder);
                      });
@@ -86,7 +87,8 @@ cpp<#
    (cypher-id "std::experimental::optional<int64_t>"
      :capnp-type "Utils.Optional(Utils.BoxInt64)"
      :capnp-save
-     (lambda (builder member)
+     (lambda (builder member capnp-name)
+       (declare (ignore capnp-name))
        #>cpp
        utils::SaveOptional<utils::capnp::BoxInt64, int64_t>(
          ${member}, &${builder}, [](auto *builder, const auto &value) {
@@ -115,7 +117,8 @@ cpp<#
    (cypher-id "std::experimental::optional<int64_t>"
      :capnp-type "Utils.Optional(Utils.BoxInt64)"
      :capnp-save
-     (lambda (builder member)
+     (lambda (builder member capnp-name)
+       (declare (ignore capnp-name))
        #>cpp
        utils::SaveOptional<utils::capnp::BoxInt64, int64_t>(
          ${member}, &${builder}, [](auto *builder, const auto &value) {

src/distributed/updates_rpc_server.cpp

@@ -193,13 +193,13 @@ UpdatesRpcServer::UpdatesRpcServer(database::DistributedGraphDb *db,
       case database::StateDelta::Type::REMOVE_IN_EDGE: {
         UpdateRes res(
             GetUpdates(vertex_updates_, delta.transaction_id).Emplace(delta));
-        res.Save(res_builder);
+        Save(res, res_builder);
         return;
       }
       case DeltaType::SET_PROPERTY_EDGE: {
         UpdateRes res(
             GetUpdates(edge_updates_, delta.transaction_id).Emplace(delta));
-        res.Save(res_builder);
+        Save(res, res_builder);
         return;
       }
       default:
@@ -213,7 +213,7 @@ UpdatesRpcServer::UpdatesRpcServer(database::DistributedGraphDb *db,
         UpdateApplyReq req;
         req.Load(req_reader);
         UpdateApplyRes res(Apply(req.member));
-        res.Save(res_builder);
+        Save(res, res_builder);
       });
 
   server->Register<CreateVertexRpc>([this](const auto &req_reader,
@@ -225,7 +225,7 @@ UpdatesRpcServer::UpdatesRpcServer(database::DistributedGraphDb *db,
                                     req.member.cypher_id);
     CreateVertexRes res(
         CreateResult{UpdateResult::DONE, result.cypher_id, result.gid});
-    res.Save(res_builder);
+    Save(res, res_builder);
   });
 
   server->Register<CreateEdgeRpc>(
@@ -247,7 +247,7 @@ UpdatesRpcServer::UpdatesRpcServer(database::DistributedGraphDb *db,
         }
 
         CreateEdgeRes res(creation_result);
-        res.Save(res_builder);
+        Save(res, res_builder);
       });
 
   server->Register<AddInEdgeRpc>(
@@ -260,7 +260,7 @@ UpdatesRpcServer::UpdatesRpcServer(database::DistributedGraphDb *db,
         auto result =
             GetUpdates(vertex_updates_, req.member.tx_id).Emplace(to_delta);
         AddInEdgeRes res(result);
-        res.Save(res_builder);
+        Save(res, res_builder);
       });
 
   server->Register<RemoveVertexRpc>(
@@ -272,7 +272,7 @@ UpdatesRpcServer::UpdatesRpcServer(database::DistributedGraphDb *db,
         auto result =
             GetUpdates(vertex_updates_, req.member.tx_id).Emplace(to_delta);
         RemoveVertexRes res(result);
-        res.Save(res_builder);
+        Save(res, res_builder);
       });
 
   server->Register<RemoveEdgeRpc>(
@@ -280,7 +280,7 @@ UpdatesRpcServer::UpdatesRpcServer(database::DistributedGraphDb *db,
         RemoveEdgeReq req;
         req.Load(req_reader);
         RemoveEdgeRes res(RemoveEdge(req.member));
-        res.Save(res_builder);
+        Save(res, res_builder);
       });
 
   server->Register<RemoveInEdgeRpc>([this](const auto &req_reader,
@@ -291,7 +291,7 @@ UpdatesRpcServer::UpdatesRpcServer(database::DistributedGraphDb *db,
     RemoveInEdgeRes res(GetUpdates(vertex_updates_, data.tx_id)
                             .Emplace(database::StateDelta::RemoveInEdge(
                                 data.tx_id, data.vertex, data.edge_address)));
-    res.Save(res_builder);
+    Save(res, res_builder);
   });
 }
 

src/durability/recovery.hpp

@@ -35,13 +35,6 @@ struct RecoveryInfo {
   }
   bool operator!=(const RecoveryInfo &other) const { return !(*this == other); }
 
-  void Save(capnp::RecoveryInfo::Builder *builder) const {
-    builder->setDurabilityVersion(durability_version);
-    builder->setSnapshotTxId(snapshot_tx_id);
-    auto list_builder = builder->initWalRecovered(wal_recovered.size());
-    utils::SaveVector(wal_recovered, &list_builder);
-  }
-
   void Load(const capnp::RecoveryInfo::Reader &reader) {
     durability_version = reader.getDurabilityVersion();
     snapshot_tx_id = reader.getSnapshotTxId();
@@ -50,6 +43,14 @@ struct RecoveryInfo {
   }
 };
 
+inline void Save(const RecoveryInfo &info,
+                 capnp::RecoveryInfo::Builder *builder) {
+  builder->setDurabilityVersion(info.durability_version);
+  builder->setSnapshotTxId(info.snapshot_tx_id);
+  auto list_builder = builder->initWalRecovered(info.wal_recovered.size());
+  utils::SaveVector(info.wal_recovered, &list_builder);
+}
+
 // A data structure for exchanging info between main recovery function and
 // snapshot and WAL recovery functions.
 struct RecoveryData {
@@ -66,28 +67,6 @@ struct RecoveryData {
     indexes.clear();
   }
 
-  void Save(capnp::RecoveryData::Builder *builder) const {
-    builder->setSnapshooterTxId(snapshooter_tx_id);
-    {
-      auto list_builder = builder->initWalTxToRecover(wal_tx_to_recover.size());
-      utils::SaveVector(wal_tx_to_recover, &list_builder);
-    }
-    {
-      auto list_builder =
-          builder->initSnapshooterTxSnapshot(snapshooter_tx_snapshot.size());
-      utils::SaveVector(snapshooter_tx_snapshot, &list_builder);
-    }
-    {
-      auto list_builder = builder->initIndexes(indexes.size());
-      utils::SaveVector<utils::capnp::Pair<::capnp::Text, ::capnp::Text>,
-                        std::pair<std::string, std::string>>(
-          indexes, &list_builder, [](auto *builder, const auto value) {
-            builder->setFirst(value.first);
-            builder->setSecond(value.second);
-          });
-    }
-  }
-
   void Load(const capnp::RecoveryData::Reader &reader) {
     snapshooter_tx_id = reader.getSnapshooterTxId();
     {
@@ -109,6 +88,30 @@ struct RecoveryData {
   }
 };
 
+inline void Save(const RecoveryData &data,
+                 capnp::RecoveryData::Builder *builder) {
+  builder->setSnapshooterTxId(data.snapshooter_tx_id);
+  {
+    auto list_builder =
+        builder->initWalTxToRecover(data.wal_tx_to_recover.size());
+    utils::SaveVector(data.wal_tx_to_recover, &list_builder);
+  }
+  {
+    auto list_builder =
+        builder->initSnapshooterTxSnapshot(data.snapshooter_tx_snapshot.size());
+    utils::SaveVector(data.snapshooter_tx_snapshot, &list_builder);
+  }
+  {
+    auto list_builder = builder->initIndexes(data.indexes.size());
+    utils::SaveVector<utils::capnp::Pair<::capnp::Text, ::capnp::Text>,
+                      std::pair<std::string, std::string>>(
+        data.indexes, &list_builder, [](auto *builder, const auto value) {
+          builder->setFirst(value.first);
+          builder->setSecond(value.second);
+        });
+  }
+}
+
 /** Reads snapshot metadata from the end of the file without messing up the
  * hash. */
 bool ReadSnapshotSummary(HashedFileReader &buffer, int64_t &vertex_count,

src/io/network/endpoint.cpp

@@ -24,10 +24,10 @@ Endpoint::Endpoint(const std::string &address, uint16_t port)
   CHECK(family_ != 0) << "Not a valid IPv4 or IPv6 address: " << address;
 }
 
-void Endpoint::Save(capnp::Endpoint::Builder *builder) const {
-  builder->setAddress(address_);
-  builder->setPort(port_);
-  builder->setFamily(family_);
+void Save(const Endpoint &endpoint, capnp::Endpoint::Builder *builder) {
+  builder->setAddress(endpoint.address());
+  builder->setPort(endpoint.port());
+  builder->setFamily(endpoint.family());
 }
 
 void Endpoint::Load(const capnp::Endpoint::Reader &reader) {

src/io/network/endpoint.hpp

@@ -27,7 +27,6 @@ class Endpoint {
   bool operator==(const Endpoint &other) const;
   friend std::ostream &operator<<(std::ostream &os, const Endpoint &endpoint);
 
-  void Save(capnp::Endpoint::Builder *builder) const;
   void Load(const capnp::Endpoint::Reader &reader);
 
  private:
@@ -36,4 +35,6 @@ class Endpoint {
   unsigned char family_{0};
 };
 
+void Save(const Endpoint &endpoint, capnp::Endpoint::Builder *builder);
+
 }  // namespace io::network

src/lisp/lcp.lisp

@@ -414,10 +414,8 @@ NIL, returns a string."
           (format s "~{  ~%~A~}~%"
                   (mapcar #'member-declaration (cpp-class-members-scoped :public)))))
       (when (cpp-class-capnp-opts cpp-class)
-        (let ((save (capnp-save-declaration cpp-class))
-              (construct (capnp-construct-declaration cpp-class))
+        (let ((construct (capnp-construct-declaration cpp-class))
               (load (capnp-load-declaration cpp-class)))
-          (when save (format s "  ~A;~2%" save))
           (when construct (format s "  ~A;~2%" construct))
           (when load (format s "  ~A;~2%" load))))
       (when (or (cpp-class-protected cpp-class) (cpp-class-members-scoped :protected))
@@ -432,6 +430,24 @@ NIL, returns a string."
                 (mapcar #'member-declaration (cpp-class-members-scoped :private))))
       (write-line "};" s))))
 
+(defun cpp-function-declaration (name &key args (returns "void") type-params)
+  "Generate a C++ top level function declaration named NAME as a string.  ARGS
+is a list of (variable type) function arguments. RETURNS is the return type of
+the function.  TYPE-PARAMS is a list of names for template argments"
+  (declare (type string name))
+  (declare (type string returns))
+  (let ((template (if type-params (cpp-template type-params) ""))
+        (args (format nil "~:{~A ~A~:^, ~}"
+                      (mapcar (lambda (name-and-type)
+                                (list (cpp-type-name (second name-and-type))
+                                      (cpp-variable-name (first name-and-type))))
+                              args))))
+    (raw-cpp-string
+     #>cpp
+     ${template}
+     ${returns} ${name}(${args})
+     cpp<#)))
+
 (defun cpp-method-declaration (class method-name
                                &key args (returns "void") (inline t) static
                                  virtual const override)
@@ -755,54 +771,67 @@ encoded as union inheritance in Cap'n Proto."
 ;;;
 ;;; Algorithm is closely tied with the generated schema (see above).
 ;;;
-;;; 1) Generate the method declaration.
+;;; 1) Generate the function declaration.
+;;;
+;;; We are using top level functions, so that we can easily decouple the
+;;; serialization code from class definitions.  This requires the class to
+;;; have public access to its serializable fields.
 ;;;
 ;;; Two problems arise:
+;;;
 ;;;  * inheritance and
 ;;;  * helper arguments (for tracking pointers or similar).
 ;;;
-;;; The method will always take a pointer to a capnp::<T>::Builder
-;;; class. Additional arguments are optional, and are supplied when declaring
-;;; that the class should be serialized with capnp.
+;;; The function will always take a `const &T` and a pointer to a
+;;; `capnp::<T>::Builder` class.  Additional arguments are optional, and are
+;;; supplied when declaring that the class should be serialized with capnp.
 ;;;
 ;;; To determine the concrete T we need to know whether this class is a
-;;; derived one or is inherited from. If it is, then T needs to be the
-;;; top-most parent that is modeled by union and not composition. (For the
+;;; derived one or is inherited from.  If it is, then T needs to be the
+;;; top-most parent that is modeled by union and not composition.  (For the
 ;;; inheritance modeling problem, refer to the description of schema
-;;; generation.) Obviously, the method now needs to be virtual. If this class
-;;; has no inheritance in any direction, then we just use the class name for
-;;; T prepended with capnp:: namespace.
+;;; generation.)  Since we opted for using top level functions, we cannot use
+;;; virtual call dispatch to get the concrete type.  (We could use the visitor
+;;; pattern, but that introduces the coupling we are avoiding with regular
+;;; functions.)  Therefore, we use dynamic_cast in functions to determine the
+;;; concrete serialization code.  If this class has no inheritance in any
+;;; direction, then we just serialize T to its corresponding capnp::T schema
+;;; type.
 ;;;
 ;;; Helper arguments are obtained from SAVE-ARGS of `CAPNP-OPTS'.
 ;;;
 ;;; 2) Generate parent calls for serialization (if we have parent classes).
 ;;;
-;;; For the first (and only) parent which is modeled through union, generate a
-;;; <Parent>::Save call. The call is passed all of the arguments from out
-;;; function declaration.
+;;; For the first (and only) parent which is modeled through union, generate
+;;; the parent serialization code.  This is done recursively for each union
+;;; parent.  The generated code sees all of the arguments from our function
+;;; declaration.
 ;;;
-;;; Find our own concrete builder by traversing through the union schema of
-;;; the base builder. It is expected (and required) that the parent call has
-;;; initialized them correctly. We just need to initialize the most concrete
-;;; builder.
+;;; Then, find our own concrete builder by traversing through the union schema
+;;; of the base builder.  It is expected (and required) that the parent code
+;;; has initialized them correctly.  We just need to initialize the most
+;;; concrete builder.
 ;;;
-;;; Other parents are required to be modelled through composition. Therefore,
+;;; Other parents are required to be modeled through composition.  Therefore,
 ;;; we generate calls to parents by passing builders for the composed structs.
+;;;
 ;;;     auto parent_builder = builder->initParent();
-;;;     Parent::Save(&parent_builder);
-;;; Any additional helper arguments are also passed to the above call.
+;;;     // Parent Save code
+;;;
+;;; Any additional helper arguments are also visited in the generated code.
 ;;;
 ;;; 3) Generate member serialization.
 ;;;
-;;; For primitive typed member, generate builder->setMember(member_); calls.
+;;; For primitive typed members, generate `builder->setMember(member);` calls.
 ;;;
-;;; For std types, generate hardcoded calls to our wrapper functions. Most of
-;;; these require a lambda function which serializes the element inside the
-;;; std class. This can be done recursively with this step.
+;;; For `std` types, generate hard-coded calls to our wrapper functions.  Most
+;;; of these require a lambda function which serializes the element inside the
+;;; `std` class.  This can be done recursively with this step.
 ;;;
 ;;; For composite types, check whether we have been given a custom save
-;;; invocation. If not, assume that the type has a member function called Save
-;;; which expects a builder for that type and any additional helper arguments.
+;;; invocation.  If not, assume that the type has an accompanying function
+;;; called `Save` which expects an instance of that type and a builder for it,
+;;; as well as any additional helper arguments.
 
 (defun capnp-extra-args (cpp-class save-or-load)
   "Get additional arguments to Save/Load function for CPP-CLASS."
@@ -815,22 +844,25 @@ encoded as union inheritance in Cap'n Proto."
                   (:load (capnp-opts-load-args opts)))
      when args return args))
 
-(defun capnp-save-declaration (cpp-class &key (inline t))
-  "Generate Cap'n Proto save function declaration for CPP-CLASS. If
-INLINE is NIL, the declaration is namespaced for the class so that it can be
-used for outside definition."
+(defun capnp-save-function-declaration (cpp-class)
+  "Generate Cap'n Proto save function declaration for CPP-CLASS."
   (declare (type cpp-class cpp-class))
   (let* ((parents (capnp-union-parents-rec cpp-class))
-         (top-parent-class (if parents
-                               (cpp-type-decl (find-cpp-class (car (last parents))) :type-args nil :namespace nil)
-                               (cpp-type-decl cpp-class :type-args nil :namespace nil)))
+         (top-parent-class
+          (if parents
+              (cpp-type-decl (find-cpp-class (car (last parents))) :type-args nil :namespace nil)
+              (cpp-type-decl cpp-class :type-args nil :namespace nil)))
+         (self-arg
+          (list 'self (format nil "const ~A &"
+                              (cpp-type-decl cpp-class :namespace nil))))
          (builder-arg
           (list (if parents 'base-builder 'builder)
                 (format nil "capnp::~A::Builder *" top-parent-class))))
-    (cpp-method-declaration
-     cpp-class "Save" :args (cons builder-arg (capnp-extra-args cpp-class :save))
-     :virtual (and (not parents) (capnp-union-subclasses cpp-class))
-     :const t :override parents :inline inline)))
+    (cpp-function-declaration
+     "Save"
+     :args (cons self-arg
+                 (cons builder-arg (capnp-extra-args cpp-class :save)))
+     :type-params (cpp-type-type-params cpp-class))))
 
 (defun capnp-cpp-type<-cpp-type (cpp-type &key boxp)
   (declare (type cpp-type cpp-type))
@@ -852,9 +884,12 @@ used for outside definition."
                    :name name :namespace namespace
                    :enclosing-class (cpp-type-enclosing-class cpp-type))))
 
-(defun capnp-save-default (member-name member-type member-builder)
-  "Generate the default call to save for member."
-  (declare (type string member-name member-type member-builder))
+(defun capnp-save-default (member-name member-type member-builder capnp-name)
+  "Generate the default call to save for member.  MEMBER-NAME and MEMBER-TYPE
+are strings describing the member being serialized.  MEMBER-BUILDER is the
+name of the builder variable.  CAPNP-NAME is the name of the member in Cap'n
+Proto schema."
+  (declare (type string member-name member-type member-builder capnp-name))
   (let* ((type (parse-cpp-type-declaration member-type))
          (type-name (cpp-type-base-name type))
          (cpp-enum (find-cpp-enum member-type)))
@@ -862,8 +897,8 @@ used for outside definition."
       (cpp-enum
        (funcall
         (capnp-save-enum (cpp-type-decl (capnp-cpp-type<-cpp-type cpp-enum))
-                         member-type (cpp-enum-values cpp-enum))
-        member-builder member-name))
+                         (cpp-type-decl cpp-enum) (cpp-enum-values cpp-enum))
+        member-builder member-name capnp-name))
       ((string= "vector" type-name)
        (let* ((elem-type (car (cpp-type-type-args type)))
               (capnp-cpp-type (capnp-cpp-type<-cpp-type elem-type)))
@@ -874,7 +909,7 @@ used for outside definition."
               cpp<#)
              (raw-cpp-string
               (funcall (capnp-save-vector (cpp-type-decl capnp-cpp-type) (cpp-type-decl elem-type))
-                       member-builder member-name)))))
+                       member-builder member-name capnp-name)))))
       ((string= "optional" type-name)
        (let* ((elem-type (car (cpp-type-type-args type)))
               (capnp-cpp-type (capnp-cpp-type<-cpp-type elem-type :boxp t))
@@ -883,7 +918,7 @@ used for outside definition."
          (raw-cpp-string
           (funcall (capnp-save-optional
                     (cpp-type-decl capnp-cpp-type) (cpp-type-decl elem-type) lambda-code)
-                   member-builder member-name))))
+                   member-builder member-name capnp-name))))
       ((member type-name '("unique_ptr" "shared_ptr" "vector") :test #'string=)
        (error "Use a custom :capnp-save function for ~A ~A" type-name member-name))
       (t
@@ -892,28 +927,67 @@ used for outside definition."
                             (mapcar (lambda (name-and-type)
                                       (cpp-variable-name (first name-and-type)))
                                     (capnp-extra-args cpp-class :save)))))
-         (format nil "~A.Save(&~A~{, ~A~});"
+         (format nil "Save(~A, &~A~{, ~A~});"
                  member-name member-builder extra-args))))))
 
-(defun capnp-save-code (cpp-class)
-  "Generate Cap'n Proto saving code for CPP-CLASS"
+(defun capnp-save-members (cpp-class &key instance-access)
+  "Generate Cap'n Proto saving code for members of CPP-CLASS.  INSTANCE-ACCESS
+  is a C++ string which is prefixed to member access.  For example,
+  INSTANCE-ACCESS could be `my_struct->`"
   (declare (type cpp-class cpp-class))
+  (declare (type string instance-access))
   (with-output-to-string (s)
-    (format s "~A {~%" (capnp-save-declaration cpp-class :inline nil))
-    (flet ((parent-args (parent)
-             (mapcar (lambda (name-and-type)
-                       (cpp-variable-name (first name-and-type)))
-                     (capnp-extra-args (find-cpp-class parent) :save))))
-      (multiple-value-bind (direct-union-parents compose-parents)
-          (capnp-union-and-compose-parents cpp-class)
-        (declare (ignore direct-union-parents))
-        ;; Handle the union inheritance calls first.
-        (let ((parents (capnp-union-parents-rec cpp-class)))
-          (when parents
-            (let ((first-parent (first parents)))
-              (format s " ~A::Save(base_builder~{, ~A~});~%"
-                      (cpp-type-name first-parent) (parent-args first-parent)))
-            (if (or compose-parents (cpp-class-members cpp-class))
+    (dolist (member (cpp-class-members cpp-class))
+      (unless (eq :dont-save (cpp-member-capnp-save member))
+        (let ((member-access
+               (concatenate 'string instance-access
+                            (if (eq :public (cpp-member-scope member))
+                                (cpp-member-name member :struct (cpp-class-structp cpp-class))
+                                (format nil "~A()" (cpp-member-name member :struct t)))))
+              (member-builder (format nil "~A_builder" (cpp-member-name member :struct t)))
+              (capnp-name (cpp-type-name (cpp-member-symbol member))))
+          (cond
+            ((and (not (cpp-member-capnp-save member))
+                  (capnp-primitive-type-p (capnp-type-of-member member)))
+             (format s "  builder->set~A(~A);~%" capnp-name member-access))
+            (t
+             (write-line "{" s) ;; Enclose larger save code in new scope
+             (let ((size (if (string= "vector" (cpp-type-base-name
+                                                (parse-cpp-type-declaration
+                                                 (cpp-member-type member))))
+                             (format nil "~A.size()" member-access)
+                             "")))
+               (if (and (cpp-member-capnp-init member)
+                        (not (find-cpp-enum (cpp-member-type member))))
+                   (format s "  auto ~A = builder->init~A(~A);~%"
+                           member-builder capnp-name size)
+                   (setf member-builder "builder")))
+             (if (cpp-member-capnp-save member)
+                 (format s "  ~A~%"
+                         (cpp-code (funcall (cpp-member-capnp-save member)
+                                            member-builder member-access capnp-name)))
+                 (write-line (capnp-save-default member-access (cpp-member-type member)
+                                                 member-builder capnp-name)
+                             s))
+             (write-line "}" s))))))))
+
+(defun capnp-save-parents (cpp-class save-parent)
+  "Generate Cap'n Proto code for serializing parent classes of CPP-CLASS.
+SAVE-PARENT is a function which generates the code for serializing a parent.
+It takes a parent class symbol."
+  (declare (type cpp-class cpp-class))
+  (declare (type (function (symbol) string) save-parent))
+  (multiple-value-bind (direct-union-parents compose-parents)
+      (capnp-union-and-compose-parents cpp-class)
+    (declare (ignore direct-union-parents))
+    ;; Handle the union inheritance calls first.
+    (with-output-to-string (s)
+      (let ((parents (capnp-union-parents-rec cpp-class)))
+        (when parents
+          (let ((first-parent (first parents)))
+            (write-line
+             (funcall save-parent first-parent) s))
+          (if (or compose-parents (cpp-class-members cpp-class))
               (progn
                 (format s "  auto ~A_builder = base_builder->~{get~A().~}init~A();~%"
                         (cpp-variable-name (cpp-type-base-name cpp-class))
@@ -924,57 +998,93 @@ used for outside definition."
               (format s "  base_builder->~{get~A().~}init~A();~%"
                       (mapcar #'cpp-type-name (cdr (reverse parents)))
                       (cpp-type-name cpp-class)))
-            (when (capnp-union-subclasses cpp-class)
-              ;; We are in the middle of inheritance hierarchy, so set our
-              ;; union Void field.
-              (format s "  builder->set~A();" (cpp-type-name cpp-class)))))
-        ;; Now handle composite inheritance calls.
-        (dolist (parent compose-parents)
-          (write-line "{" s)
-          (format s "  auto ~A_builder = builder->init~A();~%"
-                  (cpp-variable-name parent) (cpp-type-name parent))
-          (format s " ~A::Save(&~A_builder~{, ~A~});~%"
-                  (cpp-type-name parent) (cpp-variable-name parent) (parent-args parent))
-          (write-line "}" s))))
-    ;; Set the template instantiations
-    (when (and (capnp-opts-type-args (cpp-class-capnp-opts cpp-class))
-               (/= 1 (list-length (cpp-type-type-params cpp-class))))
-      (error "Don't know how to save templated class ~A" (cpp-type-base-name cpp-class)))
-    (let ((type-param (first (cpp-type-type-params cpp-class))))
-      (dolist (type-arg (capnp-opts-type-args (cpp-class-capnp-opts cpp-class)))
-        (format s "  if (std::is_same<~A, ~A>::value) { builder->set~A(); }"
-                (cpp-type-name type-arg) (cpp-type-name type-param) (cpp-type-name type-arg))))
-    (dolist (member (cpp-class-members cpp-class))
-      (unless (eq :dont-save (cpp-member-capnp-save member))
-        (let ((member-name (cpp-member-name member :struct (cpp-class-structp cpp-class)))
-              (member-builder (format nil "~A_builder" (cpp-member-name member :struct t)))
-              (capnp-name (cpp-type-name (cpp-member-symbol member))))
-          (cond
-            ((and (not (cpp-member-capnp-save member))
-                  (capnp-primitive-type-p (capnp-type-of-member member)))
-             (format s "  builder->set~A(~A);~%" capnp-name member-name))
-            (t
-             (write-line "{" s) ;; Enclose larger save code in new scope
-             (let ((size (if (string= "vector" (cpp-type-base-name
-                                                (parse-cpp-type-declaration
-                                                 (cpp-member-type member))))
-                             (format nil "~A.size()" member-name)
-                             "")))
-               (if (and (cpp-member-capnp-init member)
-                        (not (find-cpp-enum (cpp-member-type member))))
-                   (format s "  auto ~A = builder->init~A(~A);~%"
-                           member-builder capnp-name size)
-                   (setf member-builder "builder")))
-             (if (cpp-member-capnp-save member)
-                 (format s "  ~A~%"
-                         (cpp-code (funcall (cpp-member-capnp-save member)
-                                            member-builder member-name)))
-                 (write-line (capnp-save-default member-name
-                                                 (cpp-member-type member)
-                                                 member-builder)
-                             s))
-             (write-line "}" s))))))
-    (write-line "}" s)))
+          (when (capnp-union-subclasses cpp-class)
+            ;; We are in the middle of inheritance hierarchy, so set our
+            ;; union Void field.
+            (format s "  builder->set~A();" (cpp-type-name cpp-class)))))
+      ;; Now handle composite inheritance calls.
+      (dolist (parent compose-parents)
+        (write-line "{" s)
+        (let* ((builder (format nil "~A_builder" (cpp-variable-name parent))))
+          (format s "  auto ~A = builder->init~A();~%" builder (cpp-type-name parent))
+          (format s "  auto *builder = &~A;~%" builder)
+          (write-line (funcall save-parent parent) s))
+        (write-line "}" s)))))
+
+(defun capnp-save-function-code (cpp-class)
+  "Generate Cap'n Proto save code for CPP-CLASS."
+  (declare (type cpp-class cpp-class))
+  (labels ((save-class (cpp-class cpp-out)
+             "Output the serialization code for members of this and parent class."
+             (write-line (capnp-save-parents cpp-class #'save-parent) cpp-out)
+             ;; Set the template instantiations
+             (when (and (capnp-opts-type-args (cpp-class-capnp-opts cpp-class))
+                        (/= 1 (list-length (cpp-type-type-params cpp-class))))
+               (error "Don't know how to save templated class ~A" (cpp-type-base-name cpp-class)))
+             (let ((type-param (first (mapcar #'cpp-type-name (cpp-type-type-params cpp-class)))))
+               (dolist (type-arg (mapcar #'cpp-type-name
+                                         (capnp-opts-type-args (cpp-class-capnp-opts cpp-class))))
+                 (write-string
+                  (raw-cpp-string
+                   #>cpp
+                   if (std::is_same<${type-arg}, ${type-param}>::value) {
+                   builder->set${type-arg}();
+                   }
+                   cpp<#)
+                  cpp-out)))
+             (write-line (capnp-save-members cpp-class :instance-access "self.") cpp-out))
+           (save-parent (parent)
+             "Generate serialization code for parent class."
+             (let ((cpp-class (find-cpp-class parent)))
+               (with-output-to-string (s)
+                 (write-line "{" s)
+                 (format s "// Save base class ~A~%" (cpp-type-name parent))
+                 (save-class cpp-class s)
+                 (write-line "}" s)))))
+    (with-output-to-string (cpp-out)
+      (let ((subclasses (direct-subclasses-of cpp-class)))
+        (when subclasses
+          (write-line "// Forward serialization to most derived type" cpp-out)
+          (dolist (subclass subclasses)
+            (let ((derived-name (cpp-type-name subclass))
+                  (save-args
+                   (format nil "~A~{, ~A~}"
+                           (if (capnp-union-parents-rec cpp-class)
+                               "base_builder"
+                               "builder")
+                           (mapcar (lambda (name-and-type)
+                                     (cpp-variable-name (first name-and-type)))
+                                   (capnp-extra-args cpp-class :save))))
+                  (type-args (capnp-opts-type-args (cpp-class-capnp-opts subclass))))
+              (if type-args
+                  ;; Handle template instantiation
+                  (dolist (type-arg (mapcar #'cpp-type-name type-args))
+                    (write-string
+                     (raw-cpp-string
+                      #>cpp
+                      if (const auto *derived = dynamic_cast<const ${derived-name}<${type-arg}> *>(&self)) {
+                      return Save(*derived, ${save-args});
+                      }
+                      cpp<#)
+                     cpp-out))
+                  ;; Just forward the serialization normally.
+                  (write-string
+                   (raw-cpp-string
+                    #>cpp
+                    if (const auto *derived = dynamic_cast<const ${derived-name} *>(&self)) {
+                    return Save(*derived, ${save-args});
+                    }
+                    cpp<#)
+                   cpp-out))))))
+      (save-class cpp-class cpp-out))))
+
+(defun capnp-save-function-definition (cpp-class)
+  "Generate Cap'n Proto save function."
+  (declare (type cpp-class cpp-class))
+  (with-output-to-string (cpp-out)
+    (format cpp-out "~A {~%" (capnp-save-function-declaration cpp-class))
+    (write-line (capnp-save-function-code cpp-class) cpp-out)
+    (write-line "}" cpp-out)))
 
 ;;; Capnp C++ deserialization code generation
 ;;;
@@ -1188,10 +1298,16 @@ are passed as template parameters, while the optional LAMBDA-CODE is used to
 save the value inside the std::optional."
   (declare (type string capnp-type cpp-type)
            (type (or null string) lambda-code))
-  (let ((lambda-code (if lambda-code
+  ;; TODO: Try using `capnp-save-default'
+  (let* ((namespace (format nil "~{~A::~}"
+                            (cpp-type-namespace (parse-cpp-type-declaration cpp-type))))
+         (lambda-code (if lambda-code
                          lambda-code
-                         "[](auto *builder, const auto &val) { val.Save(builder); }")))
-    (lambda (builder member)
+                         (format nil
+                                 "[](auto *builder, const auto &val) { ~ASave(val, builder); }"
+                                 namespace))))
+    (lambda (builder member capnp-name)
+      (declare (ignore capnp-name))
       #>cpp
       utils::SaveOptional<${capnp-type}, ${cpp-type}>(${member}, &${builder}, ${lambda-code});
       cpp<#)))
@@ -1218,10 +1334,16 @@ are passed as template parameters, while LAMBDA-CODE is used to save each
 element."
   (declare (type string capnp-type cpp-type)
            (type (or null string) lambda-code))
-  (let ((lambda-code (if lambda-code
+  ;; TODO: Why not use our `capnp-save-default' for this?
+  (let* ((namespace (format nil "~{~A::~}"
+                           (cpp-type-namespace (parse-cpp-type-declaration cpp-type))))
+        (lambda-code (if lambda-code
                          lambda-code
-                         "[](auto *builder, const auto &val) { val.Save(builder); }")))
-    (lambda (builder member-name)
+                         (format nil
+                                 "[](auto *builder, const auto &val) { ~ASave(val, builder); }"
+                                 namespace))))
+    (lambda (builder member-name capnp-name)
+      (declare (ignore capnp-name))
       #>cpp
       utils::SaveVector<${capnp-type}, ${cpp-type}>(${member-name}, &${builder}, ${lambda-code});
       cpp<#)))
@@ -1248,16 +1370,15 @@ the values to CAPNP-TYPE. If ENUM-VALUES are not specified, tries to find the
 CPP-TYPE among defined enums."
   (declare (type string capnp-type)
            (type (or symbol string) cpp-type))
-  (lambda (builder member)
+  (lambda (builder member capnp-name)
     (let* ((enum-values (if enum-values
                             enum-values
                             (cpp-enum-values (find-cpp-enum cpp-type))))
-           (member-setter (remove #\_ (string-capitalize member)))
            (cases (mapcar (lambda (value-symbol)
                             (let ((value (cl-ppcre:regex-replace-all "-" (string value-symbol) "_")))
                               #>cpp
                               case ${cpp-type}::${value}:
-                                ${builder}->set${member-setter}(${capnp-type}::${value});
+                                ${builder}->set${capnp-name}(${capnp-type}::${value});
                                 break;
                               cpp<#))
                           enum-values)))
@@ -1542,6 +1663,41 @@ formatted and output."
                  (count-newlines in-stream
                                  :stop-position (1+ stream-pos))))))))
 
+(defun call-with-namespaced-output (out fun)
+  "Invoke FUN with a function for opening C++ namespaces.  The function takes
+care to write namespaces to OUT without redundantly opening already open
+namespaces."
+  (declare (type stream out))
+  (declare (type (function (function)) fun))
+  (let (open-namespaces)
+    (funcall fun (lambda (namespaces)
+                   ;; Check if we need to open or close namespaces
+                   (loop for namespace in namespaces
+                      with unmatched = open-namespaces do
+                        (if (string= namespace (car unmatched))
+                            (setf unmatched (cdr unmatched))
+                            (progn
+                              (dolist (to-close unmatched)
+                                (declare (ignore to-close))
+                                (format out "~%}"))
+                              (format out "namespace ~A {~2%" namespace))))
+                   (setf open-namespaces namespaces)))
+    ;; Close remaining namespaces
+    (dolist (to-close open-namespaces)
+      (declare (ignore to-close))
+      (format out "~%}"))))
+
+(defmacro with-namespaced-output ((out open-namespace-fun) &body body)
+  "Use `CALL-WITH-NAMESPACED-OUTPUT' more conveniently by executing BODY in a
+context which binds OPEN-NAMESPACE-FUN function for opening namespaces."
+  (let ((open-namespace (gensym)))
+    `(call-with-namespaced-output
+      ,out
+      (lambda (,open-namespace)
+        (flet ((,open-namespace-fun (namespaces)
+                 (funcall ,open-namespace namespaces)))
+          ,@body)))))
+
 (defun generate-capnp (cpp-types &key capnp-file capnp-id cpp-out lcp-file)
   "Generate Cap'n Proto serialization code for given CPP-TYPES.  The schema
 is written to CAPNP-FILE using the CAPNP-ID.  The C++ serialization code is
@@ -1568,31 +1724,17 @@ code generation."
   ;; Now generate the save/load C++ code in the cpp file.
   (write-line "// Autogenerated Cap'n Proto serialization code" cpp-out)
   (write-line "#include \"utils/serialization.hpp\"" cpp-out)
-  (let (open-namespaces)
+  (with-namespaced-output (cpp-out open-namespace)
     (dolist (cpp-class (remove-if (lambda (cpp-type) (not (typep cpp-type 'cpp-class))) cpp-types))
-      ;; Check if we need to open or close namespaces
-      (loop for namespace in (cpp-type-namespace cpp-class)
-         with unmatched = open-namespaces do
-           (if (string= namespace (car unmatched))
-               (setf unmatched (cdr unmatched))
-               (progn
-                 (dolist (to-close unmatched)
-                   (declare (ignore to-close))
-                   (format cpp-out "~%}"))
-                 (format cpp-out "namespace ~A {~2%" namespace))))
-      (setf open-namespaces (cpp-type-namespace cpp-class))
-      ;; Output the serialization code
+      (open-namespace (cpp-type-namespace cpp-class))
       (format cpp-out "// Serialize code for ~A~2%" (cpp-type-name cpp-class))
-      (let ((save-code (capnp-save-code cpp-class))
-            (construct-code (capnp-construct-code cpp-class))
+      ;; Top level functions
+      (write-line (capnp-save-function-definition cpp-class) cpp-out)
+      ;; Member functions
+      (let ((construct-code (capnp-construct-code cpp-class))
             (load-code (capnp-load-code cpp-class)))
-        (when save-code (write-line save-code cpp-out))
         (when construct-code (write-line construct-code cpp-out))
-        (when load-code (write-line load-code cpp-out))))
-    ;; Close remaining namespaces
-    (dolist (to-close open-namespaces)
-      (declare (ignore to-close))
-      (format cpp-out "~%}"))))
+        (when load-code (write-line load-code cpp-out))))))
 
 (defun process-file (lcp-file &key capnp-id capnp-declaration)
   "Process a LCP-FILE and write the output to .hpp file in the same directory.
@@ -1630,10 +1772,19 @@ file."
           (write-line (cpp-code res) out)))
       (when *cpp-namespaces*
         (error "Unclosed namespaces: ~A" (reverse *cpp-namespaces*)))
-      ;; If we have a capnp-id, generate the schema
+      ;; If we have a capnp-id, generate the schema and serialization code
       (let ((types-for-capnp (when capnp-id
                                (append (remove-if (complement #'cpp-class-capnp-opts) *cpp-classes*)
                                        (remove-if (complement #'cpp-enum-capnp-schema) *cpp-enums*)))))
+        ;; Append top-level declarations for Cap'n Proto serialization
+        (with-open-file (out hpp-file :direction :output :if-exists :append)
+          (terpri out)
+          (write-line "// Cap'n Proto serialization declarations" out)
+          (with-namespaced-output (out open-namespace)
+            (dolist (type-for-capnp types-for-capnp)
+              (when (typep type-for-capnp 'cpp-class)
+                (open-namespace (cpp-type-namespace type-for-capnp))
+                (format out "~A;~%" (capnp-save-function-declaration type-for-capnp))))))
         ;; When we have either capnp or C++ code for the .cpp file, generate the .cpp file
         (when (or *cpp-impl* types-for-capnp)
           (with-open-file (out cpp-file :direction :output :if-exists :supersede)
@@ -1642,27 +1793,11 @@ file."
                     (file-namestring lcp-file))
             (format out "#include \"~A\"~2%" (file-namestring hpp-file))
             ;; First output the C++ code from the user
-            (let (open-namespaces)
+            (with-namespaced-output (out open-namespace)
               (dolist (cpp *cpp-impl*)
                 (destructuring-bind (namespaces . code) cpp
-                  ;; Check if we need to open or close namespaces
-                  (loop for namespace in namespaces
-                     with unmatched = open-namespaces do
-                       (if (string= namespace (car unmatched))
-                           (setf unmatched (cdr unmatched))
-                           (progn
-                             (dolist (to-close unmatched)
-                               (declare (ignore to-close))
-                               (format out "~%}"))
-                             (format out "namespace ~A {~2%" namespace))))
-                  (setf open-namespaces namespaces)
-                  ;; Output the code
-                  (write-line (cpp-code code) out)))
-              ;; Close remaining namespaces
-              (dolist (to-close open-namespaces)
-                (declare (ignore to-close))
-                (format out "~%}")))
-            ;; Now output the capnp code
+                  (open-namespace namespaces)
+                  (write-line (cpp-code code) out))))
             (when types-for-capnp
               (generate-capnp types-for-capnp :capnp-file capnp-file :capnp-id capnp-id
                               :cpp-out out :lcp-file lcp-file))))))))

src/query/common.cpp

@@ -294,6 +294,16 @@ void TypedValueVectorCompare::Save(
   }
 }
 
+void Save(const TypedValueVectorCompare &comparator,
+          capnp::TypedValueVectorCompare::Builder *builder) {
+  auto ordering_builder = builder->initOrdering(comparator.ordering().size());
+  for (size_t i = 0; i < comparator.ordering().size(); ++i) {
+    ordering_builder.set(i, comparator.ordering()[i] == Ordering::ASC
+                                ? capnp::Ordering::ASC
+                                : capnp::Ordering::DESC);
+  }
+}
+
 void TypedValueVectorCompare::Load(
     const capnp::TypedValueVectorCompare::Reader &reader) {
   std::vector<Ordering> ordering;

src/query/common.hpp

@@ -53,6 +53,9 @@ class TypedValueVectorCompare final {
   std::vector<Ordering> ordering_;
 };
 
+void Save(const TypedValueVectorCompare &comparator,
+          capnp::TypedValueVectorCompare::Builder *builder);
+
 /// Switch the given [Vertex/Edge]Accessor to the desired state.
 template <class TAccessor>
 void SwitchAccessor(TAccessor &accessor, GraphView graph_view);

src/query/frontend/ast/ast.cpp

@@ -299,7 +299,7 @@ void MapLiteral::Save(capnp::BaseLiteral::Builder *base_literal_builder,
     auto key_builder = entry_builder.getKey();
     key_builder.setFirst(entry.first.first);
     auto storage_property_builder = key_builder.getSecond();
-    entry.first.second.Save(&storage_property_builder);
+    storage::Save(entry.first.second, &storage_property_builder);
     auto value_builder = entry_builder.getValue();
     if (entry.second) entry.second->Save(&value_builder, saved_uids);
     ++i;
@@ -967,7 +967,7 @@ void LabelsTest::Save(capnp::LabelsTest::Builder *builder,
   auto common_builders = builder->initLabels(labels_.size());
   for (size_t i = 0; i < labels_.size(); ++i) {
     auto common_builder = common_builders[i];
-    labels_[i].Save(&common_builder);
+    storage::Save(labels_[i], &common_builder);
   }
 }
 
@@ -1027,7 +1027,7 @@ void PropertyLookup::Save(capnp::PropertyLookup::Builder *builder,
   }
   builder->setPropertyName(property_name_);
   auto storage_property_builder = builder->initProperty();
-  property_.Save(&storage_property_builder);
+  storage::Save(property_, &storage_property_builder);
 }
 
 void PropertyLookup::Load(const capnp::Tree::Reader &base_reader,
@@ -1347,9 +1347,9 @@ void CreateIndex::Save(capnp::Clause::Builder *builder,
 void CreateIndex::Save(capnp::CreateIndex::Builder *builder,
                        std::vector<int> *saved_uids) {
   auto label_builder = builder->getLabel();
-  label_.Save(&label_builder);
+  storage::Save(label_, &label_builder);
   auto property_builder = builder->getProperty();
-  property_.Save(&property_builder);
+  storage::Save(property_, &property_builder);
 }
 
 CreateIndex *CreateIndex::Construct(const capnp::CreateIndex::Reader &reader,
@@ -1725,7 +1725,7 @@ void RemoveLabels::Save(capnp::RemoveLabels::Builder *builder,
   auto common_builders = builder->initLabels(labels_.size());
   for (size_t i = 0; i < labels_.size(); ++i) {
     auto common_builder = common_builders[i];
-    labels_[i].Save(&common_builder);
+    storage::Save(labels_[i], &common_builder);
   }
 }
 
@@ -1893,7 +1893,7 @@ void SetLabels::Save(capnp::SetLabels::Builder *builder,
   auto common_builders = builder->initLabels(labels_.size());
   for (size_t i = 0; i < labels_.size(); ++i) {
     auto common_builder = common_builders[i];
-    labels_[i].Save(&common_builder);
+    storage::Save(labels_[i], &common_builder);
   }
 }
 
@@ -2285,7 +2285,7 @@ void CypherUnion::Save(capnp::CypherUnion::Builder *builder,
   auto symbol_builders = builder->initUnionSymbols(union_symbols_.size());
   for (size_t i = 0; i < union_symbols_.size(); ++i) {
     auto symbol_builder = symbol_builders[i];
-    union_symbols_[i].Save(&symbol_builder);
+    query::Save(union_symbols_[i], &symbol_builder);
   }
 }
 
@@ -2460,7 +2460,7 @@ void NodeAtom::Save(capnp::NodeAtom::Builder *builder,
     auto key_builder = entry_builder.getKey();
     key_builder.setFirst(entry.first.first);
     auto storage_property_builder = key_builder.getSecond();
-    entry.first.second.Save(&storage_property_builder);
+    storage::Save(entry.first.second, &storage_property_builder);
     auto value_builder = entry_builder.getValue();
     if (entry.second) entry.second->Save(&value_builder, saved_uids);
     ++i;
@@ -2468,7 +2468,7 @@ void NodeAtom::Save(capnp::NodeAtom::Builder *builder,
   auto common_builders = builder->initLabels(labels_.size());
   for (size_t i = 0; i < labels_.size(); ++i) {
     auto common_builder = common_builders[i];
-    labels_[i].Save(&common_builder);
+    storage::Save(labels_[i], &common_builder);
   }
 }
 
@@ -2557,7 +2557,7 @@ void EdgeAtom::Save(capnp::EdgeAtom::Builder *builder,
   auto common_builders = builder->initEdgeTypes(edge_types_.size());
   for (size_t i = 0; i < edge_types_.size(); ++i) {
     auto common_builder = common_builders[i];
-    edge_types_[i].Save(&common_builder);
+    storage::Save(edge_types_[i], &common_builder);
   }
 
   ::capnp::List<capnp::EdgeAtom::Entry>::Builder map_builder =
@@ -2568,7 +2568,7 @@ void EdgeAtom::Save(capnp::EdgeAtom::Builder *builder,
     auto key_builder = entry_builder.getKey();
     key_builder.setFirst(entry.first.first);
     auto storage_property_builder = key_builder.getSecond();
-    entry.first.second.Save(&storage_property_builder);
+    storage::Save(entry.first.second, &storage_property_builder);
     auto value_builder = entry_builder.getValue();
     if (entry.second) entry.second->Save(&value_builder, saved_uids);
     ++i;

src/query/frontend/semantic/symbol.hpp

@@ -39,33 +39,6 @@ class Symbol {
   bool user_declared() const { return user_declared_; }
   int token_position() const { return token_position_; }
 
-  void Save(capnp::Symbol::Builder *builder) const {
-    builder->setName(name_);
-    builder->setPosition(position_);
-    builder->setUserDeclared(user_declared_);
-    builder->setTokenPosition(token_position_);
-    switch (type_) {
-      case Type::Any:
-        builder->setType(capnp::Symbol::Type::ANY);
-        break;
-      case Type::Edge:
-        builder->setType(capnp::Symbol::Type::EDGE);
-        break;
-      case Type::EdgeList:
-        builder->setType(capnp::Symbol::Type::EDGE_LIST);
-        break;
-      case Type::Number:
-        builder->setType(capnp::Symbol::Type::NUMBER);
-        break;
-      case Type::Path:
-        builder->setType(capnp::Symbol::Type::PATH);
-        break;
-      case Type::Vertex:
-        builder->setType(capnp::Symbol::Type::VERTEX);
-        break;
-    }
-  }
-
   void Load(const capnp::Symbol::Reader &reader) {
     name_ = reader.getName();
     position_ = reader.getPosition();
@@ -101,6 +74,33 @@ class Symbol {
   int token_position_ = -1;
 };
 
+inline void Save(const Symbol &symbol, capnp::Symbol::Builder *builder) {
+  builder->setName(symbol.name());
+  builder->setPosition(symbol.position());
+  builder->setUserDeclared(symbol.user_declared());
+  builder->setTokenPosition(symbol.token_position());
+  switch (symbol.type()) {
+    case Symbol::Type::Any:
+      builder->setType(capnp::Symbol::Type::ANY);
+      break;
+    case Symbol::Type::Edge:
+      builder->setType(capnp::Symbol::Type::EDGE);
+      break;
+    case Symbol::Type::EdgeList:
+      builder->setType(capnp::Symbol::Type::EDGE_LIST);
+      break;
+    case Symbol::Type::Number:
+      builder->setType(capnp::Symbol::Type::NUMBER);
+      break;
+    case Symbol::Type::Path:
+      builder->setType(capnp::Symbol::Type::PATH);
+      break;
+    case Symbol::Type::Vertex:
+      builder->setType(capnp::Symbol::Type::VERTEX);
+      break;
+  }
+}
+
 }  // namespace query
 
 namespace std {

src/query/frontend/semantic/symbol_table.hpp

@@ -28,18 +28,6 @@ class SymbolTable final {
 
   const auto &table() const { return table_; }
 
-  void Save(capnp::SymbolTable::Builder *builder) const {
-    builder->setPosition(position_);
-    auto list_builder = builder->initTable(table_.size());
-    size_t i = 0;
-    for (const auto &entry : table_) {
-      auto entry_builder = list_builder[i++];
-      entry_builder.setKey(entry.first);
-      auto sym_builder = entry_builder.initVal();
-      entry.second.Save(&sym_builder);
-    }
-  }
-
   void Load(const capnp::SymbolTable::Reader &reader) {
     position_ = reader.getPosition();
     table_.clear();
@@ -56,4 +44,17 @@ class SymbolTable final {
   std::map<int, Symbol> table_;
 };
 
+inline void Save(const SymbolTable &symbol_table,
+                 capnp::SymbolTable::Builder *builder) {
+  builder->setPosition(symbol_table.max_position());
+  auto list_builder = builder->initTable(symbol_table.table().size());
+  size_t i = 0;
+  for (const auto &entry : symbol_table.table()) {
+    auto entry_builder = list_builder[i++];
+    entry_builder.setKey(entry.first);
+    auto sym_builder = entry_builder.initVal();
+    Save(entry.second, &sym_builder);
+  }
+}
+
 }  // namespace query

src/query/plan/distributed.cpp

@@ -23,7 +23,7 @@ std::pair<std::unique_ptr<LogicalOperator>, AstStorage> Clone(
   {
     auto builder = message.initRoot<query::plan::capnp::LogicalOperator>();
     LogicalOperator::SaveHelper helper;
-    original_plan.Save(&builder, &helper);
+    Save(original_plan, &builder, &helper);
   }
   auto reader = message.getRoot<query::plan::capnp::LogicalOperator>();
   auto plan_copy = LogicalOperator::Construct(reader);

src/query/plan/operator.lcp

@@ -239,17 +239,16 @@ can serve as inputs to others and thus a sequence of operations is formed.")
    };
    cpp<#)
   (:serialize :capnp :base t
-              :save-args '((helper "SaveHelper *"))
-              :load-args '((helper "LoadHelper *"))))
+              :save-args '((helper "LogicalOperator::SaveHelper *"))
+              :load-args '((helper "LogicalOperator::LoadHelper *"))))
 
-(defun save-ast-pointer (builder member)
-  (let ((member-getter (remove #\_ (string-capitalize member))))
-    #>cpp
-    if (${member}) {
-      auto ${member}_builder = ${builder}->init${member-getter}();
-      ${member}->Save(&${member}_builder, &helper->saved_ast_uids);
-    }
-    cpp<#))
+(defun save-ast-pointer (builder member capnp-name)
+  #>cpp
+  if (${member}) {
+    auto ${capnp-name}_builder = ${builder}->init${capnp-name}();
+    ${member}->Save(&${capnp-name}_builder, &helper->saved_ast_uids);
+  }
+  cpp<#)
 
 (defun load-ast-pointer (ast-type)
   (lambda (reader member)
@@ -276,11 +275,11 @@ can serve as inputs to others and thus a sequence of operations is formed.")
                             return static_cast<~A>(helper->ast_storage.Load(reader, &helper->loaded_ast_uids));
                           }" ast-type)))
 
-(defun save-operator-pointer (builder member-name)
+(defun save-operator-pointer (builder member-name capnp-name)
   #>cpp
   utils::SaveSharedPtr<capnp::LogicalOperator, LogicalOperator>(${member-name}, &${builder},
                        [helper](auto *builder, const auto &val) {
-                         val.Save(builder, helper);
+                         Save(val, builder, helper);
                        }, &helper->saved_ops);
   cpp<#)
 
@@ -548,18 +547,19 @@ given label.
   (:private #>cpp ScanAllByLabel() {} cpp<#)
   (:serialize :capnp))
 
-(defun save-optional-bound (builder member)
+(defun save-optional-bound (builder member capnp-name)
   (let ((save-bound
          "[helper](auto *builder, const auto &bound) {
-           builder->setType(bound.type() == Bound::Type::INCLUSIVE ?
+           builder->setType(bound.type() == utils::BoundType::INCLUSIVE ?
                             ::utils::capnp::Bound<::query::capnp::Tree>::Type::INCLUSIVE :
                             ::utils::capnp::Bound<::query::capnp::Tree>::Type::EXCLUSIVE);
            auto value_builder = builder->initValue();
            bound.value()->Save(&value_builder, &helper->saved_ast_uids);
          }"))
-    (funcall (lcp:capnp-save-optional "::utils::capnp::Bound<::query::capnp::Tree>" "Bound"
+    (funcall (lcp:capnp-save-optional "::utils::capnp::Bound<::query::capnp::Tree>"
+                                      "utils::Bound<Expression *>"
                                       save-bound)
-             builder member)))
+             builder member capnp-name)))
 
 (defun load-optional-bound (reader member)
   (let ((load-bound
@@ -675,7 +675,7 @@ property value.
                :capnp-load (lcp:capnp-load-vector "::storage::capnp::Common" "storage::EdgeType"))
    ;; the input op and the symbol under which the op's result
    ;; can be found in the frame
-   (input "std::shared_ptr<LogicalOperator>" :scope :protected
+   (input "std::shared_ptr<LogicalOperator>" :scope :protected :reader t
           :capnp-save #'save-operator-pointer
           :capnp-load #'load-operator-pointer)
    (input-symbol "Symbol" :reader t :scope :protected)
@@ -836,10 +836,10 @@ pulled.")
    (filter-lambda "Lambda" :reader t)
    (weight-lambda "std::experimental::optional<Lambda>" :reader t
                   :capnp-save (lcp:capnp-save-optional
-                               "capnp::ExpandVariable::Lambda" "Lambda"
-                               "[helper](auto *builder, const auto &val) { val.Save(builder, helper); }")
+                               "capnp::ExpandVariable::Lambda" "ExpandVariable::Lambda"
+                               "[helper](auto *builder, const auto &val) { Save(val, builder, helper); }")
                   :capnp-load (lcp:capnp-load-optional
-                               "capnp::ExpandVariable::Lambda" "Lambda"
+                               "capnp::ExpandVariable::Lambda" "ExpandVariable::Lambda"
                                "[helper](const auto &reader) { Lambda val; val.Load(reader, helper); return val; }"))
    (total-weight "std::experimental::optional<Symbol>" :reader t
                  :capnp-save (lcp:capnp-save-optional "::query::capnp::Symbol" "Symbol")
@@ -1068,7 +1068,7 @@ RETURN clause) the Produce's pull succeeds exactly once.")
                 :capnp-type "List(Ast.Tree)"
                 :capnp-save (save-ast-vector "Expression *")
                 :capnp-load (load-ast-vector "Expression *"))
-   (detach :bool :documentation
+   (detach :bool :reader t :documentation
            "if the vertex should be detached before deletion if not detached,
            and has connections, an error is raised ignored when deleting edges"))
   (:documentation
@@ -1165,7 +1165,7 @@ can be stored (a TypedValue that can be converted to PropertyValue).")
    (rhs "Expression *" :reader t
         :capnp-type "Ast.Tree" :capnp-init nil
         :capnp-save #'save-ast-pointer :capnp-load (load-ast-pointer "Expression *"))
-   (op "Op"))
+   (op "Op" :reader t))
   (:documentation
    "Logical op for setting the whole properties set on a vertex or an edge.
 
@@ -1231,7 +1231,7 @@ that the old props are discarded and replaced with new ones.")
           :capnp-save #'save-operator-pointer
           :capnp-load #'load-operator-pointer)
    (input-symbol "Symbol" :reader t)
-   (labels "std::vector<storage::Label>"
+   (labels "std::vector<storage::Label>" :reader t
      :capnp-save (lcp:capnp-save-vector "::storage::capnp::Common" "storage::Label")
      :capnp-load (lcp:capnp-load-vector "::storage::capnp::Common" "storage::Label")))
   (:documentation
@@ -1319,7 +1319,7 @@ It does NOT remove labels that are already set on that Vertex.")
           :capnp-save #'save-operator-pointer
           :capnp-load #'load-operator-pointer)
    (input-symbol "Symbol" :reader t)
-   (labels "std::vector<storage::Label>"
+   (labels "std::vector<storage::Label>" :reader t
      :capnp-save (lcp:capnp-save-vector "::storage::capnp::Common" "storage::Label")
      :capnp-load (lcp:capnp-load-vector "::storage::capnp::Common" "storage::Label")))
   (:documentation
@@ -1513,8 +1513,8 @@ cpp<#
           :capnp-load #'load-operator-pointer)
    (aggregations "std::vector<Element>" :reader t
                  :capnp-save (lcp:capnp-save-vector
-                              "capnp::Aggregate::Element" "Element"
-                              "[helper](auto *builder, const auto &val) { val.Save(builder, helper); }")
+                              "capnp::Aggregate::Element" "Aggregate::Element"
+                              "[helper](auto *builder, const auto &val) { Save(val, builder, helper); }")
                  :capnp-load (lcp:capnp-load-vector
                               "capnp::Aggregate::Element" "Element"
                               "[helper](const auto &reader) { Element val; val.Load(reader, helper); return val; }"))
@@ -2002,10 +2002,10 @@ and returns true, once.")
    (privileges "std::vector<AuthQuery::Privilege>" :reader t
                :capnp-type "List(Ast.AuthQuery.Privilege)"
                :capnp-save
-                 (lambda (builder member-name)
+                 (lambda (builder member capnp-name)
                    #>cpp
-                   for (size_t i = 0; i < ${member-name}.size(); ++i) {
-                    switch (privileges_[i]) {
+                   for (size_t i = 0; i < ${member}.size(); ++i) {
+                    switch (${member}[i]) {
                       case AuthQuery::Privilege::CREATE:
                         ${builder}.set(i, query::capnp::AuthQuery::Privilege::CREATE);
                         break;
@@ -2106,7 +2106,7 @@ and returns true, once.")
    (input-expression "Expression *" :reader t
                      :capnp-type "Ast.Tree" :capnp-init nil
                      :capnp-save #'save-ast-pointer :capnp-load (load-ast-pointer "Expression *"))
-   (output-symbol "Symbol"))
+   (output-symbol "Symbol" :reader t))
   (:documentation
    "Takes a list TypedValue as it's input and yields each
 element as it's output.

src/storage/address.hpp

@@ -89,10 +89,6 @@ class Address {
     return storage_ == other.storage_;
   }
 
-  void Save(capnp::Address::Builder *builder) const {
-    builder->setStorage(storage_);
-  }
-
   void Load(const capnp::Address::Reader &reader) {
     storage_ = reader.getStorage();
   }
@@ -101,4 +97,14 @@ class Address {
   StorageT storage_{0};
 };
 
+template <typename TLocalObj>
+void Save(const Address<TLocalObj> &address, capnp::Address::Builder *builder) {
+  builder->setStorage(address.raw());
+}
+
+template <typename TLocalObj>
+Address<TLocalObj> Load(const capnp::Address::Reader &reader) {
+  return Address<TLocalObj>(reader.getStorage());
+}
+
 }  // namespace storage

src/storage/concurrent_id_mapper_master.cpp

@@ -19,14 +19,14 @@ void RegisterRpc(MasterConcurrentIdMapper<TId> &mapper,
           type##IdReq req;                                          \
           req.Load(req_reader);                                     \
           type##IdRes res(mapper.value_to_id(req.member));          \
-          res.Save(res_builder);                                    \
+          Save(res, res_builder);                                   \
         });                                                         \
     rpc_server.Register<Id##type##Rpc>(                             \
         [&mapper](const auto &req_reader, auto *res_builder) {      \
           Id##type##Req req;                                        \
           req.Load(req_reader);                                     \
           Id##type##Res res(mapper.id_to_value(req.member));        \
-          res.Save(res_builder);                                    \
+          Save(res, res_builder);                                   \
         });                                                         \
   }
 

src/storage/serialization.cpp

@@ -95,7 +95,7 @@ void SaveProperties(const PropertyValueStore &properties,
   for (const auto &kv : properties) {
     auto kv_builder = props_builder[i++];
     auto id_builder = kv_builder.initId();
-    kv.first.Save(&id_builder);
+    Save(kv.first, &id_builder);
     auto value_builder = kv_builder.initValue();
     SaveCapnpPropertyValue(kv.second, &value_builder);
   }

src/storage/types.hpp

@@ -54,14 +54,12 @@ class Common : public utils::TotalOrdering<TSpecificType> {
     size_t operator()(const TSpecificType &t) const { return hash(t.id_); }
   };
 
-  void Save(capnp::Common::Builder *builder) const {
-    builder->setStorage(id_);
-  }
-
   void Load(const capnp::Common::Reader &reader) {
     id_ = reader.getStorage();
   }
 
+  auto Raw() const { return id_; }
+
  protected:
   ~Common() {}
 
@@ -72,6 +70,11 @@ class Common : public utils::TotalOrdering<TSpecificType> {
   IdT id_{0};
 };
 
+template <class Type>
+void Save(const Common<Type> &common, capnp::Common::Builder *builder) {
+  builder->setStorage(common.Raw());
+}
+
 class Label final : public Common<Label> {
   using Common::Common;
 };

src/transactions/distributed/engine_master.cpp

@@ -19,13 +19,13 @@ EngineMaster::EngineMaster(communication::rpc::Server *server,
       [this](const auto &req_reader, auto *res_builder) {
         auto tx = this->Begin();
         BeginRes res(TxAndSnapshot{tx->id_, tx->snapshot()});
-        res.Save(res_builder);
+        Save(res, res_builder);
       });
 
   server_->Register<AdvanceRpc>(
       [this](const auto &req_reader, auto *res_builder) {
         AdvanceRes res(this->Advance(req_reader.getMember()));
-        res.Save(res_builder);
+        Save(res, res_builder);
       });
 
   server_->Register<CommitRpc>(
@@ -44,7 +44,7 @@ EngineMaster::EngineMaster(communication::rpc::Server *server,
         // transaction that's done, and that there are no race conditions here.
         SnapshotRes res(
             this->RunningTransaction(req_reader.getMember())->snapshot());
-        res.Save(res_builder);
+        Save(res, res_builder);
       });
 
   server_->Register<CommandRpc>(
@@ -52,25 +52,25 @@ EngineMaster::EngineMaster(communication::rpc::Server *server,
         // It is guaranteed that the Worker will not be requesting this for a
         // transaction that's done, and that there are no race conditions here.
         CommandRes res(this->RunningTransaction(req_reader.getMember())->cid());
-        res.Save(res_builder);
+        Save(res, res_builder);
       });
 
   server_->Register<GcSnapshotRpc>(
       [this](const auto &req_reader, auto *res_builder) {
         GcSnapshotRes res(this->GlobalGcSnapshot());
-        res.Save(res_builder);
+        Save(res, res_builder);
       });
 
   server_->Register<ClogInfoRpc>(
       [this](const auto &req_reader, auto *res_builder) {
         ClogInfoRes res(this->Info(req_reader.getMember()));
-        res.Save(res_builder);
+        Save(res, res_builder);
       });
 
   server_->Register<ActiveTransactionsRpc>(
       [this](const auto &req_reader, auto *res_builder) {
         ActiveTransactionsRes res(this->GlobalActiveTransactions());
-        res.Save(res_builder);
+        Save(res, res_builder);
       });
 
   server_->Register<EnsureNextIdGreaterRpc>(
@@ -81,7 +81,7 @@ EngineMaster::EngineMaster(communication::rpc::Server *server,
   server_->Register<GlobalLastRpc>(
       [this](const auto &req_reader, auto *res_builder) {
         GlobalLastRes res(this->GlobalLast());
-        res.Save(res_builder);
+        Save(res, res_builder);
       });
 }
 

src/transactions/distributed/serialization.lcp

@@ -1,15 +1,20 @@
 ;; This file doesn't need to be preprocessed. It only holds helper functions.
 
-(defun save-commitlog-info (builder member) #>cpp ${builder}->setMember(${member}); cpp<#)
+(defun save-commitlog-info (builder member capnp-name)
+  #>cpp
+  ${builder}->set${capnp-name}(${member});
+  cpp<#)
 
-(defun load-commitlog-info (reader member) #>cpp ${member} = CommitLog::Info(${reader}.getMember()); cpp<#)
+(defun load-commitlog-info (reader member)
+  #>cpp
+  ${member} = CommitLog::Info(${reader}.getMember());
+  cpp<#)
 
-(defun save-snapshot (builder member)
-  (let ((capnp-member (remove #\_ (string-capitalize member))))
-    #>cpp
-    auto list_builder = builder->init${capnp-member}(${member}.transaction_ids().size());
-    utils::SaveVector(${member}.transaction_ids(), &list_builder);
-    cpp<#))
+(defun save-snapshot (builder member capnp-name)
+  #>cpp
+  auto list_builder = builder->init${capnp-name}(${member}.transaction_ids().size());
+  utils::SaveVector(${member}.transaction_ids(), &list_builder);
+  cpp<#)
 
 (defun load-snapshot (reader member)
   (let ((capnp-member (remove #\_ (string-capitalize member))))

tests/unit/query_planner.cpp

@@ -668,7 +668,7 @@ class Planner {
 void SavePlan(const LogicalOperator &plan, ::capnp::MessageBuilder *message) {
   auto builder = message->initRoot<query::plan::capnp::LogicalOperator>();
   LogicalOperator::SaveHelper helper;
-  plan.Save(&builder, &helper);
+  Save(plan, &builder, &helper);
 }
 
 auto LoadPlan(const ::query::plan::capnp::LogicalOperator::Reader &reader) {

tests/unit/rpc.cpp

@@ -22,12 +22,6 @@ struct SumReq {
   int x;
   int y;
 
-  void Save(::capnp::AnyPointer::Builder *builder) const {
-    auto list_builder = builder->initAs<::capnp::List<int>>(2);
-    list_builder.set(0, x);
-    list_builder.set(1, y);
-  }
-
   void Load(const ::capnp::AnyPointer::Reader &reader) {
     auto list_reader = reader.getAs<::capnp::List<int>>();
     x = list_reader[0];
@@ -35,6 +29,12 @@ struct SumReq {
   }
 };
 
+void Save(const SumReq &sum, ::capnp::AnyPointer::Builder *builder) {
+  auto list_builder = builder->initAs<::capnp::List<int>>(2);
+  list_builder.set(0, sum.x);
+  list_builder.set(1, sum.y);
+}
+
 const MessageType SumReq::TypeInfo{0, "SumReq"};
 
 struct SumRes {
@@ -46,17 +46,17 @@ struct SumRes {
 
   int sum;
 
-  void Save(::capnp::AnyPointer::Builder *builder) const {
-    auto list_builder = builder->initAs<::capnp::List<int>>(1);
-    list_builder.set(0, sum);
-  }
-
   void Load(const ::capnp::AnyPointer::Reader &reader) {
     auto list_reader = reader.getAs<::capnp::List<int>>();
     sum = list_reader[0];
   }
 };
 
+void Save(const SumRes &res, ::capnp::AnyPointer::Builder *builder) {
+  auto list_builder = builder->initAs<::capnp::List<int>>(1);
+  list_builder.set(0, res.sum);
+}
+
 const MessageType SumRes::TypeInfo{1, "SumRes"};
 
 using Sum = RequestResponse<SumReq, SumRes>;
@@ -70,17 +70,17 @@ struct EchoMessage {
 
   std::string data;
 
-  void Save(::capnp::AnyPointer::Builder *builder) const {
-    auto list_builder = builder->initAs<::capnp::List<::capnp::Text>>(1);
-    list_builder.set(0, data);
-  }
-
   void Load(const ::capnp::AnyPointer::Reader &reader) {
     auto list_reader = reader.getAs<::capnp::List<::capnp::Text>>();
     data = list_reader[0];
   }
 };
 
+void Save(const EchoMessage &echo, ::capnp::AnyPointer::Builder *builder) {
+  auto list_builder = builder->initAs<::capnp::List<::capnp::Text>>(1);
+  list_builder.set(0, echo.data);
+}
+
 const MessageType EchoMessage::TypeInfo{2, "EchoMessage"};
 
 using Echo = RequestResponse<EchoMessage, EchoMessage>;
@@ -91,7 +91,7 @@ TEST(Rpc, Call) {
     SumReq req;
     req.Load(req_reader);
     SumRes res(req.x + req.y);
-    res.Save(res_builder);
+    Save(res, res_builder);
   });
   std::this_thread::sleep_for(100ms);
 
@@ -110,7 +110,7 @@ TEST(Rpc, Abort) {
     req.Load(req_reader);
     std::this_thread::sleep_for(500ms);
     SumRes res(req.x + req.y);
-    res.Save(res_builder);
+    Save(res, res_builder);
   });
   std::this_thread::sleep_for(100ms);
 
@@ -140,7 +140,7 @@ TEST(Rpc, ClientPool) {
     req.Load(req_reader);
     std::this_thread::sleep_for(100ms);
     SumRes res(req.x + req.y);
-    res.Save(res_builder);
+    Save(res, res_builder);
   });
   std::this_thread::sleep_for(100ms);
 
@@ -192,7 +192,7 @@ TEST(Rpc, LargeMessage) {
   server.Register<Echo>([](const auto &req_reader, auto *res_builder) {
     EchoMessage res;
     res.Load(req_reader);
-    res.Save(res_builder);
+    Save(res, res_builder);
   });
   std::this_thread::sleep_for(100ms);
 

tools/src/mg_statsd/main.cpp

@@ -49,7 +49,7 @@ int main(int argc, char *argv[]) {
         std::string data = GraphiteFormat(req);
         graphite_socket.Write(data);
         stats::StatsRes res;
-        res.Save(res_builder);
+        Save(res, res_builder);
       });
 
   server.Register<stats::BatchStatsRpc>(
@@ -64,7 +64,7 @@ int main(int argc, char *argv[]) {
           graphite_socket.Write(data, i + 1 < req.requests.size());
         }
         stats::BatchStatsRes res;
-        res.Save(res_builder);
+        Save(res, res_builder);
       });
 
   std::this_thread::sleep_until(std::chrono::system_clock::time_point::max());