feat: persistent lists

.clang-tidy

@@ -1,3 +1,13 @@
-# We use pointers to aggregates in a couple of places, intentionally.
-# void * would look weird.
-Checks: '-bugprone-sizeof-expression'
+Checks:
+  - bugprone-*
+  - clang-analyzer-*
+  - cppcoreguidelines-*
+  - modernize-*
+  - -modernize-use-trailing-return-type
+  - performance-*
+  - portability-*
+  - readability-*
+  - -readability-identifier-length
+FormatStyle: file
+InheritParentConfig: true
+User: connorbaker

.gitignore

@@ -155,6 +155,8 @@ result-*
 # IDE
 .vscode/
 .idea/
+.direnv/
+.envrc
 
 .pre-commit-config.yaml
 

flake.nix

@@ -98,6 +98,42 @@
               useLLVM = true;
             };
             overlays = [
+              # Update immer and range-v3 to the latest
+              # TODO(@connorbaker): Upstream this.
+              (final: prev: {
+                immer =
+                  let
+                    inherit (final.lib.strings) cmakeBool;
+                  in
+                  prev.immer.overrideAttrs (finalAttrs: _: {
+                    strictDeps = true;
+                    version = "0.8.1-unstable-2024-09-18";
+                    src = final.fetchFromGitHub {
+                      owner = "arximboldi";
+                      repo = "immer";
+                      rev = "df6ef46d97e1fe81f397015b9aeb32505cef653b";
+                      hash = "sha256-fV6Rqbg/vtUH2DdgLYULl0zLM3WUSG1qYLZtqAhaWQw=";
+                    };
+                    doCheck = false;
+                    # TODO(@connorbaker): Add support for doCheck = true;
+                    cmakeFlags = [
+                      (cmakeBool "immer_BUILD_DOCS" false)
+                      (cmakeBool "immer_BUILD_EXAMPLES" finalAttrs.doCheck)
+                      (cmakeBool "immer_BUILD_EXTRAS" false)
+                      (cmakeBool "immer_BUILD_TESTS" finalAttrs.doCheck)
+                    ];
+                  });
+                range-v3 = prev.range-v3.overrideAttrs {
+                  strictDeps = true;
+                  version = "0.12.0-unstable-2024-10-01";
+                  src = final.fetchFromGitHub {
+                    owner = "ericniebler";
+                    repo = "range-v3";
+                    rev = "7e6f34b1e820fb8321346888ef0558a0ec842b8e";
+                    hash = "sha256-FbLZ8CHnLdTs0FRdLPTOBiw2lcuIuSoFSb8E4MD8DWQ=";
+                  };
+                };
+              })
               (overlayFor (p: p.${stdenv}))
             ];
           };

packaging/dev-shell.nix

@@ -112,7 +112,10 @@ in {
     # TODO: Remove the darwin check once
     # https://github.com/NixOS/nixpkgs/pull/291814 is available
     ++ lib.optional (stdenv.cc.isClang && !stdenv.buildPlatform.isDarwin) pkgs.buildPackages.bear
-    ++ lib.optional (stdenv.cc.isClang && stdenv.hostPlatform == stdenv.buildPlatform) (lib.hiPrio pkgs.buildPackages.clang-tools);
+    ++ lib.optional (stdenv.cc.isClang && stdenv.hostPlatform == stdenv.buildPlatform) [
+      (lib.hiPrio pkgs.buildPackages.clang-tools_18)
+      pkgs.buildPackages.llvmPackages_18.lldb
+    ];
 
   buildInputs = attrs.buildInputs or []
     ++ pkgs.nixComponents.nix-util.buildInputs

src/libcmd/repl.cc

@@ -741,7 +741,7 @@ void NixRepl::loadFlake(const std::string & flakeRefS)
 
 void NixRepl::initEnv()
 {
-    env = &state->allocEnv(envSize);
+    env = state->allocEnv(envSize);
     env->up = &state->baseEnv;
     displ = 0;
     staticEnv->vars.clear();

src/libexpr-c/nix_api_expr_internal.h

@@ -19,11 +19,6 @@ struct BindingsBuilder
     nix::BindingsBuilder builder;
 };
 
-struct ListBuilder
-{
-    nix::ListBuilder builder;
-};
-
 struct nix_value
 {
     nix::Value value;

src/libexpr-c/nix_api_value.cc

@@ -324,7 +324,7 @@ 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);
-        auto * p = v.listElems()[ix];
+        auto * p = v.list()[ix];
         nix_gc_incref(nullptr, p);
         if (p != nullptr)
             state->state.forceValue(*p, nix::noPos);
@@ -490,53 +490,6 @@ nix_err nix_init_external(nix_c_context * context, nix_value * value, ExternalVa
     NIXC_CATCH_ERRS
 }
 
-ListBuilder * nix_make_list_builder(nix_c_context * context, EvalState * state, size_t capacity)
-{
-    if (context)
-        context->last_err_code = NIX_OK;
-    try {
-        auto builder = state->state.buildList(capacity);
-        return new
-#if HAVE_BOEHMGC
-            (NoGC)
-#endif
-                ListBuilder{std::move(builder)};
-    }
-    NIXC_CATCH_ERRS_NULL
-}
-
-nix_err
-nix_list_builder_insert(nix_c_context * context, ListBuilder * list_builder, unsigned int index, nix_value * value)
-{
-    if (context)
-        context->last_err_code = NIX_OK;
-    try {
-        auto & e = check_value_not_null(value);
-        list_builder->builder[index] = &e;
-    }
-    NIXC_CATCH_ERRS
-}
-
-void nix_list_builder_free(ListBuilder * list_builder)
-{
-#if HAVE_BOEHMGC
-    GC_FREE(list_builder);
-#else
-    delete list_builder;
-#endif
-}
-
-nix_err nix_make_list(nix_c_context * context, ListBuilder * list_builder, nix_value * value)
-{
-    if (context)
-        context->last_err_code = NIX_OK;
-    try {
-        auto & v = check_value_out(value);
-        v.mkList(list_builder->builder);
-    }
-    NIXC_CATCH_ERRS
-}
-
 nix_err nix_init_primop(nix_c_context * context, nix_value * value, PrimOp * p)
 {
     if (context)

src/libexpr-c/nix_api_value.h

@@ -50,15 +50,6 @@ typedef struct EvalState EvalState;
  */
 typedef struct BindingsBuilder BindingsBuilder;
 
-/** @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
  *
@@ -393,39 +384,6 @@ nix_err nix_init_apply(nix_c_context * context, nix_value * value, nix_value * f
  */
 nix_err nix_init_external(nix_c_context * context, nix_value * value, ExternalValue * val);
 
-/** @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
- * @return error code, NIX_OK on success.
- */
-nix_err nix_make_list(nix_c_context * context, ListBuilder * list_builder, nix_value * value);
-
-/** @brief Create a list builder
- * @param[out] context Optional, stores error information
- * @param[in] state nix evaluator state
- * @param[in] capacity how many bindings you'll add. Don't exceed.
- * @return owned reference to a list builder. Make sure to unref when you're done.
- */
-ListBuilder * nix_make_list_builder(nix_c_context * context, EvalState * state, size_t capacity);
-
-/** @brief Insert bindings into a builder
- * @param[out] context Optional, stores error information
- * @param[in] list_builder ListBuilder to insert into
- * @param[in] index index to manipulate
- * @param[in] value value to insert
- * @return error code, NIX_OK on success.
- */
-nix_err
-nix_list_builder_insert(nix_c_context * context, ListBuilder * list_builder, unsigned int index, nix_value * value);
-
-/** @brief Free a list builder
- *
- * Does not fail.
- * @param[in] list_builder The builder to free.
- */
-void nix_list_builder_free(ListBuilder * list_builder);
-
 /** @brief Create an attribute set from a bindings builder
  * @param[out] context Optional, stores error information
  * @param[out] value Nix value to modify

src/libexpr-tests/error_traces.cc

@@ -708,11 +708,11 @@ namespace nix {
         ASSERT_TRACE2("head 1",
                       TypeError,
                       HintFmt("expected a list but found %s: %s", "an integer", Uncolored(ANSI_CYAN "1" ANSI_NORMAL)),
-                      HintFmt("while evaluating the first argument passed to builtins.elemAt"));
+                      HintFmt("while evaluating the first argument passed to builtins.head"));
 
         ASSERT_TRACE1("head []",
                       Error,
-                      HintFmt("list index %d is out of bounds", 0));
+                      HintFmt("'head' called on an empty list"));
 
     }
 

src/libexpr-tests/meson.build

@@ -2,6 +2,8 @@ project('nix-expr-tests', 'cpp',
   version : files('.version'),
   default_options : [
     'cpp_std=c++2a',
+    # Set -Wno-array-bounds to avoid warnings in Immer's generated code.
+    'cpp_args=-Warray-bounds=0',
     # TODO(Qyriad): increase the warning level
     'warning_level=1',
     'debug=true',

src/libexpr-tests/nix_api_value.cc

@@ -143,35 +143,6 @@ TEST_F(nix_api_expr_test, nix_build_and_init_list_invalid)
     assert_ctx_err();
 }
 
-TEST_F(nix_api_expr_test, nix_build_and_init_list)
-{
-    int size = 10;
-    ListBuilder * builder = nix_make_list_builder(ctx, state, size);
-
-    nix_value * intValue = nix_alloc_value(ctx, state);
-    nix_value * intValue2 = nix_alloc_value(ctx, state);
-
-    // `init` and `insert` can be called in any order
-    nix_init_int(ctx, intValue, 42);
-    nix_list_builder_insert(ctx, builder, 0, intValue);
-    nix_list_builder_insert(ctx, builder, 1, intValue2);
-    nix_init_int(ctx, intValue2, 43);
-
-    nix_make_list(ctx, builder, value);
-    nix_list_builder_free(builder);
-
-    ASSERT_EQ(42, nix_get_int(ctx, nix_get_list_byidx(ctx, value, state, 0)));
-    ASSERT_EQ(43, nix_get_int(ctx, nix_get_list_byidx(ctx, value, state, 1)));
-    ASSERT_EQ(nullptr, nix_get_list_byidx(ctx, value, state, 2));
-    ASSERT_EQ(10, nix_get_list_size(ctx, value));
-
-    ASSERT_STREQ("a list", nix_get_typename(ctx, value));
-    ASSERT_EQ(NIX_TYPE_LIST, nix_get_type(ctx, value));
-
-    // Clean up
-    nix_gc_decref(ctx, intValue);
-}
-
 TEST_F(nix_api_expr_test, nix_build_and_init_attr_invalid)
 {
     ASSERT_EQ(nullptr, nix_get_attr_byname(ctx, nullptr, state, 0));