Skip to content

Commit

Permalink
remove transients for now; all tests pass
Browse files Browse the repository at this point in the history
  • Loading branch information
ConnorBaker committed Oct 30, 2024
1 parent 8fe7f54 commit 437f92b
Show file tree
Hide file tree
Showing 12 changed files with 77 additions and 156 deletions.
14 changes: 7 additions & 7 deletions src/libexpr-tests/value/print.cc
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ TEST_F(ValuePrintingTests, tList)

// TODO(@connorbaker): Test fails because previously, using ListBuilder and over-allocating would set entries to nullptr.
// That's no longer the case with persistent vectors.
auto list = Value::List({ &vOne, &vTwo, nullptr });
auto list = ValueList({ &vOne, &vTwo, nullptr });

Value vList;
vList.mkList(&list);
Expand Down Expand Up @@ -250,7 +250,7 @@ TEST_F(ValuePrintingTests, depthList)
Value vNested;
vNested.mkAttrs(builder2.finish());

auto list = Value::List({ &vOne, &vTwo, &vNested });
auto list = ValueList({ &vOne, &vTwo, &vNested });
Value vList;
vList.mkList(&list);

Expand Down Expand Up @@ -539,7 +539,7 @@ TEST_F(ValuePrintingTests, ansiColorsList)

// TODO(@connorbaker): Test fails because previously, using ListBuilder and over-allocating would set entries to nullptr.
// That's no longer the case with persistent vectors.
auto list = Value::List({ &vOne, &vTwo, nullptr });
auto list = ValueList({ &vOne, &vTwo, nullptr });
Value vList;
vList.mkList(&list);

Expand Down Expand Up @@ -668,7 +668,7 @@ TEST_F(ValuePrintingTests, ansiColorsListRepeated)
Value vEmpty;
vEmpty.mkAttrs(emptyBuilder.finish());

auto list = Value::List({ &vEmpty, &vEmpty });
auto list = ValueList({ &vEmpty, &vEmpty });
Value vList;
vList.mkList(&list);

Expand All @@ -686,7 +686,7 @@ TEST_F(ValuePrintingTests, listRepeated)
Value vEmpty;
vEmpty.mkAttrs(emptyBuilder.finish());

auto list = Value::List({ &vEmpty, &vEmpty });
auto list = ValueList({ &vEmpty, &vEmpty });
Value vList;
vList.mkList(&list);

Expand Down Expand Up @@ -745,7 +745,7 @@ TEST_F(ValuePrintingTests, ansiColorsListElided)
vTwo.mkInt(2);

{
auto list = Value::List({ &vOne, &vTwo });
auto list = ValueList({ &vOne, &vTwo });
Value vList;
vList.mkList(&list);

Expand All @@ -761,7 +761,7 @@ TEST_F(ValuePrintingTests, ansiColorsListElided)
vThree.mkInt(3);

{
auto list = Value::List({ &vOne, &vTwo, &vThree });
auto list = ValueList({ &vOne, &vTwo, &vThree });
Value vList;
vList.mkList(&list);

Expand Down
4 changes: 2 additions & 2 deletions src/libexpr/eval-inline.hh
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,9 @@ inline void * allocBytes(size_t n)
capacity. The space is implicitly reserved after the Bindings
structure. */
[[gnu::always_inline]]
Value::List * EvalState::allocList()
ValueList * EvalState::allocList()
{
return new (allocBytes(sizeof(Value::List))) Value::List();
return new (allocBytes(sizeof(ValueList))) ValueList();
}


Expand Down
10 changes: 2 additions & 8 deletions src/libexpr/eval.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1295,12 +1295,6 @@ void ExprLet::eval(EvalState & state, Env & env, Value & v)
void ExprList::eval(EvalState & state, Env & env, Value & v)
{
auto list = state.allocList();
// TODO(@connorbaker): Did removing the use of transients here resolve an error?
// auto transientList = list->transient();
// for (auto & i : elems)
// transientList.push_back(i->maybeThunk(state, env));

// *list = transientList.persistent();
for (auto & i : elems)
*list = list->push_back(i->maybeThunk(state, env));
v.mkList(list);
Expand Down Expand Up @@ -1904,11 +1898,11 @@ void ExprOpConcatLists::eval(EvalState & state, Env & env, Value & v)
e1->eval(state, env, *v1);
auto v2 = state.allocValue();
e2->eval(state, env, *v2);
state.concatLists(v, Value::List({v1, v2}), pos, "while evaluating one of the elements to concatenate");
state.concatLists(v, ValueList({v1, v2}), pos, "while evaluating one of the elements to concatenate");
}


void EvalState::concatLists(Value & v, const Value::List lists, const PosIdx pos, std::string_view errorCtx)
void EvalState::concatLists(Value & v, const ValueList lists, const PosIdx pos, std::string_view errorCtx)
{
nrListConcats++;

Expand Down
4 changes: 2 additions & 2 deletions src/libexpr/eval.hh
Original file line number Diff line number Diff line change
Expand Up @@ -698,7 +698,7 @@ public:
/**
* Allocation primitives.
*/
inline Value::List * allocList();
inline ValueList * allocList();
inline Value * allocValue();
inline Env & allocEnv(size_t size);

Expand Down Expand Up @@ -760,7 +760,7 @@ public:
const SingleDerivedPath & p,
Value & v);

void concatLists(Value & v, const Value::List lists, const PosIdx pos, std::string_view errorCtx);
void concatLists(Value & v, const ValueList lists, const PosIdx pos, std::string_view errorCtx);

/**
* Print statistics, if enabled.
Expand Down
4 changes: 1 addition & 3 deletions src/libexpr/json-to-value.cc
Original file line number Diff line number Diff line change
Expand Up @@ -59,10 +59,8 @@ class JSONSax : nlohmann::json_sax<json> {
std::unique_ptr<JSONState> resolve(EvalState & state) override
{
auto list = state.allocList();
auto transientList = list->transient();
for (auto & i : values)
transientList.push_back(i);
*list = transientList.persistent();
*list = list->push_back(i);
parent->value(state).mkList(list);
return std::move(parent);
}
Expand Down
Loading

0 comments on commit 437f92b

Please sign in to comment.