mirror of
https://github.com/NixOS/nix.git
synced 2025-11-08 19:46:02 +01:00
libexpr: Make Bindings::iterator a proper strong type instead of pointer
As evident from the number of tests that were holding this API completely wrong (the end() iterator returned from find() is NEVER nullptr) we should not have this footgun. A proper strong type guarantees that this confusion will not happen again. Also this will be helpful down the road when Bindings becomes something smarter than an array of Attr.
This commit is contained in:
Sergei Zimmerman
parent
e75501da3e
commit
ddabd94f82
3 changed files with 83 additions and 27 deletions
|
|
@ -195,18 +195,18 @@ TEST_F(PrimOpTest, unsafeGetAttrPos)
|
|||
auto v = eval(expr);
|
||||
ASSERT_THAT(v, IsAttrsOfSize(3));
|
||||
|
||||
auto file = v.attrs()->find(createSymbol("file"));
|
||||
auto file = v.attrs()->get(createSymbol("file"));
|
||||
ASSERT_NE(file, nullptr);
|
||||
ASSERT_THAT(*file->value, IsString());
|
||||
auto s = baseNameOf(file->value->string_view());
|
||||
ASSERT_EQ(s, "foo.nix");
|
||||
|
||||
auto line = v.attrs()->find(createSymbol("line"));
|
||||
auto line = v.attrs()->get(createSymbol("line"));
|
||||
ASSERT_NE(line, nullptr);
|
||||
state.forceValue(*line->value, noPos);
|
||||
ASSERT_THAT(*line->value, IsIntEq(4));
|
||||
|
||||
auto column = v.attrs()->find(createSymbol("column"));
|
||||
auto column = v.attrs()->get(createSymbol("column"));
|
||||
ASSERT_NE(column, nullptr);
|
||||
state.forceValue(*column->value, noPos);
|
||||
ASSERT_THAT(*column->value, IsIntEq(3));
|
||||
|
|
@ -246,7 +246,7 @@ TEST_F(PrimOpTest, removeAttrsRetains)
|
|||
{
|
||||
auto v = eval("builtins.removeAttrs { x = 1; y = 2; } [\"x\"]");
|
||||
ASSERT_THAT(v, IsAttrsOfSize(1));
|
||||
ASSERT_NE(v.attrs()->find(createSymbol("y")), nullptr);
|
||||
ASSERT_NE(v.attrs()->get(createSymbol("y")), nullptr);
|
||||
}
|
||||
|
||||
TEST_F(PrimOpTest, listToAttrsEmptyList)
|
||||
|
|
@ -266,7 +266,7 @@ TEST_F(PrimOpTest, listToAttrs)
|
|||
{
|
||||
auto v = eval("builtins.listToAttrs [ { name = \"key\"; value = 123; } ]");
|
||||
ASSERT_THAT(v, IsAttrsOfSize(1));
|
||||
auto key = v.attrs()->find(createSymbol("key"));
|
||||
auto key = v.attrs()->get(createSymbol("key"));
|
||||
ASSERT_NE(key, nullptr);
|
||||
ASSERT_THAT(*key->value, IsIntEq(123));
|
||||
}
|
||||
|
|
@ -275,7 +275,7 @@ TEST_F(PrimOpTest, intersectAttrs)
|
|||
{
|
||||
auto v = eval("builtins.intersectAttrs { a = 1; b = 2; } { b = 3; c = 4; }");
|
||||
ASSERT_THAT(v, IsAttrsOfSize(1));
|
||||
auto b = v.attrs()->find(createSymbol("b"));
|
||||
auto b = v.attrs()->get(createSymbol("b"));
|
||||
ASSERT_NE(b, nullptr);
|
||||
ASSERT_THAT(*b->value, IsIntEq(3));
|
||||
}
|
||||
|
|
@ -293,11 +293,11 @@ TEST_F(PrimOpTest, functionArgs)
|
|||
auto v = eval("builtins.functionArgs ({ x, y ? 123}: 1)");
|
||||
ASSERT_THAT(v, IsAttrsOfSize(2));
|
||||
|
||||
auto x = v.attrs()->find(createSymbol("x"));
|
||||
auto x = v.attrs()->get(createSymbol("x"));
|
||||
ASSERT_NE(x, nullptr);
|
||||
ASSERT_THAT(*x->value, IsFalse());
|
||||
|
||||
auto y = v.attrs()->find(createSymbol("y"));
|
||||
auto y = v.attrs()->get(createSymbol("y"));
|
||||
ASSERT_NE(y, nullptr);
|
||||
ASSERT_THAT(*y->value, IsTrue());
|
||||
}
|
||||
|
|
@ -307,13 +307,13 @@ TEST_F(PrimOpTest, mapAttrs)
|
|||
auto v = eval("builtins.mapAttrs (name: value: value * 10) { a = 1; b = 2; }");
|
||||
ASSERT_THAT(v, IsAttrsOfSize(2));
|
||||
|
||||
auto a = v.attrs()->find(createSymbol("a"));
|
||||
auto a = v.attrs()->get(createSymbol("a"));
|
||||
ASSERT_NE(a, nullptr);
|
||||
ASSERT_THAT(*a->value, IsThunk());
|
||||
state.forceValue(*a->value, noPos);
|
||||
ASSERT_THAT(*a->value, IsIntEq(10));
|
||||
|
||||
auto b = v.attrs()->find(createSymbol("b"));
|
||||
auto b = v.attrs()->get(createSymbol("b"));
|
||||
ASSERT_NE(b, nullptr);
|
||||
ASSERT_THAT(*b->value, IsThunk());
|
||||
state.forceValue(*b->value, noPos);
|
||||
|
|
@ -839,11 +839,11 @@ TEST_P(ParseDrvNamePrimOpTest, parseDrvName)
|
|||
auto v = eval(expr);
|
||||
ASSERT_THAT(v, IsAttrsOfSize(2));
|
||||
|
||||
auto name = v.attrs()->find(createSymbol("name"));
|
||||
auto name = v.attrs()->get(createSymbol("name"));
|
||||
ASSERT_TRUE(name);
|
||||
ASSERT_THAT(*name->value, IsStringEq(expectedName));
|
||||
|
||||
auto version = v.attrs()->find(createSymbol("version"));
|
||||
auto version = v.attrs()->get(createSymbol("version"));
|
||||
ASSERT_TRUE(version);
|
||||
ASSERT_THAT(*version->value, IsStringEq(expectedVersion));
|
||||
}
|
||||
|
|
|
|||
|
|
@ -75,11 +75,11 @@ TEST_F(TrivialExpressionTest, updateAttrs)
|
|||
{
|
||||
auto v = eval("{ a = 1; } // { b = 2; a = 3; }");
|
||||
ASSERT_THAT(v, IsAttrsOfSize(2));
|
||||
auto a = v.attrs()->find(createSymbol("a"));
|
||||
auto a = v.attrs()->get(createSymbol("a"));
|
||||
ASSERT_NE(a, nullptr);
|
||||
ASSERT_THAT(*a->value, IsIntEq(3));
|
||||
|
||||
auto b = v.attrs()->find(createSymbol("b"));
|
||||
auto b = v.attrs()->get(createSymbol("b"));
|
||||
ASSERT_NE(b, nullptr);
|
||||
ASSERT_THAT(*b->value, IsIntEq(2));
|
||||
}
|
||||
|
|
@ -176,7 +176,7 @@ TEST_P(AttrSetMergeTrvialExpressionTest, attrsetMergeLazy)
|
|||
auto v = eval(expr);
|
||||
ASSERT_THAT(v, IsAttrsOfSize(1));
|
||||
|
||||
auto a = v.attrs()->find(createSymbol("a"));
|
||||
auto a = v.attrs()->get(createSymbol("a"));
|
||||
ASSERT_NE(a, nullptr);
|
||||
|
||||
ASSERT_THAT(*a->value, IsThunk());
|
||||
|
|
@ -184,11 +184,11 @@ TEST_P(AttrSetMergeTrvialExpressionTest, attrsetMergeLazy)
|
|||
|
||||
ASSERT_THAT(*a->value, IsAttrsOfSize(2));
|
||||
|
||||
auto b = a->value->attrs()->find(createSymbol("b"));
|
||||
auto b = a->value->attrs()->get(createSymbol("b"));
|
||||
ASSERT_NE(b, nullptr);
|
||||
ASSERT_THAT(*b->value, IsIntEq(1));
|
||||
|
||||
auto c = a->value->attrs()->find(createSymbol("c"));
|
||||
auto c = a->value->attrs()->get(createSymbol("c"));
|
||||
ASSERT_NE(c, nullptr);
|
||||
ASSERT_THAT(*c->value, IsIntEq(2));
|
||||
}
|
||||
|
|
@ -330,7 +330,7 @@ TEST_F(TrivialExpressionTest, bindOr)
|
|||
{
|
||||
auto v = eval("{ or = 1; }");
|
||||
ASSERT_THAT(v, IsAttrsOfSize(1));
|
||||
auto b = v.attrs()->find(createSymbol("or"));
|
||||
auto b = v.attrs()->get(createSymbol("or"));
|
||||
ASSERT_NE(b, nullptr);
|
||||
ASSERT_THAT(*b->value, IsIntEq(1));
|
||||
}
|
||||
|
|
|
|||
|
|
@ -6,6 +6,7 @@
|
|||
|
||||
#include <algorithm>
|
||||
#include <functional>
|
||||
#include <concepts>
|
||||
|
||||
namespace nix {
|
||||
|
||||
|
|
@ -81,7 +82,55 @@ public:
|
|||
return !size_;
|
||||
}
|
||||
|
||||
typedef const Attr * const_iterator;
|
||||
class iterator
|
||||
{
|
||||
public:
|
||||
using value_type = Attr;
|
||||
using pointer = const value_type *;
|
||||
using reference = const value_type &;
|
||||
using difference_type = std::ptrdiff_t;
|
||||
using iterator_category = std::forward_iterator_tag;
|
||||
|
||||
friend class Bindings;
|
||||
|
||||
private:
|
||||
pointer ptr = nullptr;
|
||||
|
||||
explicit iterator(pointer ptr)
|
||||
: ptr(ptr)
|
||||
{
|
||||
}
|
||||
|
||||
public:
|
||||
iterator() = default;
|
||||
|
||||
reference operator*() const
|
||||
{
|
||||
return *ptr;
|
||||
}
|
||||
|
||||
const value_type * operator->() const
|
||||
{
|
||||
return ptr;
|
||||
}
|
||||
|
||||
iterator & operator++()
|
||||
{
|
||||
++ptr;
|
||||
return *this;
|
||||
}
|
||||
|
||||
iterator operator++(int)
|
||||
{
|
||||
pointer tmp = ptr;
|
||||
++*this;
|
||||
return iterator(tmp);
|
||||
}
|
||||
|
||||
bool operator==(const iterator & rhs) const = default;
|
||||
};
|
||||
|
||||
using const_iterator = iterator;
|
||||
|
||||
void push_back(const Attr & attr)
|
||||
{
|
||||
|
|
@ -91,29 +140,33 @@ public:
|
|||
const_iterator find(Symbol name) const
|
||||
{
|
||||
Attr key(name, 0);
|
||||
const_iterator i = std::lower_bound(begin(), end(), key);
|
||||
if (i != end() && i->name == name)
|
||||
return i;
|
||||
auto first = attrs;
|
||||
auto last = attrs + size_;
|
||||
const Attr * i = std::lower_bound(first, last, key);
|
||||
if (i != last && i->name == name)
|
||||
return const_iterator{i};
|
||||
return end();
|
||||
}
|
||||
|
||||
const Attr * get(Symbol name) const
|
||||
{
|
||||
Attr key(name, 0);
|
||||
const_iterator i = std::lower_bound(begin(), end(), key);
|
||||
if (i != end() && i->name == name)
|
||||
return &*i;
|
||||
auto first = attrs;
|
||||
auto last = attrs + size_;
|
||||
const Attr * i = std::lower_bound(first, last, key);
|
||||
if (i != last && i->name == name)
|
||||
return i;
|
||||
return nullptr;
|
||||
}
|
||||
|
||||
const_iterator begin() const
|
||||
{
|
||||
return &attrs[0];
|
||||
return const_iterator(attrs);
|
||||
}
|
||||
|
||||
const_iterator end() const
|
||||
{
|
||||
return &attrs[size_];
|
||||
return const_iterator(attrs + size_);
|
||||
}
|
||||
|
||||
Attr & operator[](size_t pos)
|
||||
|
|
@ -147,6 +200,9 @@ public:
|
|||
friend class EvalState;
|
||||
};
|
||||
|
||||
static_assert(std::forward_iterator<Bindings::iterator>);
|
||||
static_assert(std::ranges::forward_range<Bindings>);
|
||||
|
||||
/**
|
||||
* A wrapper around Bindings that ensures that its always in sorted
|
||||
* order at the end. The only way to consume a BindingsBuilder is to
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue