From e73fcf7b5300c045a60e42c63e6d445b445426c4 Mon Sep 17 00:00:00 2001 From: Sergei Zimmerman Date: Wed, 2 Jul 2025 21:57:02 +0300 Subject: [PATCH] libexpr: Use proxy ListView for all Value list accesses This also makes it possible to make `payload` field private in the `ValueStorage` class template. --- src/libexpr-c/nix_api_value.cc | 2 +- src/libexpr-tests/primops.cc | 101 +++++++------ src/libexpr/attr-path.cc | 2 +- src/libexpr/eval-cache.cc | 2 +- src/libexpr/eval.cc | 14 +- src/libexpr/get-drvs.cc | 9 +- src/libexpr/include/nix/expr/value.hh | 199 ++++++++++++++++++++++++-- src/libexpr/primops.cc | 72 +++++----- src/libexpr/primops/context.cc | 2 +- src/libexpr/print-ambiguous.cc | 6 +- src/libexpr/print.cc | 4 +- src/libexpr/value-to-json.cc | 2 +- src/libexpr/value-to-xml.cc | 2 +- src/libflake/flake.cc | 2 +- src/nix-env/nix-env.cc | 2 +- src/nix/prefetch.cc | 4 +- 16 files changed, 309 insertions(+), 116 deletions(-) diff --git a/src/libexpr-c/nix_api_value.cc b/src/libexpr-c/nix_api_value.cc index 8afe35a4b..fb90e2872 100644 --- a/src/libexpr-c/nix_api_value.cc +++ b/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.listView()[ix]; nix_gc_incref(nullptr, p); if (p != nullptr) state->state.forceValue(*p, nix::noPos); diff --git a/src/libexpr-tests/primops.cc b/src/libexpr-tests/primops.cc index 6c301f157..9b5590d8d 100644 --- a/src/libexpr-tests/primops.cc +++ b/src/libexpr-tests/primops.cc @@ -150,8 +150,8 @@ namespace nix { TEST_F(PrimOpTest, attrValues) { auto v = eval("builtins.attrValues { x = \"foo\"; a = 1; }"); ASSERT_THAT(v, IsListOfSize(2)); - ASSERT_THAT(*v.listElems()[0], IsIntEq(1)); - ASSERT_THAT(*v.listElems()[1], IsStringEq("foo")); + ASSERT_THAT(*v.listView()[0], IsIntEq(1)); + ASSERT_THAT(*v.listView()[1], IsStringEq("foo")); } TEST_F(PrimOpTest, getAttr) { @@ -250,8 +250,8 @@ namespace nix { TEST_F(PrimOpTest, catAttrs) { auto v = eval("builtins.catAttrs \"a\" [{a = 1;} {b = 0;} {a = 2;}]"); ASSERT_THAT(v, IsListOfSize(2)); - ASSERT_THAT(*v.listElems()[0], IsIntEq(1)); - ASSERT_THAT(*v.listElems()[1], IsIntEq(2)); + ASSERT_THAT(*v.listView()[0], IsIntEq(1)); + ASSERT_THAT(*v.listView()[1], IsIntEq(2)); } TEST_F(PrimOpTest, functionArgs) { @@ -320,7 +320,8 @@ namespace nix { TEST_F(PrimOpTest, tail) { auto v = eval("builtins.tail [ 3 2 1 0 ]"); ASSERT_THAT(v, IsListOfSize(3)); - for (const auto [n, elem] : enumerate(v.listItems())) + auto listView = v.listView(); + for (const auto [n, elem] : enumerate(listView)) ASSERT_THAT(*elem, IsIntEq(2 - static_cast(n))); } @@ -331,17 +332,17 @@ namespace nix { TEST_F(PrimOpTest, map) { auto v = eval("map (x: \"foo\" + x) [ \"bar\" \"bla\" \"abc\" ]"); ASSERT_THAT(v, IsListOfSize(3)); - auto elem = v.listElems()[0]; + auto elem = v.listView()[0]; ASSERT_THAT(*elem, IsThunk()); state.forceValue(*elem, noPos); ASSERT_THAT(*elem, IsStringEq("foobar")); - elem = v.listElems()[1]; + elem = v.listView()[1]; ASSERT_THAT(*elem, IsThunk()); state.forceValue(*elem, noPos); ASSERT_THAT(*elem, IsStringEq("foobla")); - elem = v.listElems()[2]; + elem = v.listView()[2]; ASSERT_THAT(*elem, IsThunk()); state.forceValue(*elem, noPos); ASSERT_THAT(*elem, IsStringEq("fooabc")); @@ -350,7 +351,7 @@ namespace nix { TEST_F(PrimOpTest, filter) { auto v = eval("builtins.filter (x: x == 2) [ 3 2 3 2 3 2 ]"); ASSERT_THAT(v, IsListOfSize(3)); - for (const auto elem : v.listItems()) + for (const auto elem : v.listView()) ASSERT_THAT(*elem, IsIntEq(2)); } @@ -367,7 +368,8 @@ namespace nix { TEST_F(PrimOpTest, concatLists) { auto v = eval("builtins.concatLists [[1 2] [3 4]]"); ASSERT_THAT(v, IsListOfSize(4)); - for (const auto [i, elem] : enumerate(v.listItems())) + auto listView = v.listView(); + for (const auto [i, elem] : enumerate(listView)) ASSERT_THAT(*elem, IsIntEq(static_cast(i)+1)); } @@ -405,7 +407,8 @@ namespace nix { auto v = eval("builtins.genList (x: x + 1) 3"); ASSERT_EQ(v.type(), nList); ASSERT_EQ(v.listSize(), 3u); - for (const auto [i, elem] : enumerate(v.listItems())) { + auto listView = v.listView(); + for (const auto [i, elem] : enumerate(listView)) { ASSERT_THAT(*elem, IsThunk()); state.forceValue(*elem, noPos); ASSERT_THAT(*elem, IsIntEq(static_cast(i)+1)); @@ -418,7 +421,8 @@ namespace nix { ASSERT_EQ(v.listSize(), 6u); const std::vector numbers = { 42, 77, 147, 249, 483, 526 }; - for (const auto [n, elem] : enumerate(v.listItems())) + auto listView = v.listView(); + for (const auto [n, elem] : enumerate(listView)) ASSERT_THAT(*elem, IsIntEq(numbers[n])); } @@ -429,17 +433,17 @@ namespace nix { auto right = v.attrs()->get(createSymbol("right")); ASSERT_NE(right, nullptr); ASSERT_THAT(*right->value, IsListOfSize(2)); - ASSERT_THAT(*right->value->listElems()[0], IsIntEq(23)); - ASSERT_THAT(*right->value->listElems()[1], IsIntEq(42)); + ASSERT_THAT(*right->value->listView()[0], IsIntEq(23)); + ASSERT_THAT(*right->value->listView()[1], IsIntEq(42)); auto wrong = v.attrs()->get(createSymbol("wrong")); ASSERT_NE(wrong, nullptr); ASSERT_EQ(wrong->value->type(), nList); ASSERT_EQ(wrong->value->listSize(), 3u); ASSERT_THAT(*wrong->value, IsListOfSize(3)); - ASSERT_THAT(*wrong->value->listElems()[0], IsIntEq(1)); - ASSERT_THAT(*wrong->value->listElems()[1], IsIntEq(9)); - ASSERT_THAT(*wrong->value->listElems()[2], IsIntEq(3)); + ASSERT_THAT(*wrong->value->listView()[0], IsIntEq(1)); + ASSERT_THAT(*wrong->value->listView()[1], IsIntEq(9)); + ASSERT_THAT(*wrong->value->listView()[2], IsIntEq(3)); } TEST_F(PrimOpTest, concatMap) { @@ -448,7 +452,8 @@ namespace nix { ASSERT_EQ(v.listSize(), 6u); const std::vector numbers = { 1, 2, 0, 3, 4, 0 }; - for (const auto [n, elem] : enumerate(v.listItems())) + auto listView = v.listView(); + for (const auto [n, elem] : enumerate(listView)) ASSERT_THAT(*elem, IsIntEq(numbers[n])); } @@ -682,7 +687,8 @@ namespace nix { ASSERT_THAT(v, IsListOfSize(4)); const std::vector strings = { "1", "2", "3", "git" }; - for (const auto [n, p] : enumerate(v.listItems())) + auto listView = v.listView(); + for (const auto [n, p] : enumerate(listView)) ASSERT_THAT(*p, IsStringEq(strings[n])); } @@ -772,12 +778,12 @@ namespace nix { auto v = eval("builtins.split \"(a)b\" \"abc\""); ASSERT_THAT(v, IsListOfSize(3)); - ASSERT_THAT(*v.listElems()[0], IsStringEq("")); + ASSERT_THAT(*v.listView()[0], IsStringEq("")); - ASSERT_THAT(*v.listElems()[1], IsListOfSize(1)); - ASSERT_THAT(*v.listElems()[1]->listElems()[0], IsStringEq("a")); + ASSERT_THAT(*v.listView()[1], IsListOfSize(1)); + ASSERT_THAT(*v.listView()[1]->listView()[0], IsStringEq("a")); - ASSERT_THAT(*v.listElems()[2], IsStringEq("c")); + ASSERT_THAT(*v.listView()[2], IsStringEq("c")); } TEST_F(PrimOpTest, split2) { @@ -785,17 +791,17 @@ namespace nix { auto v = eval("builtins.split \"([ac])\" \"abc\""); ASSERT_THAT(v, IsListOfSize(5)); - ASSERT_THAT(*v.listElems()[0], IsStringEq("")); + ASSERT_THAT(*v.listView()[0], IsStringEq("")); - ASSERT_THAT(*v.listElems()[1], IsListOfSize(1)); - ASSERT_THAT(*v.listElems()[1]->listElems()[0], IsStringEq("a")); + ASSERT_THAT(*v.listView()[1], IsListOfSize(1)); + ASSERT_THAT(*v.listView()[1]->listView()[0], IsStringEq("a")); - ASSERT_THAT(*v.listElems()[2], IsStringEq("b")); + ASSERT_THAT(*v.listView()[2], IsStringEq("b")); - ASSERT_THAT(*v.listElems()[3], IsListOfSize(1)); - ASSERT_THAT(*v.listElems()[3]->listElems()[0], IsStringEq("c")); + ASSERT_THAT(*v.listView()[3], IsListOfSize(1)); + ASSERT_THAT(*v.listView()[3]->listView()[0], IsStringEq("c")); - ASSERT_THAT(*v.listElems()[4], IsStringEq("")); + ASSERT_THAT(*v.listView()[4], IsStringEq("")); } TEST_F(PrimOpTest, split3) { @@ -803,36 +809,36 @@ namespace nix { ASSERT_THAT(v, IsListOfSize(5)); // First list element - ASSERT_THAT(*v.listElems()[0], IsStringEq("")); + ASSERT_THAT(*v.listView()[0], IsStringEq("")); // 2nd list element is a list [ "" null ] - ASSERT_THAT(*v.listElems()[1], IsListOfSize(2)); - ASSERT_THAT(*v.listElems()[1]->listElems()[0], IsStringEq("a")); - ASSERT_THAT(*v.listElems()[1]->listElems()[1], IsNull()); + ASSERT_THAT(*v.listView()[1], IsListOfSize(2)); + ASSERT_THAT(*v.listView()[1]->listView()[0], IsStringEq("a")); + ASSERT_THAT(*v.listView()[1]->listView()[1], IsNull()); // 3rd element - ASSERT_THAT(*v.listElems()[2], IsStringEq("b")); + ASSERT_THAT(*v.listView()[2], IsStringEq("b")); // 4th element is a list: [ null "c" ] - ASSERT_THAT(*v.listElems()[3], IsListOfSize(2)); - ASSERT_THAT(*v.listElems()[3]->listElems()[0], IsNull()); - ASSERT_THAT(*v.listElems()[3]->listElems()[1], IsStringEq("c")); + ASSERT_THAT(*v.listView()[3], IsListOfSize(2)); + ASSERT_THAT(*v.listView()[3]->listView()[0], IsNull()); + ASSERT_THAT(*v.listView()[3]->listView()[1], IsStringEq("c")); // 5th element is the empty string - ASSERT_THAT(*v.listElems()[4], IsStringEq("")); + ASSERT_THAT(*v.listView()[4], IsStringEq("")); } TEST_F(PrimOpTest, split4) { auto v = eval("builtins.split \"([[:upper:]]+)\" \" FOO \""); ASSERT_THAT(v, IsListOfSize(3)); - auto first = v.listElems()[0]; - auto second = v.listElems()[1]; - auto third = v.listElems()[2]; + auto first = v.listView()[0]; + auto second = v.listView()[1]; + auto third = v.listView()[2]; ASSERT_THAT(*first, IsStringEq(" ")); ASSERT_THAT(*second, IsListOfSize(1)); - ASSERT_THAT(*second->listElems()[0], IsStringEq("FOO")); + ASSERT_THAT(*second->listView()[0], IsStringEq("FOO")); ASSERT_THAT(*third, IsStringEq(" ")); } @@ -850,14 +856,14 @@ namespace nix { TEST_F(PrimOpTest, match3) { auto v = eval("builtins.match \"a(b)(c)\" \"abc\""); ASSERT_THAT(v, IsListOfSize(2)); - ASSERT_THAT(*v.listElems()[0], IsStringEq("b")); - ASSERT_THAT(*v.listElems()[1], IsStringEq("c")); + ASSERT_THAT(*v.listView()[0], IsStringEq("b")); + ASSERT_THAT(*v.listView()[1], IsStringEq("c")); } TEST_F(PrimOpTest, match4) { auto v = eval("builtins.match \"[[:space:]]+([[:upper:]]+)[[:space:]]+\" \" FOO \""); ASSERT_THAT(v, IsListOfSize(1)); - ASSERT_THAT(*v.listElems()[0], IsStringEq("FOO")); + ASSERT_THAT(*v.listView()[0], IsStringEq("FOO")); } TEST_F(PrimOpTest, match5) { @@ -874,7 +880,8 @@ namespace nix { // ensure that the list is sorted const std::vector expected { "a", "x", "y", "z" }; - for (const auto [n, elem] : enumerate(v.listItems())) + auto listView = v.listView(); + for (const auto [n, elem] : enumerate(listView)) ASSERT_THAT(*elem, IsStringEq(expected[n])); } diff --git a/src/libexpr/attr-path.cc b/src/libexpr/attr-path.cc index 722b57bbf..111d04cf2 100644 --- a/src/libexpr/attr-path.cc +++ b/src/libexpr/attr-path.cc @@ -95,7 +95,7 @@ std::pair findAlongAttrPath(EvalState & state, const std::strin if (*attrIndex >= v->listSize()) throw AttrPathNotFound("list index %1% in selection path '%2%' is out of range", *attrIndex, attrPath); - v = v->listElems()[*attrIndex]; + v = v->listView()[*attrIndex]; pos = noPos; } diff --git a/src/libexpr/eval-cache.cc b/src/libexpr/eval-cache.cc index 0d732f59c..27d60d6ef 100644 --- a/src/libexpr/eval-cache.cc +++ b/src/libexpr/eval-cache.cc @@ -721,7 +721,7 @@ std::vector AttrCursor::getListOfStrings() std::vector res; - for (auto & elem : v.listItems()) + for (auto elem : v.listView()) res.push_back(std::string(root->state.forceStringNoCtx(*elem, noPos, "while evaluating an attribute for caching"))); if (root->db) diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index fba1e6665..1321e00a5 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -2007,9 +2007,10 @@ void EvalState::concatLists(Value & v, size_t nrLists, Value * const * lists, co auto list = buildList(len); auto out = list.elems; for (size_t n = 0, pos = 0; n < nrLists; ++n) { - auto l = lists[n]->listSize(); + auto listView = lists[n]->listView(); + auto l = listView.size(); if (l) - memcpy(out + pos, lists[n]->listElems(), l * sizeof(Value *)); + memcpy(out + pos, listView.data(), l * sizeof(Value *)); pos += l; } v.mkList(list); @@ -2174,7 +2175,7 @@ void EvalState::forceValueDeep(Value & v) } else if (v.isList()) { - for (auto v2 : v.listItems()) + for (auto v2 : v.listView()) recurse(*v2); } }; @@ -2411,7 +2412,8 @@ BackedStringView EvalState::coerceToString( if (v.isList()) { std::string result; - for (auto [n, v2] : enumerate(v.listItems())) { + auto listView = v.listView(); + for (auto [n, v2] : enumerate(listView)) { try { result += *coerceToString(pos, *v2, context, "while evaluating one element of the list", @@ -2666,7 +2668,7 @@ void EvalState::assertEqValues(Value & v1, Value & v2, const PosIdx pos, std::st } for (size_t n = 0; n < v1.listSize(); ++n) { try { - assertEqValues(*v1.listElems()[n], *v2.listElems()[n], pos, errorCtx); + assertEqValues(*v1.listView()[n], *v2.listView()[n], pos, errorCtx); } catch (Error & e) { e.addTrace(positions[pos], "while comparing list element %d", n); throw; @@ -2818,7 +2820,7 @@ bool EvalState::eqValues(Value & v1, Value & v2, const PosIdx pos, std::string_v case nList: if (v1.listSize() != v2.listSize()) return false; for (size_t n = 0; n < v1.listSize(); ++n) - if (!eqValues(*v1.listElems()[n], *v2.listElems()[n], pos, errorCtx)) return false; + if (!eqValues(*v1.listView()[n], *v2.listView()[n], pos, errorCtx)) return false; return true; case nAttrs: { diff --git a/src/libexpr/get-drvs.cc b/src/libexpr/get-drvs.cc index f15ad4d73..3c9ff9ff3 100644 --- a/src/libexpr/get-drvs.cc +++ b/src/libexpr/get-drvs.cc @@ -117,7 +117,7 @@ PackageInfo::Outputs PackageInfo::queryOutputs(bool withPaths, bool onlyOutputsT state->forceList(*i->value, i->pos, "while evaluating the 'outputs' attribute of a derivation"); /* For each output... */ - for (auto elem : i->value->listItems()) { + for (auto elem : i->value->listView()) { std::string output(state->forceStringNoCtx(*elem, i->pos, "while evaluating the name of an output of a derivation")); if (withPaths) { @@ -159,7 +159,7 @@ PackageInfo::Outputs PackageInfo::queryOutputs(bool withPaths, bool onlyOutputsT /* ^ this shows during `nix-env -i` right under the bad derivation */ if (!outTI->isList()) throw errMsg; Outputs result; - for (auto elem : outTI->listItems()) { + for (auto elem : outTI->listView()) { if (elem->type() != nString) throw errMsg; auto out = outputs.find(elem->c_str()); if (out == outputs.end()) throw errMsg; @@ -206,7 +206,7 @@ bool PackageInfo::checkMeta(Value & v) { state->forceValue(v, v.determinePos(noPos)); if (v.type() == nList) { - for (auto elem : v.listItems()) + for (auto elem : v.listView()) if (!checkMeta(*elem)) return false; return true; } @@ -400,7 +400,8 @@ static void getDerivations(EvalState & state, Value & vIn, } else if (v.type() == nList) { - for (auto [n, elem] : enumerate(v.listItems())) { + auto listView = v.listView(); + for (auto [n, elem] : enumerate(listView)) { std::string pathPrefix2 = addToPath(pathPrefix, fmt("%d", n)); if (getDerivation(state, *elem, pathPrefix2, drvs, done, ignoreAssertionFailures)) getDerivations(state, *elem, pathPrefix2, autoArgs, drvs, done, ignoreAssertionFailures); diff --git a/src/libexpr/include/nix/expr/value.hh b/src/libexpr/include/nix/expr/value.hh index 642459463..3f28aca01 100644 --- a/src/libexpr/include/nix/expr/value.hh +++ b/src/libexpr/include/nix/expr/value.hh @@ -4,6 +4,7 @@ #include #include #include +#include #include "nix/expr/eval-gc.hh" #include "nix/expr/value/context.hh" @@ -317,12 +318,11 @@ protected: #undef NIX_VALUE_STORAGE_DEFINE_FIELD }; - Payload payload; - private: InternalType internalType = tUninitialized; + Payload payload; -public: +protected: #define NIX_VALUE_STORAGE_GET_IMPL(K, FIELD_NAME, DISCRIMINATOR) \ void getStorage(K & val) const noexcept \ { \ @@ -351,6 +351,189 @@ public: } }; +/** + * View into a list of Value * that is itself immutable. + * + * Since not all representations of ValueStorage can provide + * a pointer to a const array of Value * this proxy class either + * stores the small list inline or points to the big list. + */ +class ListView +{ + using SpanType = std::span; + using SmallList = detail::ValueBase::SmallList; + using List = detail::ValueBase::List; + + std::variant raw; + +public: + ListView(SmallList list) + : raw(list) + { + } + + ListView(List list) + : raw(list) + { + } + + Value * const * data() const & noexcept + { + return std::visit( + overloaded{ + [](const SmallList & list) { return list.data(); }, [](const List & list) { return list.elems; }}, + raw); + } + + std::size_t size() const noexcept + { + return std::visit( + overloaded{ + [](const SmallList & list) -> std::size_t { return list.back() == nullptr ? 1 : 2; }, + [](const List & list) -> std::size_t { return list.size; }}, + raw); + } + + Value * operator[](std::size_t i) const noexcept + { + return data()[i]; + } + + SpanType span() const & + { + return SpanType(data(), size()); + } + + /* Ensure that no dangling views can be created accidentally, as that + would lead to hard to diagnose bugs that only affect small lists. */ + SpanType span() && = delete; + Value * const * data() && noexcept = delete; + + /** + * Random-access iterator that only allows iterating over a constant range + * of mutable Value pointers. + * + * @note Not a pointer to minimize potential misuses and implicitly relying + * on the iterator being a pointer. + **/ + class iterator + { + public: + using value_type = Value *; + using pointer = const value_type *; + using reference = const value_type &; + using difference_type = std::ptrdiff_t; + using iterator_category = std::random_access_iterator_tag; + + private: + pointer ptr = nullptr; + + friend class ListView; + + iterator(pointer ptr) + : ptr(ptr) + { + } + + public: + iterator() = default; + + reference operator*() const + { + return *ptr; + } + + const value_type * operator->() const + { + return ptr; + } + + reference operator[](difference_type diff) const + { + return ptr[diff]; + } + + iterator & operator++() + { + ++ptr; + return *this; + } + + iterator operator++(int) + { + pointer tmp = ptr; + ++*this; + return iterator(tmp); + } + + iterator & operator--() + { + --ptr; + return *this; + } + + iterator operator--(int) + { + pointer tmp = ptr; + --*this; + return iterator(tmp); + } + + iterator & operator+=(difference_type diff) + { + ptr += diff; + return *this; + } + + iterator operator+(difference_type diff) const + { + return iterator(ptr + diff); + } + + friend iterator operator+(difference_type diff, const iterator & rhs) + { + return iterator(diff + rhs.ptr); + } + + iterator & operator-=(difference_type diff) + { + ptr -= diff; + return *this; + } + + iterator operator-(difference_type diff) const + { + return iterator(ptr - diff); + } + + difference_type operator-(const iterator & rhs) const + { + return ptr - rhs.ptr; + } + + std::strong_ordering operator<=>(const iterator & rhs) const = default; + }; + + using const_iterator = iterator; + + iterator begin() const & + { + return data(); + } + + iterator end() const & + { + return data() + size(); + } + + /* Ensure that no dangling iterators can be created accidentally, as that + would lead to hard to diagnose bugs that only affect small lists. */ + iterator begin() && = delete; + iterator end() && = delete; +}; + +static_assert(std::random_access_iterator); + struct Value : public ValueStorage { friend std::string showType(const Value & v); @@ -564,15 +747,9 @@ public: return isa(); } - std::span listItems() const noexcept + ListView listView() const noexcept { - assert(isList()); - return std::span(listElems(), listSize()); - } - - Value * const * listElems() const noexcept - { - return isa() ? payload.smallList.data() : getStorage().elems; + return isa() ? ListView(getStorage()) : ListView(getStorage()); } size_t listSize() const noexcept diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index 40ff250a9..f9f834a62 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -415,7 +415,7 @@ void prim_importNative(EvalState & state, const PosIdx pos, Value * * args, Valu void prim_exec(EvalState & state, const PosIdx pos, Value * * args, Value & v) { state.forceList(*args[0], pos, "while evaluating the first argument passed to builtins.exec"); - auto elems = args[0]->listElems(); + auto elems = args[0]->listView(); auto count = args[0]->listSize(); if (count == 0) state.error("at least one argument to 'exec' required").atPos(pos).debugThrow(); @@ -660,8 +660,8 @@ struct CompareValues return false; } else if (i == v1->listSize()) { return true; - } else if (!state.eqValues(*v1->listElems()[i], *v2->listElems()[i], pos, errorCtx)) { - return (*this)(v1->listElems()[i], v2->listElems()[i], "while comparing two list elements"); + } else if (!state.eqValues(*v1->listView()[i], *v2->listView()[i], pos, errorCtx)) { + return (*this)(v1->listView()[i], v2->listView()[i], "while comparing two list elements"); } } default: @@ -689,7 +689,7 @@ static void prim_genericClosure(EvalState & state, const PosIdx pos, Value * * a state.forceList(*startSet->value, noPos, "while evaluating the 'startSet' attribute passed as argument to builtins.genericClosure"); ValueList workSet; - for (auto elem : startSet->value->listItems()) + for (auto elem : startSet->value->listView()) workSet.push_back(elem); if (startSet->value->listSize() == 0) { @@ -727,7 +727,7 @@ static void prim_genericClosure(EvalState & state, const PosIdx pos, Value * * a state.forceList(newElements, noPos, "while evaluating the return value of the `operator` passed to builtins.genericClosure"); /* Add the values returned by the operator to the work set. */ - for (auto elem : newElements.listItems()) { + for (auto elem : newElements.listView()) { state.forceValue(*elem, noPos); // "while evaluating one one of the elements returned by the `operator` passed to builtins.genericClosure"); workSet.push_back(elem); } @@ -1367,7 +1367,7 @@ static void derivationStrictInternal( command-line arguments to the builder. */ else if (i->name == state.sArgs) { state.forceList(*i->value, pos, context_below); - for (auto elem : i->value->listItems()) { + for (auto elem : i->value->listView()) { auto s = state.coerceToString(pos, *elem, context, "while evaluating an element of the argument list", true).toOwned(); @@ -1399,7 +1399,7 @@ static void derivationStrictInternal( /* Require ‘outputs’ to be a list of strings. */ state.forceList(*i->value, pos, context_below); Strings ss; - for (auto elem : i->value->listItems()) + for (auto elem : i->value->listView()) ss.emplace_back(state.forceStringNoCtx(*elem, pos, context_below)); handleOutputs(ss); } @@ -1882,7 +1882,7 @@ static void prim_findFile(EvalState & state, const PosIdx pos, Value * * args, V LookupPath lookupPath; - for (auto v2 : args[0]->listItems()) { + for (auto v2 : args[0]->listView()) { state.forceAttrs(*v2, pos, "while evaluating an element of the list passed to builtins.findFile"); std::string prefix; @@ -2920,7 +2920,7 @@ static void prim_removeAttrs(EvalState & state, const PosIdx pos, Value * * args // 64: large enough to fit the attributes of a derivation boost::container::small_vector names; names.reserve(args[1]->listSize()); - for (auto elem : args[1]->listItems()) { + for (auto elem : args[1]->listView()) { state.forceStringNoCtx(*elem, pos, "while evaluating the values of the second argument passed to builtins.removeAttrs"); names.emplace_back(state.symbols.create(elem->string_view()), nullptr); } @@ -2963,11 +2963,12 @@ static void prim_listToAttrs(EvalState & state, const PosIdx pos, Value * * args state.forceList(*args[0], pos, "while evaluating the argument passed to builtins.listToAttrs"); // Step 1. Sort the name-value attrsets in place using the memory we allocate for the result - size_t listSize = args[0]->listSize(); + auto listView = args[0]->listView(); + size_t listSize = listView.size(); auto & bindings = *state.allocBindings(listSize); using ElemPtr = decltype(&bindings[0].value); - for (const auto & [n, v2] : enumerate(args[0]->listItems())) { + for (const auto & [n, v2] : enumerate(listView)) { state.forceAttrs(*v2, pos, "while evaluating an element of the list passed to builtins.listToAttrs"); auto j = state.getAttr(state.sName, v2->attrs(), "in a {name=...; value=...;} pair"); @@ -3122,7 +3123,7 @@ static void prim_catAttrs(EvalState & state, const PosIdx pos, Value * * args, V SmallValueVector res(args[1]->listSize()); size_t found = 0; - for (auto v2 : args[1]->listItems()) { + for (auto v2 : args[1]->listView()) { state.forceAttrs(*v2, pos, "while evaluating an element in the list passed as second argument to builtins.catAttrs"); if (auto i = v2->attrs()->get(attrName)) res[found++] = i->value; @@ -3247,7 +3248,7 @@ static void prim_zipAttrsWith(EvalState & state, const PosIdx pos, Value * * arg state.forceFunction(*args[0], pos, "while evaluating the first argument passed to builtins.zipAttrsWith"); state.forceList(*args[1], pos, "while evaluating the second argument passed to builtins.zipAttrsWith"); - const auto listItems = args[1]->listItems(); + const auto listItems = args[1]->listView(); for (auto & vElem : listItems) { state.forceAttrs(*vElem, noPos, "while evaluating a value of the list passed as second argument to builtins.zipAttrsWith"); @@ -3346,8 +3347,8 @@ static void prim_elemAt(EvalState & state, const PosIdx pos, Value * * args, Val n, args[0]->listSize() ).atPos(pos).debugThrow(); - state.forceValue(*args[0]->listElems()[n], pos); - v = *args[0]->listElems()[n]; + state.forceValue(*args[0]->listView()[n], pos); + v = *args[0]->listView()[n]; } static RegisterPrimOp primop_elemAt({ @@ -3368,8 +3369,8 @@ static void prim_head(EvalState & state, const PosIdx pos, Value * * args, Value state.error( "'builtins.head' called on an empty list" ).atPos(pos).debugThrow(); - state.forceValue(*args[0]->listElems()[0], pos); - v = *args[0]->listElems()[0]; + state.forceValue(*args[0]->listView()[0], pos); + v = *args[0]->listView()[0]; } static RegisterPrimOp primop_head({ @@ -3394,7 +3395,7 @@ static void prim_tail(EvalState & state, const PosIdx pos, Value * * args, Value auto list = state.buildList(args[0]->listSize() - 1); for (const auto & [n, v] : enumerate(list)) - v = args[0]->listElems()[n + 1]; + v = args[0]->listView()[n + 1]; v.mkList(list); } @@ -3429,7 +3430,7 @@ static void prim_map(EvalState & state, const PosIdx pos, Value * * args, Value auto list = state.buildList(args[1]->listSize()); for (const auto & [n, v] : enumerate(list)) (v = state.allocValue())->mkApp( - args[0], args[1]->listElems()[n]); + args[0], args[1]->listView()[n]); v.mkList(list); } @@ -3470,9 +3471,9 @@ static void prim_filter(EvalState & state, const PosIdx pos, Value * * args, Val bool same = true; for (size_t n = 0; n < len; ++n) { Value res; - state.callFunction(*args[0], *args[1]->listElems()[n], res, noPos); + state.callFunction(*args[0], *args[1]->listView()[n], res, noPos); if (state.forceBool(res, pos, "while evaluating the return value of the filtering function passed to builtins.filter")) - vs[k++] = args[1]->listElems()[n]; + vs[k++] = args[1]->listView()[n]; else same = false; } @@ -3501,7 +3502,7 @@ static void prim_elem(EvalState & state, const PosIdx pos, Value * * args, Value { bool res = false; state.forceList(*args[1], pos, "while evaluating the second argument passed to builtins.elem"); - for (auto elem : args[1]->listItems()) + for (auto elem : args[1]->listView()) if (state.eqValues(*args[0], *elem, pos, "while searching for the presence of the given element in the list")) { res = true; break; @@ -3523,7 +3524,8 @@ static RegisterPrimOp primop_elem({ static void prim_concatLists(EvalState & state, const PosIdx pos, Value * * args, Value & v) { state.forceList(*args[0], pos, "while evaluating the first argument passed to builtins.concatLists"); - state.concatLists(v, args[0]->listSize(), args[0]->listElems(), pos, "while evaluating a value of the list passed to builtins.concatLists"); + auto listView = args[0]->listView(); + state.concatLists(v, args[0]->listSize(), listView.data(), pos, "while evaluating a value of the list passed to builtins.concatLists"); } static RegisterPrimOp primop_concatLists({ @@ -3561,7 +3563,8 @@ static void prim_foldlStrict(EvalState & state, const PosIdx pos, Value * * args if (args[2]->listSize()) { Value * vCur = args[1]; - for (auto [n, elem] : enumerate(args[2]->listItems())) { + auto listView = args[2]->listView(); + for (auto [n, elem] : enumerate(listView)) { Value * vs []{vCur, elem}; vCur = n == args[2]->listSize() - 1 ? &v : state.allocValue(); state.callFunction(*args[0], vs, *vCur, pos); @@ -3603,7 +3606,7 @@ static void anyOrAll(bool any, EvalState & state, const PosIdx pos, Value * * ar : "while evaluating the return value of the function passed to builtins.all"; Value vTmp; - for (auto elem : args[1]->listItems()) { + for (auto elem : args[1]->listView()) { state.callFunction(*args[0], *elem, vTmp, pos); bool res = state.forceBool(vTmp, pos, errorCtx); if (res == any) { @@ -3701,7 +3704,7 @@ static void prim_sort(EvalState & state, const PosIdx pos, Value * * args, Value auto list = state.buildList(len); for (const auto & [n, v] : enumerate(list)) - state.forceValue(*(v = args[1]->listElems()[n]), pos); + state.forceValue(*(v = args[1]->listView()[n]), pos); auto comparator = [&](Value * a, Value * b) { /* Optimization: if the comparator is lessThan, bypass @@ -3787,7 +3790,7 @@ static void prim_partition(EvalState & state, const PosIdx pos, Value * * args, ValueVector right, wrong; for (size_t n = 0; n < len; ++n) { - auto vElem = args[1]->listElems()[n]; + auto vElem = args[1]->listView()[n]; state.forceValue(*vElem, pos); Value res; state.callFunction(*args[0], *vElem, res, pos); @@ -3844,7 +3847,7 @@ static void prim_groupBy(EvalState & state, const PosIdx pos, Value * * args, Va ValueVectorMap attrs; - for (auto vElem : args[1]->listItems()) { + for (auto vElem : args[1]->listView()) { Value res; state.callFunction(*args[0], *vElem, res, pos); auto name = state.forceStringNoCtx(res, pos, "while evaluating the return value of the grouping function passed to builtins.groupBy"); @@ -3900,7 +3903,7 @@ static void prim_concatMap(EvalState & state, const PosIdx pos, Value * * args, size_t len = 0; for (size_t n = 0; n < nrLists; ++n) { - Value * vElem = args[1]->listElems()[n]; + Value * vElem = args[1]->listView()[n]; state.callFunction(*args[0], *vElem, lists[n], pos); state.forceList(lists[n], lists[n].determinePos(args[0]->determinePos(pos)), "while evaluating the return value of the function passed to builtins.concatMap"); len += lists[n].listSize(); @@ -3909,9 +3912,10 @@ static void prim_concatMap(EvalState & state, const PosIdx pos, Value * * args, auto list = state.buildList(len); auto out = list.elems; for (size_t n = 0, pos = 0; n < nrLists; ++n) { - auto l = lists[n].listSize(); + auto listView = lists[n].listView(); + auto l = listView.size(); if (l) - memcpy(out + pos, lists[n].listElems(), l * sizeof(Value *)); + memcpy(out + pos, listView.data(), l * sizeof(Value *)); pos += l; } v.mkList(list); @@ -4587,7 +4591,7 @@ static void prim_concatStringsSep(EvalState & state, const PosIdx pos, Value * * res.reserve((args[1]->listSize() + 32) * sep.size()); bool first = true; - for (auto elem : args[1]->listItems()) { + for (auto elem : args[1]->listView()) { if (first) first = false; else res += sep; res += *state.coerceToString(pos, *elem, context, "while evaluating one element of the list of strings to concat passed to builtins.concatStringsSep"); } @@ -4617,11 +4621,11 @@ static void prim_replaceStrings(EvalState & state, const PosIdx pos, Value * * a std::vector from; from.reserve(args[0]->listSize()); - for (auto elem : args[0]->listItems()) + for (auto elem : args[0]->listView()) from.emplace_back(state.forceString(*elem, pos, "while evaluating one of the strings to replace passed to builtins.replaceStrings")); std::unordered_map cache; - auto to = args[1]->listItems(); + auto to = args[1]->listView(); NixStringContext context; auto s = state.forceString(*args[2], context, pos, "while evaluating the third argument passed to builtins.replaceStrings"); diff --git a/src/libexpr/primops/context.cc b/src/libexpr/primops/context.cc index 496678884..56962d6a8 100644 --- a/src/libexpr/primops/context.cc +++ b/src/libexpr/primops/context.cc @@ -312,7 +312,7 @@ static void prim_appendContext(EvalState & state, const PosIdx pos, Value * * ar name ).atPos(i.pos).debugThrow(); } - for (auto elem : attr->value->listItems()) { + for (auto elem : attr->value->listView()) { auto outputName = state.forceStringNoCtx(*elem, attr->pos, "while evaluating an output name within a string context"); context.emplace(NixStringContextElem::Built { .drvPath = makeConstantStorePathRef(namePath), diff --git a/src/libexpr/print-ambiguous.cc b/src/libexpr/print-ambiguous.cc index 0646783c2..2a0b009eb 100644 --- a/src/libexpr/print-ambiguous.cc +++ b/src/libexpr/print-ambiguous.cc @@ -50,11 +50,13 @@ void printAmbiguous( break; } case nList: - if (seen && v.listSize() && !seen->insert(v.listElems()).second) + /* Use pointer to the Value instead of pointer to the elements, because + that would need to explicitly handle the case of SmallList. */ + if (seen && v.listSize() && !seen->insert(&v).second) str << "«repeated»"; else { str << "[ "; - for (auto v2 : v.listItems()) { + for (auto v2 : v.listView()) { if (v2) printAmbiguous(*v2, symbols, str, seen, depth - 1); else diff --git a/src/libexpr/print.cc b/src/libexpr/print.cc index f0e0eed2d..1f0c592c1 100644 --- a/src/libexpr/print.cc +++ b/src/libexpr/print.cc @@ -415,8 +415,8 @@ private: if (depth < options.maxDepth) { increaseIndent(); output << "["; - auto listItems = v.listItems(); - auto prettyPrint = shouldPrettyPrintList(listItems); + auto listItems = v.listView(); + auto prettyPrint = shouldPrettyPrintList(listItems.span()); size_t currentListItemsPrinted = 0; diff --git a/src/libexpr/value-to-json.cc b/src/libexpr/value-to-json.cc index 4c0667d9e..a9b51afa0 100644 --- a/src/libexpr/value-to-json.cc +++ b/src/libexpr/value-to-json.cc @@ -73,7 +73,7 @@ json printValueAsJSON(EvalState & state, bool strict, case nList: { out = json::array(); int i = 0; - for (auto elem : v.listItems()) { + for (auto elem : v.listView()) { try { out.push_back(printValueAsJSON(state, strict, *elem, pos, context, copyToStore)); } catch (Error & e) { diff --git a/src/libexpr/value-to-xml.cc b/src/libexpr/value-to-xml.cc index 54ff06f9e..235ef2627 100644 --- a/src/libexpr/value-to-xml.cc +++ b/src/libexpr/value-to-xml.cc @@ -114,7 +114,7 @@ static void printValueAsXML(EvalState & state, bool strict, bool location, case nList: { XMLOpenElement _(doc, "list"); - for (auto v2 : v.listItems()) + for (auto v2 : v.listView()) printValueAsXML(state, strict, location, *v2, doc, context, drvsSeen, pos); break; } diff --git a/src/libflake/flake.cc b/src/libflake/flake.cc index e59649fec..322abaa4a 100644 --- a/src/libflake/flake.cc +++ b/src/libflake/flake.cc @@ -289,7 +289,7 @@ static Flake readFlake( Explicit { state.forceBool(*setting.value, setting.pos, "") }); else if (setting.value->type() == nList) { std::vector ss; - for (auto elem : setting.value->listItems()) { + for (auto elem : setting.value->listView()) { if (elem->type() != nString) state.error("list element in flake configuration setting '%s' is %s while a string is expected", state.symbols[setting.name], showType(*setting.value)).debugThrow(); diff --git a/src/nix-env/nix-env.cc b/src/nix-env/nix-env.cc index 25ff39e38..fd48e67dc 100644 --- a/src/nix-env/nix-env.cc +++ b/src/nix-env/nix-env.cc @@ -1265,7 +1265,7 @@ static void opQuery(Globals & globals, Strings opFlags, Strings opArgs) } else if (v->type() == nList) { attrs2["type"] = "strings"; XMLOpenElement m(xml, "meta", attrs2); - for (auto elem : v->listItems()) { + for (auto elem : v->listView()) { if (elem->type() != nString) continue; XMLAttrs attrs3; attrs3["value"] = elem->c_str(); diff --git a/src/nix/prefetch.cc b/src/nix/prefetch.cc index 9e5e3c09a..96dcdb4e8 100644 --- a/src/nix/prefetch.cc +++ b/src/nix/prefetch.cc @@ -46,7 +46,7 @@ std::string resolveMirrorUrl(EvalState & state, const std::string & url) if (mirrorList->value->listSize() < 1) throw Error("mirror URL '%s' did not expand to anything", url); - std::string mirror(state.forceString(*mirrorList->value->listElems()[0], noPos, "while evaluating the first available mirror")); + std::string mirror(state.forceString(*mirrorList->value->listView()[0], noPos, "while evaluating the first available mirror")); return mirror + (hasSuffix(mirror, "/") ? "" : "/") + s.substr(p + 1); } @@ -221,7 +221,7 @@ static int main_nix_prefetch_url(int argc, char * * argv) state->forceList(*attr->value, noPos, "while evaluating the urls to prefetch"); if (attr->value->listSize() < 1) throw Error("'urls' list is empty"); - url = state->forceString(*attr->value->listElems()[0], noPos, "while evaluating the first url from the urls list"); + url = state->forceString(*attr->value->listView()[0], noPos, "while evaluating the first url from the urls list"); /* Extract the hash mode. */ auto attr2 = v.attrs()->get(state->symbols.create("outputHashMode"));