Skip to content

Commit

Permalink
specialize ExprOpConcatLists::eval; introduce EvalState::allocListFro…
Browse files Browse the repository at this point in the history
…mInitializerList
  • Loading branch information
ConnorBaker committed Oct 30, 2024
1 parent a865b55 commit 2ee7758
Show file tree
Hide file tree
Showing 5 changed files with 23 additions and 8 deletions.
7 changes: 7 additions & 0 deletions src/libexpr/eval-inline.hh
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,13 @@ ValueList * EvalState::allocList()
return new (allocBytes(sizeof(ValueList))) ValueList();
}

template <typename T>
[[gnu::always_inline]]
ValueList * EvalState::allocListFromInitializerList(std::initializer_list<T> values)
{
return new (allocBytes(sizeof(ValueList))) ValueList(values);
}


[[gnu::always_inline]]
Value * EvalState::allocValue()
Expand Down
16 changes: 10 additions & 6 deletions src/libexpr/eval.cc
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
#include "fetch-to-store.hh"
#include "tarball.hh"
#include "parser-tab.hh"
#include "value.hh"

#include <algorithm>
#include <iostream>
Expand Down Expand Up @@ -1294,6 +1295,10 @@ void ExprLet::eval(EvalState & state, Env & env, Value & v)

void ExprList::eval(EvalState & state, Env & env, Value & v)
{
// TODO(@connorbaker): Tried to switch to using transient here, but started getting this error:
// Value::type: invalid internal type: -183796896
// That's an indication the value is being used after it's been freed. Not sure why that's happening.
// NOTE: Running with GC_DONT_GC=1 doesn't seem to trigger the error, so it's likely a GC issue.
auto list = state.allocList();
for (auto & i : elems)
*list = list->push_back(i->maybeThunk(state, env));
Expand Down Expand Up @@ -1896,15 +1901,14 @@ void ExprOpConcatLists::eval(EvalState & state, Env & env, Value & v)
{
auto v1 = state.allocValue();
e1->eval(state, env, *v1);
state.forceList(*v1, pos, "in the left operand of the list concatenation operator");
auto v2 = state.allocValue();
e2->eval(state, env, *v2);
// TODO(@connorbaker): This kills me -- why do we need to create the list on the heap? Shouldn't I be able to pass
// a ValueList by value to concatLists without worrying about it being garbage collected *while the function is running*?
// If this doesn't work, then should I allocList in the test cases?
state.forceList(*v2, pos, "in the right operand of the list concatenation operator");

auto list = state.allocList();
*list = list->push_back(v1);
*list = list->push_back(v2);
state.concatLists(v, *list, pos, "while evaluating one of the elements to concatenate");
*list = v1->list() + v2->list();
v.mkList(list);
}


Expand Down
2 changes: 2 additions & 0 deletions src/libexpr/eval.hh
Original file line number Diff line number Diff line change
Expand Up @@ -699,6 +699,8 @@ public:
* Allocation primitives.
*/
inline ValueList * allocList();
template <typename T>
inline ValueList * allocListFromInitializerList(std::initializer_list<T> values);
inline Value * allocValue();
inline Env & allocEnv(size_t size);

Expand Down
4 changes: 3 additions & 1 deletion src/libexpr/value.hh
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
#include <immer/memory_policy.hpp>
#include <immer/refcount/no_refcount_policy.hpp>
#include <immer/flex_vector.hpp>
#include <immer/flex_vector_transient.hpp>

// declare a memory policy for using a tracing garbage collector
using gc_policy = immer::memory_policy<immer::heap_policy<immer::gc_heap>,
Expand All @@ -33,7 +34,8 @@ class BindingsBuilder;

// alias the vector type so we are not concerned about memory policies
// in the places where we actually use it
typedef immer::flex_vector<nix::Value *, gc_policy> ValueList;
typedef immer::flex_vector<nix::Value *, gc_policy, 5U, 5U> ValueList;
typedef immer::flex_vector_transient<nix::Value *, gc_policy, 5U, 5U> ValueListTransient;

typedef enum {
tUninitialized = 0,
Expand Down
2 changes: 1 addition & 1 deletion tests/functional/lang/eval-fail-list.err.exp
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
error:
while evaluating one of the elements to concatenate
in the left operand of the list concatenation operator
at /pwd/lang/eval-fail-list.nix:1:2:
1| 8++1
| ^
Expand Down

0 comments on commit 2ee7758

Please sign in to comment.