Rework fine::Ok and fine::Error

This rework ensures that `fine::Ok` and `fine::Error` can correctly
convert when used in conjunction with container types like
`std::variant`.

The downside to this approach, as implemented, is the need to go through
2 factory functions, `fine::ok` and `fine::error`, to construct these.

Author: brodeuralexis

README.md

@@ -470,19 +470,28 @@ When it comes to NIFs, errors often indicate unexpected failures and
 raising an exception makes sense, however you may also want to handle
 certain errors gracefully by returning `:ok`/`:error` tuples, similarly
 to usual Elixir functions. Fine provides `Ok<Args...>` and `Error<Args...>`
-types for this purpose.
+types for this purpose, along with their respective factories `fine::ok` and
+`fine::error`:
 
 ```c++
-fine::Ok<>()
+fine::Ok<> example() {
+  return fine::ok();
+}
 // :ok
 
-fine::Ok<int64_t>(1)
+fine::Ok<int64_t> example() {
+  return fine::ok(1); // implicit conversion
+}
 // {:ok, 1}
 
-fine::Error<>()
+fine::Error<> example() {
+  return fine::error();
+}
 // :error
 
-fine::Error<std::string>("something went wrong")
+fine::Error<std::string> example() {
+  return fine::error("something went wrong"); // implicit conversion
+}
 // {:error, "something went wrong"}
 ```
 
@@ -492,10 +501,10 @@ a NIF may return:
 ```c++
 std::variant<fine::Ok<int64_t>, fine::Error<std::string>> find_meaning(ErlNifEnv *env) {
   if (...) {
-    return fine::Error<std::string>("something went wrong");
+    return fine::error("something went wrong");
   }
 
-  return fine::Ok<int64_t>(42);
+  return fine::ok(42);
 }
 ```
 

c_include/fine.hpp

@@ -142,28 +142,79 @@ class Term {
   ERL_NIF_TERM term;
 };
 
+namespace __private__ {
+template <class T> struct UnwrapRefwrapper {
+  using type = T;
+};
+
+template <class T> struct UnwrapRefwrapper<std::reference_wrapper<T>> {
+  using type = T &;
+};
+
+template <class T>
+using UnwrapDecayType =
+    typename UnwrapRefwrapper<typename std::decay<T>::type>::type;
+} // namespace __private__
+
 // Represents a `:ok` tagged tuple, useful as a NIF result.
 template <typename... Args> class Ok {
 public:
-  Ok(const Args &...items) : items(items...) {}
+  using Items = std::tuple<Args...>;
 
-private:
-  friend struct Encoder<Ok<Args...>>;
+  Ok(Items &&items) : m_items(std::forward<Items>(items)) {}
+
+  template <typename... UArgs>
+  Ok(const Ok<UArgs...> &other) : m_items{other.items()} {}
+
+  template <typename... UArgs>
+  Ok(Ok<UArgs...> &&other) : m_items{other.items()} {}
+
+  const Items &items() const & noexcept { return m_items; }
+
+  Items &items() & noexcept { return m_items; }
+
+  Items &&items() && noexcept { return m_items; }
 
-  std::tuple<Args...> items;
+private:
+  std::tuple<Args...> m_items;
 };
 
 // Represents a `:error` tagged tuple, useful as a NIF result.
 template <typename... Args> class Error {
 public:
-  Error(const Args &...items) : items(items...) {}
+  using Items = std::tuple<Args...>;
 
-private:
-  friend struct Encoder<Error<Args...>>;
+  Error(Items &&items) : m_items(std::forward<Items>(items)) {}
+
+  template <typename... UArgs>
+  Error(const Error<UArgs...> &other) : m_items{other.items()} {}
+
+  template <typename... UArgs>
+  Error(Error<UArgs...> &&other) : m_items{other.items()} {}
+
+  const Items &items() const & noexcept { return m_items; }
+
+  Items &items() & noexcept { return m_items; }
 
-  std::tuple<Args...> items;
+  Items &&items() && noexcept { return m_items; }
+
+private:
+  std::tuple<Args...> m_items;
 };
 
+template <typename... Args>
+inline static Ok<__private__::UnwrapDecayType<Args>...> ok(Args &&...args) {
+  return Ok<__private__::UnwrapDecayType<Args>...>{
+      std::make_tuple(std::forward<Args>(args)...)};
+}
+
+template <typename... Args>
+inline static Error<__private__::UnwrapDecayType<Args>...>
+error(Args &&...args) {
+  return Error<__private__::UnwrapDecayType<Args>...>{
+      std::make_tuple(std::forward<Args>(args)...)};
+}
+
 namespace __private__ {
 template <typename T> struct ResourceWrapper {
   T resource;
@@ -895,7 +946,7 @@ template <typename... Args> struct Encoder<Ok<Args...>> {
     auto tag = __private__::atoms::ok;
 
     if constexpr (sizeof...(Args) > 0) {
-      return fine::encode(env, std::tuple_cat(std::tuple(tag), ok.items));
+      return fine::encode(env, std::tuple_cat(std::tuple(tag), ok.items()));
     } else {
       return fine::encode(env, tag);
     }
@@ -907,7 +958,7 @@ template <typename... Args> struct Encoder<Error<Args...>> {
     auto tag = __private__::atoms::error;
 
     if constexpr (sizeof...(Args) > 0) {
-      return fine::encode(env, std::tuple_cat(std::tuple(tag), error.items));
+      return fine::encode(env, std::tuple_cat(std::tuple(tag), error.items()));
     } else {
       return fine::encode(env, tag);
     }

test/c_src/finest.cpp

@@ -76,6 +76,9 @@ struct ExError {
   static constexpr auto is_exception = true;
 };
 
+template <typename T, typename E>
+using Result = std::variant<fine::Ok<T>, fine::Error<E>>;
+
 int64_t add(ErlNifEnv *, int64_t x, int64_t y) { return x + y; }
 FINE_NIF(add, 0);
 
@@ -139,6 +142,37 @@ codec_tuple_int64_and_string(ErlNifEnv *,
 }
 FINE_NIF(codec_tuple_int64_and_string, 0);
 
+Result<std::string, int64_t> codec_result_string_int64_ok(ErlNifEnv *,
+                                                          std::string term) {
+  return fine::ok(term);
+}
+FINE_NIF(codec_result_string_int64_ok, 0);
+
+Result<std::string, int64_t> codec_result_string_int64_error(ErlNifEnv *,
+                                                             int64_t term) {
+  return fine::error(term);
+}
+FINE_NIF(codec_result_string_int64_error, 0);
+
+Result<std::string, int64_t>
+codec_result_string_int64_ok_conversion(ErlNifEnv *) {
+  return fine::ok("fine");
+}
+FINE_NIF(codec_result_string_int64_ok_conversion, 0);
+
+Result<std::string, int64_t>
+codec_result_string_int64_error_conversion(ErlNifEnv *) {
+  uint16_t result = 42;
+  return fine::error(result);
+}
+FINE_NIF(codec_result_string_int64_error_conversion, 0);
+
+std::variant<fine::Ok<int64_t, std::string>, fine::Error<>>
+codec_result_int64_string_void_ok_conversion(ErlNifEnv *) {
+  return fine::ok(static_cast<int32_t>(201702), "c++17");
+}
+FINE_NIF(codec_result_int64_string_void_ok_conversion, 0);
+
 std::vector<int64_t> codec_vector_int64(ErlNifEnv *,
                                         std::vector<int64_t> term) {
   return term;
@@ -163,19 +197,19 @@ FINE_NIF(codec_struct, 0);
 ExError codec_struct_exception(ErlNifEnv *, ExError term) { return term; }
 FINE_NIF(codec_struct_exception, 0);
 
-fine::Ok<> codec_ok_empty(ErlNifEnv *) { return fine::Ok(); }
+fine::Ok<> codec_ok_empty(ErlNifEnv *) { return fine::ok(); }
 FINE_NIF(codec_ok_empty, 0);
 
 fine::Ok<int64_t> codec_ok_int64(ErlNifEnv *, int64_t term) {
-  return fine::Ok(term);
+  return fine::ok(term);
 }
 FINE_NIF(codec_ok_int64, 0);
 
-fine::Error<> codec_error_empty(ErlNifEnv *) { return fine::Error(); }
+fine::Error<> codec_error_empty(ErlNifEnv *) { return fine::error(); }
 FINE_NIF(codec_error_empty, 0);
 
 fine::Error<std::string> codec_error_string(ErlNifEnv *, std::string term) {
-  return fine::Error(term);
+  return fine::error(term);
 }
 FINE_NIF(codec_error_string, 0);
 

test/lib/finest/nif.ex

@@ -28,6 +28,11 @@ defmodule Finest.NIF do
   def codec_optional_int64(_term), do: err!()
   def codec_variant_int64_or_string(_term), do: err!()
   def codec_tuple_int64_and_string(_term), do: err!()
+  def codec_result_string_int64_ok(_term), do: err!()
+  def codec_result_string_int64_error(_term), do: err!()
+  def codec_result_string_int64_ok_conversion(), do: err!()
+  def codec_result_string_int64_error_conversion(), do: err!()
+  def codec_result_int64_string_void_ok_conversion(), do: err!()
   def codec_vector_int64(_term), do: err!()
   def codec_map_atom_int64(_term), do: err!()
   def codec_resource(_term), do: err!()

test/test/finest_test.exs

@@ -217,6 +217,14 @@ defmodule FinestTest do
       assert NIF.codec_error_empty() == :error
       assert NIF.codec_error_string("this is the reason") == {:error, "this is the reason"}
     end
+
+    test "result" do
+      assert NIF.codec_result_string_int64_ok("fine") == {:ok, "fine"}
+      assert NIF.codec_result_string_int64_error(42) == {:error, 42}
+      assert NIF.codec_result_string_int64_ok_conversion() == {:ok, "fine"}
+      assert NIF.codec_result_string_int64_error_conversion() == {:error, 42}
+      assert NIF.codec_result_int64_string_void_ok_conversion() == {:ok, 201_702, "c++17"}
+    end
   end
 
   describe "resource" do