Merge pull request #10767 from hercules-ci/fix-c-api-primop-for-strict-initializers

C API: Fix custom primops

src/libexpr-c/nix_api_expr.cc

@@ -65,6 +65,17 @@ nix_err nix_value_call(nix_c_context * context, EvalState * state, Value * fn, V
     NIXC_CATCH_ERRS
 }
 
+nix_err nix_value_call_multi(nix_c_context * context, EvalState * state, Value * fn, size_t nargs, Value ** args, Value * value)
+{
+    if (context)
+        context->last_err_code = NIX_OK;
+    try {
+        state->state.callFunction(*(nix::Value *) fn, nargs, (nix::Value * *)args, *(nix::Value *) value, nix::noPos);
+        state->state.forceValue(*(nix::Value *) value, nix::noPos);
+    }
+    NIXC_CATCH_ERRS
+}
+
 nix_err nix_value_force(nix_c_context * context, EvalState * state, Value * value)
 {
     if (context)

src/libexpr-c/nix_api_expr.h

@@ -12,6 +12,7 @@
 
 #include "nix_api_store.h"
 #include "nix_api_util.h"
+#include <stddef.h>
 
 #ifdef __cplusplus
 extern "C" {
@@ -80,6 +81,46 @@ nix_err nix_expr_eval_from_string(
  */
 nix_err nix_value_call(nix_c_context * context, EvalState * state, Value * fn, Value * arg, Value * value);
 
+/**
+ * @brief Calls a Nix function with multiple arguments.
+ *
+ * Technically these are functions that return functions. It is common for Nix
+ * functions to be curried, so this function is useful for calling them.
+ *
+ * @param[out] context Optional, stores error information
+ * @param[in] state The state of the evaluation.
+ * @param[in] fn The Nix function to call.
+ * @param[in] nargs The number of arguments.
+ * @param[in] args The arguments to pass to the function.
+ * @param[out] value The result of the function call.
+ *
+ * @see nix_value_call     For the single argument primitive.
+ * @see NIX_VALUE_CALL           For a macro that wraps this function for convenience.
+ */
+nix_err nix_value_call_multi(
+    nix_c_context * context, EvalState * state, Value * fn, size_t nargs, Value ** args, Value * value);
+
+/**
+ * @brief Calls a Nix function with multiple arguments.
+ *
+ * Technically these are functions that return functions. It is common for Nix
+ * functions to be curried, so this function is useful for calling them.
+ *
+ * @param[out] context Optional, stores error information
+ * @param[in] state The state of the evaluation.
+ * @param[out] value The result of the function call.
+ * @param[in] fn The Nix function to call.
+ * @param[in] args The arguments to pass to the function.
+ *
+ * @see nix_value_call_multi
+ */
+#define NIX_VALUE_CALL(context, state, value, fn, ...)                  \
+  do {                                                                  \
+    Value * args_array[] = {__VA_ARGS__};                               \
+    size_t nargs = sizeof(args_array) / sizeof(args_array[0]);          \
+    nix_value_call_multi(context, state, fn, nargs, args_array, value); \
+  } while (0)
+
 /**
  * @brief Forces the evaluation of a Nix value.
  *

src/libexpr-c/nix_api_value.cc

@@ -73,10 +73,43 @@ static void nix_c_primop_wrapper(
     PrimOpFun f, void * userdata, nix::EvalState & state, const nix::PosIdx pos, nix::Value ** args, nix::Value & v)
 {
     nix_c_context ctx;
-    f(userdata, &ctx, (EvalState *) &state, (Value **) args, (Value *) &v);
+
-    /* TODO: In the future, this should throw different errors depending on the error code */
+    // v currently has a thunk, but the C API initializers require an uninitialized value.
-    if (ctx.last_err_code != NIX_OK)
+    //
-        state.error<nix::EvalError>("Error from builtin function: %s", *ctx.last_err).atPos(pos).debugThrow();
+    // We can't destroy the thunk, because that makes it impossible to retry,
+    // which is needed for tryEval and for evaluation drivers that evaluate more
+    // than one value (e.g. an attrset with two derivations, both of which
+    // reference v).
+    //
+    // Instead we create a temporary value, and then assign the result to v.
+    // This does not give the primop definition access to the thunk, but that's
+    // ok because we don't see a need for this yet (e.g. inspecting thunks,
+    // or maybe something to make blackholes work better; we don't know).
+    nix::Value vTmp;
+
+    f(userdata, &ctx, (EvalState *) &state, (Value **) args, (Value *) &vTmp);
+
+    if (ctx.last_err_code != NIX_OK) {
+        /* TODO: Throw different errors depending on the error code */
+        state.error<nix::EvalError>("Error from custom function: %s", *ctx.last_err).atPos(pos).debugThrow();
+    }
+
+    if (!vTmp.isValid()) {
+        state.error<nix::EvalError>("Implementation error in custom function: return value was not initialized")
+            .atPos(pos)
+            .debugThrow();
+    }
+
+    if (vTmp.type() == nix::nThunk) {
+        // We might allow this in the future if it makes sense for the evaluator
+        // e.g. implementing tail recursion by returning a thunk to the next
+        // "iteration". Until then, this is most likely a mistake or misunderstanding.
+        state.error<nix::EvalError>("Implementation error in custom function: return value must not be a thunk")
+            .atPos(pos)
+            .debugThrow();
+    }
+
+    v = vTmp;
 }
 
 PrimOp * nix_alloc_primop(

tests/unit/libexpr/nix_api_expr.cc

@@ -191,4 +191,212 @@ TEST_F(nix_api_expr_test, nix_expr_realise_context)
     nix_realised_string_free(r);
 }
 
+const char * SAMPLE_USER_DATA = "whatever";
+
+static void primop_square(void * user_data, nix_c_context * context, EvalState * state, Value ** args, Value * ret)
+{
+    assert(context);
+    assert(state);
+    assert(user_data == SAMPLE_USER_DATA);
+    auto i = nix_get_int(context, args[0]);
+    nix_init_int(context, ret, i * i);
+}
+
+TEST_F(nix_api_expr_test, nix_expr_primop)
+{
+    PrimOp * primop =
+        nix_alloc_primop(ctx, primop_square, 1, "square", nullptr, "square an integer", (void *) SAMPLE_USER_DATA);
+    assert_ctx_ok();
+    Value * primopValue = nix_alloc_value(ctx, state);
+    assert_ctx_ok();
+    nix_init_primop(ctx, primopValue, primop);
+    assert_ctx_ok();
+
+    Value * three = nix_alloc_value(ctx, state);
+    assert_ctx_ok();
+    nix_init_int(ctx, three, 3);
+    assert_ctx_ok();
+
+    Value * result = nix_alloc_value(ctx, state);
+    assert_ctx_ok();
+    nix_value_call(ctx, state, primopValue, three, result);
+    assert_ctx_ok();
+
+    auto r = nix_get_int(ctx, result);
+    ASSERT_EQ(9, r);
+}
+
+static void primop_repeat(void * user_data, nix_c_context * context, EvalState * state, Value ** args, Value * ret)
+{
+    assert(context);
+    assert(state);
+    assert(user_data == SAMPLE_USER_DATA);
+
+    // Get the string to repeat
+    std::string s;
+    if (nix_get_string(context, args[0], OBSERVE_STRING(s)) != NIX_OK)
+        return;
+
+    // Get the number of times to repeat
+    auto n = nix_get_int(context, args[1]);
+    if (nix_err_code(context) != NIX_OK)
+        return;
+
+    // Repeat the string
+    std::string result;
+    for (int i = 0; i < n; ++i)
+        result += s;
+
+    nix_init_string(context, ret, result.c_str());
+}
+
+TEST_F(nix_api_expr_test, nix_expr_primop_arity_2_multiple_calls)
+{
+    PrimOp * primop =
+        nix_alloc_primop(ctx, primop_repeat, 2, "repeat", nullptr, "repeat a string", (void *) SAMPLE_USER_DATA);
+    assert_ctx_ok();
+    Value * primopValue = nix_alloc_value(ctx, state);
+    assert_ctx_ok();
+    nix_init_primop(ctx, primopValue, primop);
+    assert_ctx_ok();
+
+    Value * hello = nix_alloc_value(ctx, state);
+    assert_ctx_ok();
+    nix_init_string(ctx, hello, "hello");
+    assert_ctx_ok();
+
+    Value * three = nix_alloc_value(ctx, state);
+    assert_ctx_ok();
+    nix_init_int(ctx, three, 3);
+    assert_ctx_ok();
+
+    Value * partial = nix_alloc_value(ctx, state);
+    assert_ctx_ok();
+    nix_value_call(ctx, state, primopValue, hello, partial);
+    assert_ctx_ok();
+
+    Value * result = nix_alloc_value(ctx, state);
+    assert_ctx_ok();
+    nix_value_call(ctx, state, partial, three, result);
+    assert_ctx_ok();
+
+    std::string r;
+    nix_get_string(ctx, result, OBSERVE_STRING(r));
+    ASSERT_STREQ("hellohellohello", r.c_str());
+}
+
+TEST_F(nix_api_expr_test, nix_expr_primop_arity_2_single_call)
+{
+    PrimOp * primop =
+        nix_alloc_primop(ctx, primop_repeat, 2, "repeat", nullptr, "repeat a string", (void *) SAMPLE_USER_DATA);
+    assert_ctx_ok();
+    Value * primopValue = nix_alloc_value(ctx, state);
+    assert_ctx_ok();
+    nix_init_primop(ctx, primopValue, primop);
+    assert_ctx_ok();
+
+    Value * hello = nix_alloc_value(ctx, state);
+    assert_ctx_ok();
+    nix_init_string(ctx, hello, "hello");
+    assert_ctx_ok();
+
+    Value * three = nix_alloc_value(ctx, state);
+    assert_ctx_ok();
+    nix_init_int(ctx, three, 3);
+    assert_ctx_ok();
+
+    Value * result = nix_alloc_value(ctx, state);
+    assert_ctx_ok();
+    NIX_VALUE_CALL(ctx, state, result, primopValue, hello, three);
+    assert_ctx_ok();
+
+    std::string r;
+    nix_get_string(ctx, result, OBSERVE_STRING(r));
+    assert_ctx_ok();
+
+    ASSERT_STREQ("hellohellohello", r.c_str());
+}
+
+static void
+primop_bad_no_return(void * user_data, nix_c_context * context, EvalState * state, Value ** args, Value * ret)
+{
+}
+
+TEST_F(nix_api_expr_test, nix_expr_primop_bad_no_return)
+{
+    PrimOp * primop =
+        nix_alloc_primop(ctx, primop_bad_no_return, 1, "badNoReturn", nullptr, "a broken primop", nullptr);
+    assert_ctx_ok();
+    Value * primopValue = nix_alloc_value(ctx, state);
+    assert_ctx_ok();
+    nix_init_primop(ctx, primopValue, primop);
+    assert_ctx_ok();
+
+    Value * three = nix_alloc_value(ctx, state);
+    assert_ctx_ok();
+    nix_init_int(ctx, three, 3);
+    assert_ctx_ok();
+
+    Value * result = nix_alloc_value(ctx, state);
+    assert_ctx_ok();
+    nix_value_call(ctx, state, primopValue, three, result);
+    ASSERT_EQ(ctx->last_err_code, NIX_ERR_NIX_ERROR);
+    ASSERT_THAT(
+        ctx->last_err,
+        testing::Optional(
+            testing::HasSubstr("Implementation error in custom function: return value was not initialized")));
+    ASSERT_THAT(ctx->last_err, testing::Optional(testing::HasSubstr("badNoReturn")));
+}
+
+static void
+primop_bad_return_thunk(void * user_data, nix_c_context * context, EvalState * state, Value ** args, Value * ret)
+{
+    nix_init_apply(context, ret, args[0], args[1]);
+}
+TEST_F(nix_api_expr_test, nix_expr_primop_bad_return_thunk)
+{
+    PrimOp * primop =
+        nix_alloc_primop(ctx, primop_bad_return_thunk, 2, "badReturnThunk", nullptr, "a broken primop", nullptr);
+    assert_ctx_ok();
+    Value * primopValue = nix_alloc_value(ctx, state);
+    assert_ctx_ok();
+    nix_init_primop(ctx, primopValue, primop);
+    assert_ctx_ok();
+
+    Value * toString = nix_alloc_value(ctx, state);
+    assert_ctx_ok();
+    nix_expr_eval_from_string(ctx, state, "builtins.toString", ".", toString);
+    assert_ctx_ok();
+
+    Value * four = nix_alloc_value(ctx, state);
+    assert_ctx_ok();
+    nix_init_int(ctx, four, 4);
+    assert_ctx_ok();
+
+    Value * result = nix_alloc_value(ctx, state);
+    assert_ctx_ok();
+    NIX_VALUE_CALL(ctx, state, result, primopValue, toString, four);
+
+    ASSERT_EQ(ctx->last_err_code, NIX_ERR_NIX_ERROR);
+    ASSERT_THAT(
+        ctx->last_err,
+        testing::Optional(
+            testing::HasSubstr("Implementation error in custom function: return value must not be a thunk")));
+    ASSERT_THAT(ctx->last_err, testing::Optional(testing::HasSubstr("badReturnThunk")));
+}
+
+TEST_F(nix_api_expr_test, nix_value_call_multi_no_args)
+{
+    Value * n = nix_alloc_value(ctx, state);
+    nix_init_int(ctx, n, 3);
+    assert_ctx_ok();
+
+    Value * r = nix_alloc_value(ctx, state);
+    nix_value_call_multi(ctx, state, n, 0, nullptr, r);
+    assert_ctx_ok();
+
+    auto rInt = nix_get_int(ctx, r);
+    assert_ctx_ok();
+    ASSERT_EQ(3, rInt);
+}
 } // namespace nixC