Skip to content

Commit e73fcf7

Browse files
xokdviumxokdvium
committed
libexpr: Use proxy ListView for all Value list accesses
This also makes it possible to make `payload` field private in the `ValueStorage` class template.
1 parent c39cc00 commit e73fcf7

File tree

16 files changed

+309
-116
lines changed

16 files changed

+309
-116
lines changed

‎src/libexpr-c/nix_api_value.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -324,7 +324,7 @@ nix_value * nix_get_list_byidx(nix_c_context * context, const nix_value * value,
324324
try {
325325
auto & v = check_value_in(value);
326326
assert(v.type() == nix::nList);
327-
auto * p = v.listElems()[ix];
327+
auto * p = v.listView()[ix];
328328
nix_gc_incref(nullptr, p);
329329
if (p != nullptr)
330330
state->state.forceValue(*p, nix::noPos);

‎src/libexpr-tests/primops.cc

Lines changed: 54 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -150,8 +150,8 @@ namespace nix {
150150
TEST_F(PrimOpTest, attrValues) {
151151
auto v = eval("builtins.attrValues { x = \"foo\"; a = 1; }");
152152
ASSERT_THAT(v, IsListOfSize(2));
153-
ASSERT_THAT(*v.listElems()[0], IsIntEq(1));
154-
ASSERT_THAT(*v.listElems()[1], IsStringEq("foo"));
153+
ASSERT_THAT(*v.listView()[0], IsIntEq(1));
154+
ASSERT_THAT(*v.listView()[1], IsStringEq("foo"));
155155
}
156156

157157
TEST_F(PrimOpTest, getAttr) {
@@ -250,8 +250,8 @@ namespace nix {
250250
TEST_F(PrimOpTest, catAttrs) {
251251
auto v = eval("builtins.catAttrs \"a\" [{a = 1;} {b = 0;} {a = 2;}]");
252252
ASSERT_THAT(v, IsListOfSize(2));
253-
ASSERT_THAT(*v.listElems()[0], IsIntEq(1));
254-
ASSERT_THAT(*v.listElems()[1], IsIntEq(2));
253+
ASSERT_THAT(*v.listView()[0], IsIntEq(1));
254+
ASSERT_THAT(*v.listView()[1], IsIntEq(2));
255255
}
256256

257257
TEST_F(PrimOpTest, functionArgs) {
@@ -320,7 +320,8 @@ namespace nix {
320320
TEST_F(PrimOpTest, tail) {
321321
auto v = eval("builtins.tail [ 3 2 1 0 ]");
322322
ASSERT_THAT(v, IsListOfSize(3));
323-
for (const auto [n, elem] : enumerate(v.listItems()))
323+
auto listView = v.listView();
324+
for (const auto [n, elem] : enumerate(listView))
324325
ASSERT_THAT(*elem, IsIntEq(2 - static_cast<int>(n)));
325326
}
326327

@@ -331,17 +332,17 @@ namespace nix {
331332
TEST_F(PrimOpTest, map) {
332333
auto v = eval("map (x: \"foo\" + x) [ \"bar\" \"bla\" \"abc\" ]");
333334
ASSERT_THAT(v, IsListOfSize(3));
334-
auto elem = v.listElems()[0];
335+
auto elem = v.listView()[0];
335336
ASSERT_THAT(*elem, IsThunk());
336337
state.forceValue(*elem, noPos);
337338
ASSERT_THAT(*elem, IsStringEq("foobar"));
338339

339-
elem = v.listElems()[1];
340+
elem = v.listView()[1];
340341
ASSERT_THAT(*elem, IsThunk());
341342
state.forceValue(*elem, noPos);
342343
ASSERT_THAT(*elem, IsStringEq("foobla"));
343344

344-
elem = v.listElems()[2];
345+
elem = v.listView()[2];
345346
ASSERT_THAT(*elem, IsThunk());
346347
state.forceValue(*elem, noPos);
347348
ASSERT_THAT(*elem, IsStringEq("fooabc"));
@@ -350,7 +351,7 @@ namespace nix {
350351
TEST_F(PrimOpTest, filter) {
351352
auto v = eval("builtins.filter (x: x == 2) [ 3 2 3 2 3 2 ]");
352353
ASSERT_THAT(v, IsListOfSize(3));
353-
for (const auto elem : v.listItems())
354+
for (const auto elem : v.listView())
354355
ASSERT_THAT(*elem, IsIntEq(2));
355356
}
356357

@@ -367,7 +368,8 @@ namespace nix {
367368
TEST_F(PrimOpTest, concatLists) {
368369
auto v = eval("builtins.concatLists [[1 2] [3 4]]");
369370
ASSERT_THAT(v, IsListOfSize(4));
370-
for (const auto [i, elem] : enumerate(v.listItems()))
371+
auto listView = v.listView();
372+
for (const auto [i, elem] : enumerate(listView))
371373
ASSERT_THAT(*elem, IsIntEq(static_cast<int>(i)+1));
372374
}
373375

@@ -405,7 +407,8 @@ namespace nix {
405407
auto v = eval("builtins.genList (x: x + 1) 3");
406408
ASSERT_EQ(v.type(), nList);
407409
ASSERT_EQ(v.listSize(), 3u);
408-
for (const auto [i, elem] : enumerate(v.listItems())) {
410+
auto listView = v.listView();
411+
for (const auto [i, elem] : enumerate(listView)) {
409412
ASSERT_THAT(*elem, IsThunk());
410413
state.forceValue(*elem, noPos);
411414
ASSERT_THAT(*elem, IsIntEq(static_cast<int>(i)+1));
@@ -418,7 +421,8 @@ namespace nix {
418421
ASSERT_EQ(v.listSize(), 6u);
419422

420423
const std::vector<int> numbers = { 42, 77, 147, 249, 483, 526 };
421-
for (const auto [n, elem] : enumerate(v.listItems()))
424+
auto listView = v.listView();
425+
for (const auto [n, elem] : enumerate(listView))
422426
ASSERT_THAT(*elem, IsIntEq(numbers[n]));
423427
}
424428

@@ -429,17 +433,17 @@ namespace nix {
429433
auto right = v.attrs()->get(createSymbol("right"));
430434
ASSERT_NE(right, nullptr);
431435
ASSERT_THAT(*right->value, IsListOfSize(2));
432-
ASSERT_THAT(*right->value->listElems()[0], IsIntEq(23));
433-
ASSERT_THAT(*right->value->listElems()[1], IsIntEq(42));
436+
ASSERT_THAT(*right->value->listView()[0], IsIntEq(23));
437+
ASSERT_THAT(*right->value->listView()[1], IsIntEq(42));
434438

435439
auto wrong = v.attrs()->get(createSymbol("wrong"));
436440
ASSERT_NE(wrong, nullptr);
437441
ASSERT_EQ(wrong->value->type(), nList);
438442
ASSERT_EQ(wrong->value->listSize(), 3u);
439443
ASSERT_THAT(*wrong->value, IsListOfSize(3));
440-
ASSERT_THAT(*wrong->value->listElems()[0], IsIntEq(1));
441-
ASSERT_THAT(*wrong->value->listElems()[1], IsIntEq(9));
442-
ASSERT_THAT(*wrong->value->listElems()[2], IsIntEq(3));
444+
ASSERT_THAT(*wrong->value->listView()[0], IsIntEq(1));
445+
ASSERT_THAT(*wrong->value->listView()[1], IsIntEq(9));
446+
ASSERT_THAT(*wrong->value->listView()[2], IsIntEq(3));
443447
}
444448

445449
TEST_F(PrimOpTest, concatMap) {
@@ -448,7 +452,8 @@ namespace nix {
448452
ASSERT_EQ(v.listSize(), 6u);
449453

450454
const std::vector<int> numbers = { 1, 2, 0, 3, 4, 0 };
451-
for (const auto [n, elem] : enumerate(v.listItems()))
455+
auto listView = v.listView();
456+
for (const auto [n, elem] : enumerate(listView))
452457
ASSERT_THAT(*elem, IsIntEq(numbers[n]));
453458
}
454459

@@ -682,7 +687,8 @@ namespace nix {
682687
ASSERT_THAT(v, IsListOfSize(4));
683688

684689
const std::vector<std::string_view> strings = { "1", "2", "3", "git" };
685-
for (const auto [n, p] : enumerate(v.listItems()))
690+
auto listView = v.listView();
691+
for (const auto [n, p] : enumerate(listView))
686692
ASSERT_THAT(*p, IsStringEq(strings[n]));
687693
}
688694

@@ -772,67 +778,67 @@ namespace nix {
772778
auto v = eval("builtins.split \"(a)b\" \"abc\"");
773779
ASSERT_THAT(v, IsListOfSize(3));
774780

775-
ASSERT_THAT(*v.listElems()[0], IsStringEq(""));
781+
ASSERT_THAT(*v.listView()[0], IsStringEq(""));
776782

777-
ASSERT_THAT(*v.listElems()[1], IsListOfSize(1));
778-
ASSERT_THAT(*v.listElems()[1]->listElems()[0], IsStringEq("a"));
783+
ASSERT_THAT(*v.listView()[1], IsListOfSize(1));
784+
ASSERT_THAT(*v.listView()[1]->listView()[0], IsStringEq("a"));
779785

780-
ASSERT_THAT(*v.listElems()[2], IsStringEq("c"));
786+
ASSERT_THAT(*v.listView()[2], IsStringEq("c"));
781787
}
782788

783789
TEST_F(PrimOpTest, split2) {
784790
// v is expected to be a list [ "" [ "a" ] "b" [ "c"] "" ]
785791
auto v = eval("builtins.split \"([ac])\" \"abc\"");
786792
ASSERT_THAT(v, IsListOfSize(5));
787793

788-
ASSERT_THAT(*v.listElems()[0], IsStringEq(""));
794+
ASSERT_THAT(*v.listView()[0], IsStringEq(""));
789795

790-
ASSERT_THAT(*v.listElems()[1], IsListOfSize(1));
791-
ASSERT_THAT(*v.listElems()[1]->listElems()[0], IsStringEq("a"));
796+
ASSERT_THAT(*v.listView()[1], IsListOfSize(1));
797+
ASSERT_THAT(*v.listView()[1]->listView()[0], IsStringEq("a"));
792798

793-
ASSERT_THAT(*v.listElems()[2], IsStringEq("b"));
799+
ASSERT_THAT(*v.listView()[2], IsStringEq("b"));
794800

795-
ASSERT_THAT(*v.listElems()[3], IsListOfSize(1));
796-
ASSERT_THAT(*v.listElems()[3]->listElems()[0], IsStringEq("c"));
801+
ASSERT_THAT(*v.listView()[3], IsListOfSize(1));
802+
ASSERT_THAT(*v.listView()[3]->listView()[0], IsStringEq("c"));
797803

798-
ASSERT_THAT(*v.listElems()[4], IsStringEq(""));
804+
ASSERT_THAT(*v.listView()[4], IsStringEq(""));
799805
}
800806

801807
TEST_F(PrimOpTest, split3) {
802808
auto v = eval("builtins.split \"(a)|(c)\" \"abc\"");
803809
ASSERT_THAT(v, IsListOfSize(5));
804810

805811
// First list element
806-
ASSERT_THAT(*v.listElems()[0], IsStringEq(""));
812+
ASSERT_THAT(*v.listView()[0], IsStringEq(""));
807813

808814
// 2nd list element is a list [ "" null ]
809-
ASSERT_THAT(*v.listElems()[1], IsListOfSize(2));
810-
ASSERT_THAT(*v.listElems()[1]->listElems()[0], IsStringEq("a"));
811-
ASSERT_THAT(*v.listElems()[1]->listElems()[1], IsNull());
815+
ASSERT_THAT(*v.listView()[1], IsListOfSize(2));
816+
ASSERT_THAT(*v.listView()[1]->listView()[0], IsStringEq("a"));
817+
ASSERT_THAT(*v.listView()[1]->listView()[1], IsNull());
812818

813819
// 3rd element
814-
ASSERT_THAT(*v.listElems()[2], IsStringEq("b"));
820+
ASSERT_THAT(*v.listView()[2], IsStringEq("b"));
815821

816822
// 4th element is a list: [ null "c" ]
817-
ASSERT_THAT(*v.listElems()[3], IsListOfSize(2));
818-
ASSERT_THAT(*v.listElems()[3]->listElems()[0], IsNull());
819-
ASSERT_THAT(*v.listElems()[3]->listElems()[1], IsStringEq("c"));
823+
ASSERT_THAT(*v.listView()[3], IsListOfSize(2));
824+
ASSERT_THAT(*v.listView()[3]->listView()[0], IsNull());
825+
ASSERT_THAT(*v.listView()[3]->listView()[1], IsStringEq("c"));
820826

821827
// 5th element is the empty string
822-
ASSERT_THAT(*v.listElems()[4], IsStringEq(""));
828+
ASSERT_THAT(*v.listView()[4], IsStringEq(""));
823829
}
824830

825831
TEST_F(PrimOpTest, split4) {
826832
auto v = eval("builtins.split \"([[:upper:]]+)\" \" FOO \"");
827833
ASSERT_THAT(v, IsListOfSize(3));
828-
auto first = v.listElems()[0];
829-
auto second = v.listElems()[1];
830-
auto third = v.listElems()[2];
834+
auto first = v.listView()[0];
835+
auto second = v.listView()[1];
836+
auto third = v.listView()[2];
831837

832838
ASSERT_THAT(*first, IsStringEq(" "));
833839

834840
ASSERT_THAT(*second, IsListOfSize(1));
835-
ASSERT_THAT(*second->listElems()[0], IsStringEq("FOO"));
841+
ASSERT_THAT(*second->listView()[0], IsStringEq("FOO"));
836842

837843
ASSERT_THAT(*third, IsStringEq(" "));
838844
}
@@ -850,14 +856,14 @@ namespace nix {
850856
TEST_F(PrimOpTest, match3) {
851857
auto v = eval("builtins.match \"a(b)(c)\" \"abc\"");
852858
ASSERT_THAT(v, IsListOfSize(2));
853-
ASSERT_THAT(*v.listElems()[0], IsStringEq("b"));
854-
ASSERT_THAT(*v.listElems()[1], IsStringEq("c"));
859+
ASSERT_THAT(*v.listView()[0], IsStringEq("b"));
860+
ASSERT_THAT(*v.listView()[1], IsStringEq("c"));
855861
}
856862

857863
TEST_F(PrimOpTest, match4) {
858864
auto v = eval("builtins.match \"[[:space:]]+([[:upper:]]+)[[:space:]]+\" \" FOO \"");
859865
ASSERT_THAT(v, IsListOfSize(1));
860-
ASSERT_THAT(*v.listElems()[0], IsStringEq("FOO"));
866+
ASSERT_THAT(*v.listView()[0], IsStringEq("FOO"));
861867
}
862868

863869
TEST_F(PrimOpTest, match5) {
@@ -874,7 +880,8 @@ namespace nix {
874880

875881
// ensure that the list is sorted
876882
const std::vector<std::string_view> expected { "a", "x", "y", "z" };
877-
for (const auto [n, elem] : enumerate(v.listItems()))
883+
auto listView = v.listView();
884+
for (const auto [n, elem] : enumerate(listView))
878885
ASSERT_THAT(*elem, IsStringEq(expected[n]));
879886
}
880887

‎src/libexpr/attr-path.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ std::pair<Value *, PosIdx> findAlongAttrPath(EvalState & state, const std::strin
9595
if (*attrIndex >= v->listSize())
9696
throw AttrPathNotFound("list index %1% in selection path '%2%' is out of range", *attrIndex, attrPath);
9797

98-
v = v->listElems()[*attrIndex];
98+
v = v->listView()[*attrIndex];
9999
pos = noPos;
100100
}
101101

‎src/libexpr/eval-cache.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -721,7 +721,7 @@ std::vector<std::string> AttrCursor::getListOfStrings()
721721

722722
std::vector<std::string> res;
723723

724-
for (auto & elem : v.listItems())
724+
for (auto elem : v.listView())
725725
res.push_back(std::string(root->state.forceStringNoCtx(*elem, noPos, "while evaluating an attribute for caching")));
726726

727727
if (root->db)

‎src/libexpr/eval.cc

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2007,9 +2007,10 @@ void EvalState::concatLists(Value & v, size_t nrLists, Value * const * lists, co
20072007
auto list = buildList(len);
20082008
auto out = list.elems;
20092009
for (size_t n = 0, pos = 0; n < nrLists; ++n) {
2010-
auto l = lists[n]->listSize();
2010+
auto listView = lists[n]->listView();
2011+
auto l = listView.size();
20112012
if (l)
2012-
memcpy(out + pos, lists[n]->listElems(), l * sizeof(Value *));
2013+
memcpy(out + pos, listView.data(), l * sizeof(Value *));
20132014
pos += l;
20142015
}
20152016
v.mkList(list);
@@ -2174,7 +2175,7 @@ void EvalState::forceValueDeep(Value & v)
21742175
}
21752176

21762177
else if (v.isList()) {
2177-
for (auto v2 : v.listItems())
2178+
for (auto v2 : v.listView())
21782179
recurse(*v2);
21792180
}
21802181
};
@@ -2411,7 +2412,8 @@ BackedStringView EvalState::coerceToString(
24112412

24122413
if (v.isList()) {
24132414
std::string result;
2414-
for (auto [n, v2] : enumerate(v.listItems())) {
2415+
auto listView = v.listView();
2416+
for (auto [n, v2] : enumerate(listView)) {
24152417
try {
24162418
result += *coerceToString(pos, *v2, context,
24172419
"while evaluating one element of the list",
@@ -2666,7 +2668,7 @@ void EvalState::assertEqValues(Value & v1, Value & v2, const PosIdx pos, std::st
26662668
}
26672669
for (size_t n = 0; n < v1.listSize(); ++n) {
26682670
try {
2669-
assertEqValues(*v1.listElems()[n], *v2.listElems()[n], pos, errorCtx);
2671+
assertEqValues(*v1.listView()[n], *v2.listView()[n], pos, errorCtx);
26702672
} catch (Error & e) {
26712673
e.addTrace(positions[pos], "while comparing list element %d", n);
26722674
throw;
@@ -2818,7 +2820,7 @@ bool EvalState::eqValues(Value & v1, Value & v2, const PosIdx pos, std::string_v
28182820
case nList:
28192821
if (v1.listSize() != v2.listSize()) return false;
28202822
for (size_t n = 0; n < v1.listSize(); ++n)
2821-
if (!eqValues(*v1.listElems()[n], *v2.listElems()[n], pos, errorCtx)) return false;
2823+
if (!eqValues(*v1.listView()[n], *v2.listView()[n], pos, errorCtx)) return false;
28222824
return true;
28232825

28242826
case nAttrs: {

‎src/libexpr/get-drvs.cc

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,7 @@ PackageInfo::Outputs PackageInfo::queryOutputs(bool withPaths, bool onlyOutputsT
117117
state->forceList(*i->value, i->pos, "while evaluating the 'outputs' attribute of a derivation");
118118

119119
/* For each output... */
120-
for (auto elem : i->value->listItems()) {
120+
for (auto elem : i->value->listView()) {
121121
std::string output(state->forceStringNoCtx(*elem, i->pos, "while evaluating the name of an output of a derivation"));
122122

123123
if (withPaths) {
@@ -159,7 +159,7 @@ PackageInfo::Outputs PackageInfo::queryOutputs(bool withPaths, bool onlyOutputsT
159159
/* ^ this shows during `nix-env -i` right under the bad derivation */
160160
if (!outTI->isList()) throw errMsg;
161161
Outputs result;
162-
for (auto elem : outTI->listItems()) {
162+
for (auto elem : outTI->listView()) {
163163
if (elem->type() != nString) throw errMsg;
164164
auto out = outputs.find(elem->c_str());
165165
if (out == outputs.end()) throw errMsg;
@@ -206,7 +206,7 @@ bool PackageInfo::checkMeta(Value & v)
206206
{
207207
state->forceValue(v, v.determinePos(noPos));
208208
if (v.type() == nList) {
209-
for (auto elem : v.listItems())
209+
for (auto elem : v.listView())
210210
if (!checkMeta(*elem)) return false;
211211
return true;
212212
}
@@ -400,7 +400,8 @@ static void getDerivations(EvalState & state, Value & vIn,
400400
}
401401

402402
else if (v.type() == nList) {
403-
for (auto [n, elem] : enumerate(v.listItems())) {
403+
auto listView = v.listView();
404+
for (auto [n, elem] : enumerate(listView)) {
404405
std::string pathPrefix2 = addToPath(pathPrefix, fmt("%d", n));
405406
if (getDerivation(state, *elem, pathPrefix2, drvs, done, ignoreAssertionFailures))
406407
getDerivations(state, *elem, pathPrefix2, autoArgs, drvs, done, ignoreAssertionFailures);

0 commit comments

Comments
 (0)