diff --git a/src/libexpr-c/nix_api_value.cc b/src/libexpr-c/nix_api_value.cc index 0f6595e49..093daf2f8 100644 --- a/src/libexpr-c/nix_api_value.cc +++ b/src/libexpr-c/nix_api_value.cc @@ -594,7 +594,7 @@ nix_err nix_bindings_builder_insert(nix_c_context * context, BindingsBuilder * b context->last_err_code = NIX_OK; try { auto & v = check_value_not_null(value); - nix::Symbol s = bb->builder.state.symbols.create(name); + nix::Symbol s = bb->builder.state.get().symbols.create(name); bb->builder.insert(s, &v); } NIXC_CATCH_ERRS diff --git a/src/libexpr-tests/value/print.cc b/src/libexpr-tests/value/print.cc index 739d4e40b..1959fddf2 100644 --- a/src/libexpr-tests/value/print.cc +++ b/src/libexpr-tests/value/print.cc @@ -61,7 +61,7 @@ TEST_F(ValuePrintingTests, tAttrs) Value vTwo; vTwo.mkInt(2); - BindingsBuilder builder(state, state.allocBindings(10)); + BindingsBuilder builder = state.buildBindings(10); builder.insert(state.symbols.create("one"), &vOne); builder.insert(state.symbols.create("two"), &vTwo); @@ -196,11 +196,11 @@ TEST_F(ValuePrintingTests, depthAttrs) Value vTwo; vTwo.mkInt(2); - BindingsBuilder builderEmpty(state, state.allocBindings(0)); + BindingsBuilder builderEmpty = state.buildBindings(0); Value vAttrsEmpty; vAttrsEmpty.mkAttrs(builderEmpty.finish()); - BindingsBuilder builder(state, state.allocBindings(10)); + BindingsBuilder builder = state.buildBindings(10); builder.insert(state.symbols.create("one"), &vOne); builder.insert(state.symbols.create("two"), &vTwo); builder.insert(state.symbols.create("nested"), &vAttrsEmpty); @@ -208,7 +208,7 @@ TEST_F(ValuePrintingTests, depthAttrs) Value vAttrs; vAttrs.mkAttrs(builder.finish()); - BindingsBuilder builder2(state, state.allocBindings(10)); + BindingsBuilder builder2 = state.buildBindings(10); builder2.insert(state.symbols.create("one"), &vOne); builder2.insert(state.symbols.create("two"), &vTwo); builder2.insert(state.symbols.create("nested"), &vAttrs); @@ -233,14 +233,14 @@ TEST_F(ValuePrintingTests, depthList) Value vTwo; vTwo.mkInt(2); - BindingsBuilder builder(state, state.allocBindings(10)); + BindingsBuilder builder = state.buildBindings(10); builder.insert(state.symbols.create("one"), &vOne); builder.insert(state.symbols.create("two"), &vTwo); Value vAttrs; vAttrs.mkAttrs(builder.finish()); - BindingsBuilder builder2(state, state.allocBindings(10)); + BindingsBuilder builder2 = state.buildBindings(10); builder2.insert(state.symbols.create("one"), &vOne); builder2.insert(state.symbols.create("two"), &vTwo); builder2.insert(state.symbols.create("nested"), &vAttrs); @@ -295,7 +295,7 @@ TEST_F(ValuePrintingTests, attrsTypeFirst) Value vApple; vApple.mkStringNoCopy("apple"); - BindingsBuilder builder(state, state.allocBindings(10)); + BindingsBuilder builder = state.buildBindings(10); builder.insert(state.symbols.create("type"), &vType); builder.insert(state.symbols.create("apple"), &vApple); @@ -374,7 +374,7 @@ TEST_F(ValuePrintingTests, ansiColorsAttrs) Value vTwo; vTwo.mkInt(2); - BindingsBuilder builder(state, state.allocBindings(10)); + BindingsBuilder builder = state.buildBindings(10); builder.insert(state.symbols.create("one"), &vOne); builder.insert(state.symbols.create("two"), &vTwo); @@ -392,7 +392,7 @@ TEST_F(ValuePrintingTests, ansiColorsDerivation) Value vDerivation; vDerivation.mkStringNoCopy("derivation"); - BindingsBuilder builder(state, state.allocBindings(10)); + BindingsBuilder builder = state.buildBindings(10); builder.insert(state.s.type, &vDerivation); Value vAttrs; @@ -437,7 +437,7 @@ TEST_F(ValuePrintingTests, ansiColorsDerivationError) Value vDerivation; vDerivation.mkStringNoCopy("derivation"); - BindingsBuilder builder(state, state.allocBindings(10)); + BindingsBuilder builder = state.buildBindings(10); builder.insert(state.s.type, &vDerivation); builder.insert(state.s.drvPath, &vError); @@ -553,12 +553,12 @@ TEST_F(ValuePrintingTests, ansiColorsBlackhole) TEST_F(ValuePrintingTests, ansiColorsAttrsRepeated) { - BindingsBuilder emptyBuilder(state, state.allocBindings(1)); + BindingsBuilder emptyBuilder = state.buildBindings(1); Value vEmpty; vEmpty.mkAttrs(emptyBuilder.finish()); - BindingsBuilder builder(state, state.allocBindings(10)); + BindingsBuilder builder = state.buildBindings(10); builder.insert(state.symbols.create("a"), &vEmpty); builder.insert(state.symbols.create("b"), &vEmpty); @@ -570,7 +570,7 @@ TEST_F(ValuePrintingTests, ansiColorsAttrsRepeated) TEST_F(ValuePrintingTests, ansiColorsListRepeated) { - BindingsBuilder emptyBuilder(state, state.allocBindings(1)); + BindingsBuilder emptyBuilder = state.buildBindings(1); Value vEmpty; vEmpty.mkAttrs(emptyBuilder.finish()); @@ -586,7 +586,7 @@ TEST_F(ValuePrintingTests, ansiColorsListRepeated) TEST_F(ValuePrintingTests, listRepeated) { - BindingsBuilder emptyBuilder(state, state.allocBindings(1)); + BindingsBuilder emptyBuilder = state.buildBindings(1); Value vEmpty; vEmpty.mkAttrs(emptyBuilder.finish()); @@ -609,7 +609,7 @@ TEST_F(ValuePrintingTests, ansiColorsAttrsElided) Value vTwo; vTwo.mkInt(2); - BindingsBuilder builder(state, state.allocBindings(10)); + BindingsBuilder builder = state.buildBindings(10); builder.insert(state.symbols.create("one"), &vOne); builder.insert(state.symbols.create("two"), &vTwo); @@ -635,8 +635,6 @@ TEST_F(ValuePrintingTests, ansiColorsAttrsElided) TEST_F(ValuePrintingTests, ansiColorsListElided) { - BindingsBuilder emptyBuilder(state, state.allocBindings(1)); - Value vOne; vOne.mkInt(1); diff --git a/src/libexpr/attr-set.cc b/src/libexpr/attr-set.cc index 3a06441e9..eb44b0dd9 100644 --- a/src/libexpr/attr-set.cc +++ b/src/libexpr/attr-set.cc @@ -16,19 +16,19 @@ Bindings * EvalState::allocBindings(size_t capacity) throw Error("attribute set of size %d is too big", capacity); nrAttrsets++; nrAttrsInAttrsets += capacity; - return new (allocBytes(sizeof(Bindings) + sizeof(Attr) * capacity)) Bindings((Bindings::size_t) capacity); + return new (allocBytes(sizeof(Bindings) + sizeof(Attr) * capacity)) Bindings(); } Value & BindingsBuilder::alloc(Symbol name, PosIdx pos) { - auto value = state.allocValue(); + auto value = state.get().allocValue(); bindings->push_back(Attr(name, value, pos)); return *value; } Value & BindingsBuilder::alloc(std::string_view name, PosIdx pos) { - return alloc(state.symbols.create(name), pos); + return alloc(state.get().symbols.create(name), pos); } void Bindings::sort() diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index fd2108537..9d740c717 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -205,7 +205,7 @@ EvalState::EvalState( , settings{settings} , symbols(StaticEvalSymbols::staticSymbolTable()) , repair(NoRepair) - , emptyBindings(0) + , emptyBindings(Bindings()) , storeFS(makeMountedSourceAccessor({ {CanonPath::root, makeEmptySourceAccessor()}, /* In the pure eval case, we can simply require @@ -1218,7 +1218,7 @@ void ExprAttrs::eval(EvalState & state, Env & env, Value & v) *vOverrides, [&]() { return vOverrides->determinePos(noPos); }, "while evaluating the `__overrides` attribute"); - bindings.grow(state.allocBindings(bindings.capacity() + vOverrides->attrs()->size())); + bindings.grow(state.buildBindings(bindings.capacity() + vOverrides->attrs()->size())); for (auto & i : *vOverrides->attrs()) { AttrDefs::iterator j = attrs.find(i.name); if (j != attrs.end()) { diff --git a/src/libexpr/include/nix/expr/attr-set.hh b/src/libexpr/include/nix/expr/attr-set.hh index 85bba1099..b5e927a7e 100644 --- a/src/libexpr/include/nix/expr/attr-set.hh +++ b/src/libexpr/include/nix/expr/attr-set.hh @@ -5,6 +5,7 @@ #include "nix/expr/symbol-table.hh" #include +#include namespace nix { @@ -54,16 +55,14 @@ public: PosIdx pos; private: - size_t size_, capacity_; + size_t size_ = 0; Attr attrs[0]; - Bindings(size_t capacity) - : size_(0) - , capacity_(capacity) - { - } - - Bindings(const Bindings & bindings) = delete; + Bindings() = default; + Bindings(const Bindings &) = delete; + Bindings(Bindings &&) = delete; + Bindings & operator=(const Bindings &) = delete; + Bindings & operator=(Bindings &&) = delete; public: size_t size() const @@ -82,7 +81,6 @@ public: void push_back(const Attr & attr) { - assert(size_ < capacity_); attrs[size_++] = attr; } @@ -136,11 +134,6 @@ public: void sort(); - size_t capacity() const - { - return capacity_; - } - /** * Returns the attributes in lexicographically sorted order. */ @@ -165,22 +158,29 @@ public: * order at the end. The only way to consume a BindingsBuilder is to * call finish(), which sorts the bindings. */ -class BindingsBuilder +class BindingsBuilder final { - Bindings * bindings; - public: // needed by std::back_inserter using value_type = Attr; + using size_type = Bindings::size_t; - EvalState & state; +private: + Bindings * bindings; + Bindings::size_t capacity_; - BindingsBuilder(EvalState & state, Bindings * bindings) + friend class EvalState; + + BindingsBuilder(EvalState & state, Bindings * bindings, size_type capacity) : bindings(bindings) + , capacity_(capacity) , state(state) { } +public: + std::reference_wrapper state; + void insert(Symbol name, Value * value, PosIdx pos = noPos) { insert(Attr(name, value, pos)); @@ -193,6 +193,7 @@ public: void push_back(const Attr & attr) { + assert(bindings->size() < capacity_); bindings->push_back(attr); } @@ -211,16 +212,16 @@ public: return bindings; } - size_t capacity() + size_t capacity() const noexcept { - return bindings->capacity(); + return capacity_; } - void grow(Bindings * newBindings) + void grow(BindingsBuilder newBindings) { for (auto & i : *bindings) - newBindings->push_back(i); - bindings = newBindings; + newBindings.push_back(i); + std::swap(*this, newBindings); } friend struct ExprAttrs; diff --git a/src/libexpr/include/nix/expr/eval.hh b/src/libexpr/include/nix/expr/eval.hh index 04729b100..5015a009b 100644 --- a/src/libexpr/include/nix/expr/eval.hh +++ b/src/libexpr/include/nix/expr/eval.hh @@ -879,7 +879,7 @@ public: BindingsBuilder buildBindings(size_t capacity) { - return BindingsBuilder(*this, allocBindings(capacity)); + return BindingsBuilder(*this, allocBindings(capacity), capacity); } ListBuilder buildList(size_t size)