C API: refactor ListBuilder

tweag · Feb 27, 2024 · 5e46220 · 5e46220
1 parent f97242c
commit 5e46220
Show file tree

Hide file tree

Showing 4 changed files with 50 additions and 37 deletions.
diff --git a/src/libexpr/c/nix_api_expr_internal.h b/src/libexpr/c/nix_api_expr_internal.h
@@ -3,6 +3,33 @@
 
 #include "eval.hh"
 #include "attr-set.hh"
+#include "nix_api_value.h"
+
+class CListBuilder
+{
+private:
+    std::vector<nix::Value *> values;
+
+public:
+    CListBuilder(size_t capacity)
+    {
+        values.reserve(capacity);
+    }
+
+    void push_back(nix::Value * value)
+    {
+        values.push_back(value);
+    }
+
+    Value * finish(nix::EvalState * state, nix::Value * list)
+    {
+        state->mkList(*list, values.size());
+        for (size_t n = 0; n < list->listSize(); ++n) {
+            list->listElems()[n] = values[n];
+        }
+        return list;
+    }
+};
 
 struct EvalState
 {
@@ -14,6 +41,11 @@ struct BindingsBuilder
     nix::BindingsBuilder builder;
 };
 
+struct ListBuilder
+{
+    CListBuilder builder;
+};
+
 struct nix_string_return
 {
     std::string str;

diff --git a/src/libexpr/c/nix_api_value.cc b/src/libexpr/c/nix_api_value.cc
@@ -17,32 +17,6 @@
 #include "gc_cpp.h"
 #endif
 
-class ListBuilder
-{
-private:
-    std::vector<nix::Value *> values;
-
-public:
-    ListBuilder(size_t capacity)
-    {
-        values.reserve(capacity);
-    }
-
-    void push_back(nix::Value * value)
-    {
-        values.push_back(value);
-    }
-
-    Value * finish(nix::EvalState * state, nix::Value * list)
-    {
-        state->mkList(*list, values.size());
-        for (size_t n = 0; n < list->listSize(); ++n) {
-            list->listElems()[n] = values[n];
-        }
-        return list;
-    }
-};
-
 // Helper function to throw an exception if value is null
 static const nix::Value & check_value_not_null(const Value * value)
 {
@@ -443,7 +417,7 @@ ListBuilder * nix_make_list_builder(nix_c_context * context, EvalState * state,
     if (context)
         context->last_err_code = NIX_OK;
     try {
-        auto builder = ListBuilder(capacity);
+        auto builder = CListBuilder(capacity);
         return new
 #if HAVE_BOEHMGC
             (NoGC)
@@ -453,12 +427,12 @@ ListBuilder * nix_make_list_builder(nix_c_context * context, EvalState * state,
     NIXC_CATCH_ERRS_NULL
 }
 
-nix_err nix_list_builder_insert(nix_c_context * context, ListBuilder * builder, Value * value)
+nix_err nix_list_builder_insert(nix_c_context * context, ListBuilder * list_builder, Value * value)
 {
     if (context)
         context->last_err_code = NIX_OK;
     try {
-        builder->push_back((nix::Value *) value);
+        list_builder->builder.push_back((nix::Value *) value);
     }
     NIXC_CATCH_ERRS
 }
@@ -472,13 +446,13 @@ void nix_list_builder_free(ListBuilder * bb)
 #endif
 }
 
-nix_err nix_make_list(nix_c_context * context, EvalState * s, Value * value, ListBuilder * b)
+nix_err nix_make_list(nix_c_context * context, EvalState * s, ListBuilder * list_builder, Value * value)
 {
     if (context)
         context->last_err_code = NIX_OK;
     try {
         auto & v = check_value_not_null(value);
-        b->finish(&(s->state), &v);
+        list_builder->builder.finish(&(s->state), &v);
     }
     NIXC_CATCH_ERRS
 }

diff --git a/src/libexpr/c/nix_api_value.h b/src/libexpr/c/nix_api_value.h
@@ -46,7 +46,14 @@ typedef struct EvalState EvalState;
  */
 typedef struct BindingsBuilder BindingsBuilder;
 
-typedef class ListBuilder ListBuilder;
+/** @brief Stores an under-construction list
+ * @ingroup value_manip
+ *
+ * Do not reuse.
+ * @see nix_make_list_builder, nix_list_builder_free, nix_make_list
+ * @see nix_list_builder_insert
+ */
+typedef struct ListBuilder ListBuilder;
 
 /** @brief PrimOp function
  * @ingroup primops
@@ -336,11 +343,11 @@ nix_err nix_init_external(nix_c_context * context, Value * value, ExternalValue
 
 /** @brief Create a list from a list builder
  * @param[out] context Optional, stores error information
+ * @param[in] list_builder list builder to use. Make sure to unref this afterwards.
  * @param[out] value Nix value to modify
- * @param[in] b list builder to use. Make sure to unref this afterwards.
  * @return error code, NIX_OK on success.
  */
-nix_err nix_make_list(nix_c_context * context, EvalState * s, Value * value, ListBuilder * b);
+nix_err nix_make_list(nix_c_context * context, EvalState * s, ListBuilder * list_builder, Value * value);
 
 /** @brief Create a list builder
  * @param[out] context Optional, stores error information
@@ -352,11 +359,11 @@ ListBuilder * nix_make_list_builder(nix_c_context * context, EvalState * state,
 
 /** @brief Insert bindings into a builder
  * @param[out] context Optional, stores error information
- * @param[in] builder ListBuilder to insert into
+ * @param[in] list_builder ListBuilder to insert into
  * @param[in] value value to insert
  * @return error code, NIX_OK on success.
  */
-nix_err nix_list_builder_insert(nix_c_context * context, ListBuilder * builder, Value * value);
+nix_err nix_list_builder_insert(nix_c_context * context, ListBuilder * list_builder, Value * value);
 
 /** @brief Free a list builder
  *

diff --git a/tests/unit/libexpr/nix_api_value.cc b/tests/unit/libexpr/nix_api_value.cc
@@ -78,7 +78,7 @@ TEST_F(nix_api_expr_test, nix_build_and_init_list)
     Value * intValue = nix_alloc_value(nullptr, state);
     nix_init_int(nullptr, intValue, 42);
     nix_list_builder_insert(nullptr, builder, intValue);
-    nix_make_list(nullptr, state, value, builder);
+    nix_make_list(nullptr, state, builder, value);
     nix_list_builder_free(builder);
 
     ASSERT_EQ(42, nix_get_int(nullptr, nix_get_list_byidx(nullptr, value, state, 0)));