backingstore: encode the dtor function in CFallible's type

Summary:
There is no need for two indirect calls (std::function, function
pointer) when the deleter for each type is known at compile
time. Therefore, encode the deleter function in the CFallible type
itself.

Reviewed By: xavierd

Differential Revision: D40954237

fbshipit-source-id: a7d355715101c4b2bffb3516490aaadcb79112c1

eden/scm/lib/backingstore/c_api/HgNativeBackingStore.cpp

@@ -119,9 +119,8 @@ void getTreeBatchCallback(
 HgNativeBackingStore::HgNativeBackingStore(
     std::string_view repository,
     const BackingStoreOptions& options) {
-  CFallible<BackingStore> store(
-      sapling_backingstore_new(repository, &options),
-      sapling_backingstore_free);
+  CFallible<BackingStore, sapling_backingstore_free> store{
+      sapling_backingstore_new(repository, &options)};
 
   if (store.isError()) {
     throw std::runtime_error(store.getError());
@@ -136,9 +135,8 @@ std::unique_ptr<folly::IOBuf> HgNativeBackingStore::getBlob(
     bool local) {
   XLOG(DBG7) << "Importing blob name=" << name.data()
              << " node=" << folly::hexlify(node) << " from hgcache";
-  CFallible<CBytes> result(
-      sapling_backingstore_get_blob(store_.get(), name, node, local),
-      sapling_cbytes_free);
+  CFallible<CBytes, sapling_cbytes_free> result{
+      sapling_backingstore_get_blob(store_.get(), name, node, local)};
 
   if (result.isError()) {
     XLOG(DBG5) << "Error while getting blob name=" << name.data()
@@ -155,9 +153,8 @@ std::shared_ptr<FileAuxData> HgNativeBackingStore::getBlobMetadata(
     bool local) {
   XLOG(DBG7) << "Importing blob metadata"
              << " node=" << folly::hexlify(node) << " from hgcache";
-  CFallible<FileAuxData> result(
-      sapling_backingstore_get_file_aux(store_.get(), node, local),
-      sapling_file_aux_free);
+  CFallible<FileAuxData, sapling_file_aux_free> result{
+      sapling_backingstore_get_file_aux(store_.get(), node, local)};
 
   if (result.isError()) {
     XLOG(DBG5) << "Error while getting blob metadata"
@@ -205,8 +202,8 @@ void HgNativeBackingStore::getBlobMetadataBatch(
       count,
       local,
       [resolve, requests, count](size_t index, CFallibleBase raw_result) {
-        CFallible<FileAuxData> result(
-            std::move(raw_result), sapling_file_aux_free);
+        CFallible<FileAuxData, sapling_file_aux_free> result{
+            std::move(raw_result)};
 
         if (result.isError()) {
           // TODO: It would be nice if we can differentiate not found error with
@@ -270,7 +267,7 @@ void HgNativeBackingStore::getBlobBatch(
       count,
       local,
       [resolve, requests, count](size_t index, CFallibleBase raw_result) {
-        CFallible<CBytes> result(std::move(raw_result), sapling_cbytes_free);
+        CFallible<CBytes, sapling_cbytes_free> result{std::move(raw_result)};
 
         if (result.isError()) {
           // TODO: It would be nice if we can differentiate not found error with
@@ -331,7 +328,7 @@ void HgNativeBackingStore::getTreeBatch(
       count,
       local,
       [resolve, requests, count](size_t index, CFallibleBase raw_result) {
-        CFallible<Tree> result(std::move(raw_result), sapling_tree_free);
+        CFallible<Tree, sapling_tree_free> result{std::move(raw_result)};
 
         if (result.isError()) {
           // TODO: It would be nice if we can differentiate not found error with
@@ -364,9 +361,8 @@ std::shared_ptr<Tree> HgNativeBackingStore::getTree(
   XLOG(DBG7) << "Importing tree node=" << folly::hexlify(node)
              << " from hgcache";
 
-  CFallible<Tree> manifest(
-      sapling_backingstore_get_tree(store_.get(), node, local),
-      sapling_tree_free);
+  CFallible<Tree, sapling_tree_free> manifest{
+      sapling_backingstore_get_tree(store_.get(), node, local)};
 
   if (manifest.isError()) {
     XLOG(DBG5) << "Error while getting tree node=" << folly::hexlify(node)

eden/scm/lib/backingstore/c_api/HgNativeBackingStore.h

@@ -69,10 +69,9 @@ class HgNativeBackingStore {
   void flush();
 
  private:
-  std::unique_ptr<
+  sapling::CFallible<
       sapling::BackingStore,
-      std::function<void(sapling::BackingStore*)>>
-      store_;
+      sapling::sapling_backingstore_free>::Ptr store_;
 };
 
 } // namespace facebook::eden

eden/scm/lib/backingstore/c_api/RustBackingStore.h

@@ -7,7 +7,7 @@
  * This file is generated with cbindgen. Please run `./tools/cbindgen.sh` to
  * update this file.
  *
- * @generated SignedSource<<309b1270697ee5617db7d9451c3f6aa8>>
+ * @generated SignedSource<<c37bbeca9e8fe2789b4bbbe60517606d>>
  *
  */
 
@@ -17,8 +17,8 @@
 
 #pragma once
 
+#include <stdint.h>
 #include <memory>
-#include <functional>
 #include <string_view>
 #include <folly/Range.h>
 
@@ -173,18 +173,23 @@ namespace sapling {
 // struct. This is the only way to use the returned `CFallibleBase` from
 // Rust, and the user must provide a `Deleter` to correctly free the pointer
 // returned from Rust.
-template <typename T, typename Deleter = std::function<void(T*)>>
+template <typename T, void(*dtor)(T*)>
 class CFallible {
 public:
-  CFallible(CFallibleBase&& base, Deleter deleter)
-      : ptr_(reinterpret_cast<T*>(base.value), deleter), error_(base.error) {}
+  struct Deleter {
+    void operator()(T* ptr) {
+      dtor(ptr);
+    }
+  };
+  using Ptr = std::unique_ptr<T, Deleter>;
+
+  explicit CFallible(CFallibleBase&& base)
+    : ptr_{reinterpret_cast<T*>(base.value)}, error_{base.error} {}
 
   ~CFallible() {
-    if (error_ != nullptr) {
+    if (error_) {
       sapling_cfallible_free_error(error_);
     }
-
-    unwrap();
   }
 
   bool isError() const {
@@ -199,12 +204,12 @@ public:
     return ptr_.get();
   }
 
-  std::unique_ptr<T, Deleter> unwrap() {
+  Ptr unwrap() {
     return std::move(ptr_);
   }
 
 private:
-  std::unique_ptr<T, std::function<void(T*)>> ptr_;
+  Ptr ptr_;
   char* error_;
 };
 

eden/scm/lib/backingstore/cbindgen.toml

@@ -28,18 +28,23 @@ namespace sapling {
 // struct. This is the only way to use the returned `CFallibleBase` from
 // Rust, and the user must provide a `Deleter` to correctly free the pointer
 // returned from Rust.
-template <typename T, typename Deleter = std::function<void(T*)>>
+template <typename T, void(*dtor)(T*)>
 class CFallible {
 public:
-  CFallible(CFallibleBase&& base, Deleter deleter)
-      : ptr_(reinterpret_cast<T*>(base.value), deleter), error_(base.error) {}
+  struct Deleter {
+    void operator()(T* ptr) {
+      dtor(ptr);
+    }
+  };
+  using Ptr = std::unique_ptr<T, Deleter>;
+
+  explicit CFallible(CFallibleBase&& base)
+    : ptr_{reinterpret_cast<T*>(base.value)}, error_{base.error} {}
 
   ~CFallible() {
-    if (error_ != nullptr) {
+    if (error_) {
       sapling_cfallible_free_error(error_);
     }
-
-    unwrap();
   }
 
   bool isError() const {
@@ -54,12 +59,12 @@ public:
     return ptr_.get();
   }
 
-  std::unique_ptr<T, Deleter> unwrap() {
+  Ptr unwrap() {
     return std::move(ptr_);
   }
 
 private:
-  std::unique_ptr<T, std::function<void(T*)>> ptr_;
+  Ptr ptr_;
   char* error_;
 };
 
@@ -70,8 +75,8 @@ namespace = "sapling"
 pragma_once = true
 no_includes = true # avoid iostream
 sys_includes = [
+    "stdint.h",
     "memory",
-    "functional",
     "string_view",
     "folly/Range.h",
 ]

eden/scm/lib/backingstore/test/CFallibleTest.cpp

@@ -15,8 +15,8 @@ namespace {
 using namespace sapling;
 
 TEST(CFallible, returns_ok) {
-  CFallible<uint8_t> result(
-      sapling_test_cfallible_ok(), sapling_test_cfallible_ok_free);
+  CFallible<uint8_t, sapling_test_cfallible_ok_free> result{
+      sapling_test_cfallible_ok()};
 
   uint8_t abc = *result.get();
 
@@ -26,14 +26,14 @@ TEST(CFallible, returns_ok) {
 
 // Test case for correct memory management when value is not used.
 TEST(CFallible, returns_ok_no_consume) {
-  CFallible<uint8_t> result(
-      sapling_test_cfallible_ok(), sapling_test_cfallible_ok_free);
+  CFallible<uint8_t, sapling_test_cfallible_ok_free> result{
+      sapling_test_cfallible_ok()};
   EXPECT_EQ(result.isError(), false);
 }
 
 TEST(CFallible, returns_err) {
-  CFallible<uint8_t> result(
-      sapling_test_cfallible_err(), sapling_test_cfallible_ok_free);
+  CFallible<uint8_t, sapling_test_cfallible_ok_free> result{
+      sapling_test_cfallible_err()};
 
   EXPECT_EQ(result.get(), nullptr);
   EXPECT_EQ(result.isError(), true);