Merge pull request #14030 from roberth/c-api-item-access

C API: Various improvements to attribute/item access

Author: Mic92

doc/manual/rl-next/c-api-lazy-accessors.md

@@ -0,0 +1,16 @@
+---
+synopsis: "C API: Add lazy attribute and list item accessors"
+prs: [14030]
+---
+
+The C API now includes lazy accessor functions for retrieving values from lists and attribute sets without forcing evaluation:
+
+- `nix_get_list_byidx_lazy()` - Get a list element without forcing its evaluation
+- `nix_get_attr_byname_lazy()` - Get an attribute value by name without forcing evaluation
+- `nix_get_attr_byidx_lazy()` - Get an attribute by index without forcing evaluation
+
+These functions are useful when forwarding unevaluated sub-values to other lists, attribute sets, or function calls. They allow more efficient handling of Nix values by deferring evaluation until actually needed.
+
+Additionally, bounds checking has been improved for all `_byidx` functions to properly validate indices before access, preventing potential out-of-bounds errors.
+
+The documentation for `NIX_ERR_KEY` error handling has also been clarified to specify when this error code is returned.

src/libexpr-c/nix_api_value.cc

@@ -326,6 +326,10 @@ nix_value * nix_get_list_byidx(nix_c_context * context, const nix_value * value,
     try {
         auto & v = check_value_in(value);
         assert(v.type() == nix::nList);
+        if (ix >= v.listSize()) {
+            nix_set_err_msg(context, NIX_ERR_KEY, "list index out of bounds");
+            return nullptr;
+        }
         auto * p = v.listView()[ix];
         nix_gc_incref(nullptr, p);
         if (p != nullptr)
@@ -335,6 +339,26 @@ nix_value * nix_get_list_byidx(nix_c_context * context, const nix_value * value,
     NIXC_CATCH_ERRS_NULL
 }
 
+nix_value *
+nix_get_list_byidx_lazy(nix_c_context * context, const nix_value * value, EvalState * state, unsigned int ix)
+{
+    if (context)
+        context->last_err_code = NIX_OK;
+    try {
+        auto & v = check_value_in(value);
+        assert(v.type() == nix::nList);
+        if (ix >= v.listSize()) {
+            nix_set_err_msg(context, NIX_ERR_KEY, "list index out of bounds");
+            return nullptr;
+        }
+        auto * p = v.listView()[ix];
+        nix_gc_incref(nullptr, p);
+        // Note: intentionally NOT calling forceValue() to keep the element lazy
+        return as_nix_value_ptr(p);
+    }
+    NIXC_CATCH_ERRS_NULL
+}
+
 nix_value * nix_get_attr_byname(nix_c_context * context, const nix_value * value, EvalState * state, const char * name)
 {
     if (context)
@@ -355,6 +379,27 @@ nix_value * nix_get_attr_byname(nix_c_context * context, const nix_value * value
     NIXC_CATCH_ERRS_NULL
 }
 
+nix_value *
+nix_get_attr_byname_lazy(nix_c_context * context, const nix_value * value, EvalState * state, const char * name)
+{
+    if (context)
+        context->last_err_code = NIX_OK;
+    try {
+        auto & v = check_value_in(value);
+        assert(v.type() == nix::nAttrs);
+        nix::Symbol s = state->state.symbols.create(name);
+        auto attr = v.attrs()->get(s);
+        if (attr) {
+            nix_gc_incref(nullptr, attr->value);
+            // Note: intentionally NOT calling forceValue() to keep the attribute lazy
+            return as_nix_value_ptr(attr->value);
+        }
+        nix_set_err_msg(context, NIX_ERR_KEY, "missing attribute");
+        return nullptr;
+    }
+    NIXC_CATCH_ERRS_NULL
+}
+
 bool nix_has_attr_byname(nix_c_context * context, const nix_value * value, EvalState * state, const char * name)
 {
     if (context)
@@ -389,6 +434,10 @@ nix_get_attr_byidx(nix_c_context * context, nix_value * value, EvalState * state
     try {
         auto & v = check_value_in(value);
         collapse_attrset_layer_chain_if_needed(v, state);
+        if (i >= v.attrs()->size()) {
+            nix_set_err_msg(context, NIX_ERR_KEY, "attribute index out of bounds");
+            return nullptr;
+        }
         const nix::Attr & a = (*v.attrs())[i];
         *name = state->state.symbols[a.name].c_str();
         nix_gc_incref(nullptr, a.value);
@@ -398,13 +447,38 @@ nix_get_attr_byidx(nix_c_context * context, nix_value * value, EvalState * state
     NIXC_CATCH_ERRS_NULL
 }
 
+nix_value * nix_get_attr_byidx_lazy(
+    nix_c_context * context, nix_value * value, EvalState * state, unsigned int i, const char ** name)
+{
+    if (context)
+        context->last_err_code = NIX_OK;
+    try {
+        auto & v = check_value_in(value);
+        collapse_attrset_layer_chain_if_needed(v, state);
+        if (i >= v.attrs()->size()) {
+            nix_set_err_msg(context, NIX_ERR_KEY, "attribute index out of bounds (Nix C API contract violation)");
+            return nullptr;
+        }
+        const nix::Attr & a = (*v.attrs())[i];
+        *name = state->state.symbols[a.name].c_str();
+        nix_gc_incref(nullptr, a.value);
+        // Note: intentionally NOT calling forceValue() to keep the attribute lazy
+        return as_nix_value_ptr(a.value);
+    }
+    NIXC_CATCH_ERRS_NULL
+}
+
 const char * nix_get_attr_name_byidx(nix_c_context * context, nix_value * value, EvalState * state, unsigned int i)
 {
     if (context)
         context->last_err_code = NIX_OK;
     try {
         auto & v = check_value_in(value);
         collapse_attrset_layer_chain_if_needed(v, state);
+        if (i >= v.attrs()->size()) {
+            nix_set_err_msg(context, NIX_ERR_KEY, "attribute index out of bounds (Nix C API contract violation)");
+            return nullptr;
+        }
         const nix::Attr & a = (*v.attrs())[i];
         return state->state.symbols[a.name].c_str();
     }

src/libexpr-c/nix_api_value.h

@@ -265,17 +265,47 @@ ExternalValue * nix_get_external(nix_c_context * context, nix_value * value);
  */
 nix_value * nix_get_list_byidx(nix_c_context * context, const nix_value * value, EvalState * state, unsigned int ix);
 
-/** @brief Get an attr by name
+/** @brief Get the ix'th element of a list without forcing evaluation of the element
+ *
+ * Returns the list element without forcing its evaluation, allowing access to lazy values.
+ * The list value itself must already be evaluated.
  *
  * Owned by the GC. Use nix_gc_decref when you're done with the pointer
  * @param[out] context Optional, stores error information
+ * @param[in] value Nix value to inspect (must be an evaluated list)
+ * @param[in] state nix evaluator state
+ * @param[in] ix list element to get
+ * @return value, NULL in case of errors
+ */
+nix_value *
+nix_get_list_byidx_lazy(nix_c_context * context, const nix_value * value, EvalState * state, unsigned int ix);
+
+/** @brief Get an attr by name
+ *
+ * Use nix_gc_decref when you're done with the pointer
+ * @param[out] context Optional, stores error information
  * @param[in] value Nix value to inspect
  * @param[in] state nix evaluator state
  * @param[in] name attribute name
  * @return value, NULL in case of errors
  */
 nix_value * nix_get_attr_byname(nix_c_context * context, const nix_value * value, EvalState * state, const char * name);
 
+/** @brief Get an attribute value by attribute name, without forcing evaluation of the attribute's value
+ *
+ * Returns the attribute value without forcing its evaluation, allowing access to lazy values.
+ * The attribute set value itself must already be evaluated.
+ *
+ * Use nix_gc_decref when you're done with the pointer
+ * @param[out] context Optional, stores error information
+ * @param[in] value Nix value to inspect (must be an evaluated attribute set)
+ * @param[in] state nix evaluator state
+ * @param[in] name attribute name
+ * @return value, NULL in case of errors
+ */
+nix_value *
+nix_get_attr_byname_lazy(nix_c_context * context, const nix_value * value, EvalState * state, const char * name);
+
 /** @brief Check if an attribute name exists on a value
  * @param[out] context Optional, stores error information
  * @param[in] value Nix value to inspect
@@ -285,11 +315,21 @@ nix_value * nix_get_attr_byname(nix_c_context * context, const nix_value * value
  */
 bool nix_has_attr_byname(nix_c_context * context, const nix_value * value, EvalState * state, const char * name);
 
-/** @brief Get an attribute by index in the sorted bindings
+/** @brief Get an attribute by index
  *
  * Also gives you the name.
  *
- * Owned by the GC. Use nix_gc_decref when you're done with the pointer
+ * Attributes are returned in an unspecified order which is NOT suitable for
+ * reproducible operations. In Nix's domain, reproducibility is paramount. The caller
+ * is responsible for sorting the attributes or storing them in an ordered map to
+ * ensure deterministic behavior in your application.
+ *
+ * @note When Nix does sort attributes, which it does for virtually all intermediate
+ * operations and outputs, it uses byte-wise lexicographic order (equivalent to
+ * lexicographic order by Unicode scalar value for valid UTF-8). We recommend
+ * applying this same ordering for consistency.
+ *
+ * Use nix_gc_decref when you're done with the pointer
  * @param[out] context Optional, stores error information
  * @param[in] value Nix value to inspect
  * @param[in] state nix evaluator state
@@ -300,9 +340,47 @@ bool nix_has_attr_byname(nix_c_context * context, const nix_value * value, EvalS
 nix_value *
 nix_get_attr_byidx(nix_c_context * context, nix_value * value, EvalState * state, unsigned int i, const char ** name);
 
-/** @brief Get an attribute name by index in the sorted bindings
+/** @brief Get an attribute by index, without forcing evaluation of the attribute's value
+ *
+ * Also gives you the name.
+ *
+ * Returns the attribute value without forcing its evaluation, allowing access to lazy values.
+ * The attribute set value itself must already have been evaluated.
+ *
+ * Attributes are returned in an unspecified order which is NOT suitable for
+ * reproducible operations. In Nix's domain, reproducibility is paramount. The caller
+ * is responsible for sorting the attributes or storing them in an ordered map to
+ * ensure deterministic behavior in your application.
+ *
+ * @note When Nix does sort attributes, which it does for virtually all intermediate
+ * operations and outputs, it uses byte-wise lexicographic order (equivalent to
+ * lexicographic order by Unicode scalar value for valid UTF-8). We recommend
+ * applying this same ordering for consistency.
+ *
+ * Use nix_gc_decref when you're done with the pointer
+ * @param[out] context Optional, stores error information
+ * @param[in] value Nix value to inspect (must be an evaluated attribute set)
+ * @param[in] state nix evaluator state
+ * @param[in] i attribute index
+ * @param[out] name will store a pointer to the attribute name
+ * @return value, NULL in case of errors
+ */
+nix_value * nix_get_attr_byidx_lazy(
+    nix_c_context * context, nix_value * value, EvalState * state, unsigned int i, const char ** name);
+
+/** @brief Get an attribute name by index
+ *
+ * Returns the attribute name without forcing evaluation of the attribute's value.
+ *
+ * Attributes are returned in an unspecified order which is NOT suitable for
+ * reproducible operations. In Nix's domain, reproducibility is paramount. The caller
+ * is responsible for sorting the attributes or storing them in an ordered map to
+ * ensure deterministic behavior in your application.
  *
- * Useful when you want the name but want to avoid evaluation.
+ * @note When Nix does sort attributes, which it does for virtually all intermediate
+ * operations and outputs, it uses byte-wise lexicographic order (equivalent to
+ * lexicographic order by Unicode scalar value for valid UTF-8). We recommend
+ * applying this same ordering for consistency.
  *
  * Owned by the nix EvalState
  * @param[out] context Optional, stores error information

src/libexpr-tests/nix_api_expr.cc

@@ -423,6 +423,55 @@ TEST_F(nix_api_expr_test, nix_expr_primop_bad_return_thunk)
     ASSERT_THAT(nix_err_msg(nullptr, ctx, nullptr), testing::HasSubstr("badReturnThunk"));
 }
 
+static void primop_with_nix_err_key(
+    void * user_data, nix_c_context * context, EvalState * state, nix_value ** args, nix_value * ret)
+{
+    nix_set_err_msg(context, NIX_ERR_KEY, "Test error from primop");
+}
+
+TEST_F(nix_api_expr_test, nix_expr_primop_nix_err_key_conversion)
+{
+    // Test that NIX_ERR_KEY from a custom primop gets converted to a generic EvalError
+    //
+    // RATIONALE: NIX_ERR_KEY must not be propagated from custom primops because it would
+    // create semantic confusion. NIX_ERR_KEY indicates missing keys/indices in C API functions
+    // (like nix_get_attr_byname, nix_get_list_byidx). If custom primops could return NIX_ERR_KEY,
+    // an evaluation error would be indistinguishable from an actual missing attribute.
+    //
+    // For example, if nix_get_attr_byname returned NIX_ERR_KEY when the attribute is present
+    // but the value evaluation fails, callers expecting NIX_ERR_KEY to mean "missing attribute"
+    // would incorrectly handle evaluation failures as missing attributes. In places where
+    // missing attributes are tolerated (like optional attributes), this would cause the
+    // program to continue after swallowing the error, leading to silent failures.
+    PrimOp * primop = nix_alloc_primop(
+        ctx, primop_with_nix_err_key, 1, "testErrorPrimop", nullptr, "a test primop that sets NIX_ERR_KEY", nullptr);
+    assert_ctx_ok();
+    nix_value * primopValue = nix_alloc_value(ctx, state);
+    assert_ctx_ok();
+    nix_init_primop(ctx, primopValue, primop);
+    assert_ctx_ok();
+
+    nix_value * arg = nix_alloc_value(ctx, state);
+    assert_ctx_ok();
+    nix_init_int(ctx, arg, 42);
+    assert_ctx_ok();
+
+    nix_value * result = nix_alloc_value(ctx, state);
+    assert_ctx_ok();
+    nix_value_call(ctx, state, primopValue, arg, result);
+
+    // Verify that NIX_ERR_KEY gets converted to NIX_ERR_NIX_ERROR (generic evaluation error)
+    ASSERT_EQ(nix_err_code(ctx), NIX_ERR_NIX_ERROR);
+    ASSERT_THAT(nix_err_msg(nullptr, ctx, nullptr), testing::HasSubstr("Error from custom function"));
+    ASSERT_THAT(nix_err_msg(nullptr, ctx, nullptr), testing::HasSubstr("Test error from primop"));
+    ASSERT_THAT(nix_err_msg(nullptr, ctx, nullptr), testing::HasSubstr("testErrorPrimop"));
+
+    // Clean up
+    nix_gc_decref(ctx, primopValue);
+    nix_gc_decref(ctx, arg);
+    nix_gc_decref(ctx, result);
+}
+
 TEST_F(nix_api_expr_test, nix_value_call_multi_no_args)
 {
     nix_value * n = nix_alloc_value(ctx, state);