From 8ef6efc1844a9782dbe265271020f6ffee571af0 Mon Sep 17 00:00:00 2001
From: Robert Hensing <robert@roberthensing.nl>
Date: Thu, 23 May 2024 18:36:40 +0200
Subject: [PATCH] C API: Require non-thunk value from primop definition

---
 src/libexpr-c/nix_api_value.cc     |  9 +++++++
 tests/unit/libexpr/nix_api_expr.cc | 41 ++++++++++++++++++++++++++++++
 2 files changed, 50 insertions(+)

diff --git a/src/libexpr-c/nix_api_value.cc b/src/libexpr-c/nix_api_value.cc
index 043c6a568..fa24b4494 100644
--- a/src/libexpr-c/nix_api_value.cc
+++ b/src/libexpr-c/nix_api_value.cc
@@ -100,6 +100,15 @@ static void nix_c_primop_wrapper(
             .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;
 }
 
diff --git a/tests/unit/libexpr/nix_api_expr.cc b/tests/unit/libexpr/nix_api_expr.cc
index 9d629b463..e78f2bf61 100644
--- a/tests/unit/libexpr/nix_api_expr.cc
+++ b/tests/unit/libexpr/nix_api_expr.cc
@@ -257,4 +257,45 @@ TEST_F(nix_api_expr_test, nix_expr_primop_bad_no_return)
     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 * partial = nix_alloc_value(ctx, state);
+    assert_ctx_ok();
+    nix_value_call(ctx, state, primopValue, toString, partial);
+    assert_ctx_ok();
+
+    Value * result = nix_alloc_value(ctx, state);
+    assert_ctx_ok();
+    nix_value_call(ctx, state, partial, four, 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 must not be a thunk")));
+    ASSERT_THAT(ctx->last_err, testing::Optional(testing::HasSubstr("badReturnThunk")));
+}
 } // namespace nixC