diff --git a/src/libcmd/include/nix/cmd/installable-attr-path.hh b/src/libcmd/include/nix/cmd/installable-attr-path.hh index 35e8ca66f07d..b563508a95b6 100644 --- a/src/libcmd/include/nix/cmd/installable-attr-path.hh +++ b/src/libcmd/include/nix/cmd/installable-attr-path.hh @@ -27,7 +27,7 @@ namespace nix { class InstallableAttrPath : public InstallableValue { SourceExprCommand & cmd; - UniqueRootValue v; + RootValue v; std::string attrPath; ExtendedOutputsSpec extendedOutputsSpec; diff --git a/src/libcmd/installable-attr-path.cc b/src/libcmd/installable-attr-path.cc index 912cdd6492c3..3b6fb336f2b2 100644 --- a/src/libcmd/installable-attr-path.cc +++ b/src/libcmd/installable-attr-path.cc @@ -32,7 +32,7 @@ InstallableAttrPath::InstallableAttrPath( ExtendedOutputsSpec extendedOutputsSpec) : InstallableValue(state) , cmd(cmd) - , v(UniqueRootValue(v)) + , v(RootValue(v)) , attrPath(attrPath) , extendedOutputsSpec(std::move(extendedOutputsSpec)) { diff --git a/src/libexpr/eval-cache.cc b/src/libexpr/eval-cache.cc index b4363a65cdfb..c6dd2766c88b 100644 --- a/src/libexpr/eval-cache.cc +++ b/src/libexpr/eval-cache.cc @@ -361,7 +361,7 @@ Value * EvalCache::getRootValue() { if (!value) { debug("getting root value"); - value = UniqueRootValue(rootLoader()); + value = RootValue(rootLoader()); } return *value; } @@ -378,7 +378,7 @@ AttrCursor::AttrCursor( , cachedValue(std::move(cachedValue)) { if (value) - _value = UniqueRootValue(value); + _value = RootValue(value); } AttrKey AttrCursor::getKey() @@ -401,9 +401,9 @@ Value & AttrCursor::getValue() auto attr = vParent.attrs()->get(parent->second); if (!attr) throw Error("attribute '%s' is unexpectedly missing", getAttrPathStr()); - _value = UniqueRootValue(attr->value); + _value = RootValue(attr->value); } else - _value = UniqueRootValue(root->getRootValue()); + _value = RootValue(root->getRootValue()); } return **_value; } diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index 70aba544f32d..1fe593502c55 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -575,7 +575,7 @@ Value * EvalState::addPrimOp(PrimOp && primOp) v->mkPrimOp(new PrimOp(primOp)); if (primOp.internal) - internalPrimOps.emplace(primOp.name, UniqueRootValue(v)); + internalPrimOps.emplace(primOp.name, RootValue(v)); else { staticBaseEnv->vars.emplace_back(envName, baseEnvDispl); baseEnv.values[baseEnvDispl++] = v; @@ -749,11 +749,11 @@ void mapStaticEnvBindings(const SymbolTable & st, const StaticEnv & se, const En if (se.isWith && env.values[0]->isFinished()) { // add 'with' bindings. for (auto & j : *env.values[0]->attrs()) - vm.insert_or_assign(std::string(st[j.name]), UniqueRootValue(j.value)); + vm.insert_or_assign(std::string(st[j.name]), RootValue(j.value)); } else { // iterate through staticenv bindings and add them. for (auto & i : se.vars) - vm.insert_or_assign(std::string(st[i.first]), UniqueRootValue(env.values[i.second])); + vm.insert_or_assign(std::string(st[i.first]), RootValue(env.values[i.second])); } } } @@ -1177,7 +1177,7 @@ void EvalState::evalFile(const SourcePath & path, Value & v, bool mustBeTrivial) fileEvalCache->try_emplace_and_cvisit( *resolvedPath, - UniqueRootValue(nullptr), + RootValue(nullptr), [&](auto & i) { vExpr = allocValue(); vExpr->mkThunk(&baseEnv, &expr); diff --git a/src/libexpr/include/nix/expr/eval-cache.hh b/src/libexpr/include/nix/expr/eval-cache.hh index 0bbe398b9356..3a148988a102 100644 --- a/src/libexpr/include/nix/expr/eval-cache.hh +++ b/src/libexpr/include/nix/expr/eval-cache.hh @@ -45,7 +45,7 @@ public: private: typedef fun RootLoader; RootLoader rootLoader; - UniqueRootValue value; + RootValue value; Value * getRootValue(); @@ -112,7 +112,7 @@ public: private: using Parent = std::optional, Symbol>>; const Parent parent; - UniqueRootValue _value; + RootValue _value; std::optional> cachedValue; AttrKey getKey(); diff --git a/src/libexpr/include/nix/expr/eval.hh b/src/libexpr/include/nix/expr/eval.hh index 098cdaee6e21..8688b7a3842e 100644 --- a/src/libexpr/include/nix/expr/eval.hh +++ b/src/libexpr/include/nix/expr/eval.hh @@ -168,7 +168,7 @@ struct Constant bool impureOnly = false; }; -typedef std::map ValMap; +typedef std::map ValMap; typedef boost::unordered_flat_map> DocCommentMap; @@ -472,7 +472,7 @@ private: /** * A cache from resolved paths to values. */ - const ref> fileEvalCache; + const ref> fileEvalCache; /** * Associate source positions of certain AST nodes with their preceding doc comment, if they have one. @@ -810,7 +810,7 @@ public: /** * Internal primops not exposed to the user. */ - boost::unordered_flat_map internalPrimOps; + boost::unordered_flat_map internalPrimOps; /** * Name and documentation about every constant. diff --git a/src/libexpr/include/nix/expr/get-drvs.hh b/src/libexpr/include/nix/expr/get-drvs.hh index 858151df6149..4f655d514ed9 100644 --- a/src/libexpr/include/nix/expr/get-drvs.hh +++ b/src/libexpr/include/nix/expr/get-drvs.hh @@ -32,7 +32,7 @@ private: */ bool failed = false; - // FIXME: make this a UniqueRootValue. + // FIXME: make this a RootValue. const Bindings *attrs = nullptr, *meta = nullptr; const Bindings * getMeta(); diff --git a/src/libexpr/include/nix/expr/parallel-eval.hh b/src/libexpr/include/nix/expr/parallel-eval.hh index f3678451b884..6e771dc72d64 100644 --- a/src/libexpr/include/nix/expr/parallel-eval.hh +++ b/src/libexpr/include/nix/expr/parallel-eval.hh @@ -21,6 +21,7 @@ namespace nix { struct Executor { + // FIXME: support std::moveable_function. using work_t = std::function; struct Item diff --git a/src/libexpr/include/nix/expr/root-value.hh b/src/libexpr/include/nix/expr/root-value.hh index c22db67c1685..7d5fa8767240 100644 --- a/src/libexpr/include/nix/expr/root-value.hh +++ b/src/libexpr/include/nix/expr/root-value.hh @@ -8,59 +8,53 @@ namespace nix { struct Value; -/** - * Allocate a slot from the root value pool, i.e. a GC-visible - * `Value *` cell that keeps the value it points to alive across - * garbage collections. Use `UniqueRootValue`/`RootValue` rather than - * calling this directly. - */ -Value ** allocRootValueSlot(Value * v); - -/** - * Clear the given slot and return it to the root value pool. - */ -void freeRootValueSlot(Value ** slot); - /** * A move-only handle rooting a Value, i.e. keeping it and everything * reachable from it alive across garbage collections. Prefer this * over `RootValue` unless the handle must be copyable (e.g. when it's * captured in a `std::function`-backed lambda). */ -class UniqueRootValue +class RootValue { Value ** slot = nullptr; + /** + * Clear the given slot and return it to the root value pool. + */ + void freeRootValueSlot(); + public: - UniqueRootValue() = default; + RootValue() = default; - explicit UniqueRootValue(Value * v) - : slot(allocRootValueSlot(v)) - { - } + /** + * Allocate a slot from the root value pool, i.e. a GC-visible + * `Value *` cell that keeps the value it points to alive across + * garbage collections. Use `RootValue`/`RootValue` rather than + * calling this directly. + */ + explicit RootValue(Value * v); - UniqueRootValue(const UniqueRootValue &) = delete; - UniqueRootValue & operator=(const UniqueRootValue &) = delete; + RootValue(const RootValue &) = delete; + RootValue & operator=(const RootValue &) = delete; - UniqueRootValue(UniqueRootValue && other) noexcept + RootValue(RootValue && other) noexcept : slot(std::exchange(other.slot, nullptr)) { } - UniqueRootValue & operator=(UniqueRootValue && other) noexcept + RootValue & operator=(RootValue && other) noexcept { if (this == &other) return *this; if (slot) - freeRootValueSlot(slot); + freeRootValueSlot(); slot = std::exchange(other.slot, nullptr); return *this; } - ~UniqueRootValue() + ~RootValue() { - if (slot) - freeRootValueSlot(slot); + reset(); } /** @@ -68,10 +62,8 @@ public: */ void reset() { - if (slot) { - freeRootValueSlot(slot); - slot = nullptr; - } + if (slot) + freeRootValueSlot(); } Value *& operator*() const @@ -85,12 +77,4 @@ public: } }; -/** - * A copyable, shared handle rooting a Value. Only use this instead of - * `UniqueRootValue` if the handle must be copyable. - */ -typedef std::shared_ptr RootValue; - -RootValue allocRootValue(Value * v); - } // namespace nix diff --git a/src/libexpr/json-to-value.cc b/src/libexpr/json-to-value.cc index 4416978be12f..77020d704294 100644 --- a/src/libexpr/json-to-value.cc +++ b/src/libexpr/json-to-value.cc @@ -18,7 +18,7 @@ class JSONSax : nlohmann::json_sax { protected: std::unique_ptr parent; - UniqueRootValue v; + RootValue v; public: virtual std::unique_ptr resolve(EvalState &) { @@ -31,7 +31,7 @@ class JSONSax : nlohmann::json_sax } explicit JSONState(Value * v) - : v(UniqueRootValue(v)) + : v(RootValue(v)) { } @@ -40,7 +40,7 @@ class JSONSax : nlohmann::json_sax Value & value(EvalState & state) { if (!v) - v = UniqueRootValue(state.allocValue()); + v = RootValue(state.allocValue()); return **v; } diff --git a/src/libexpr/parallel-eval.cc b/src/libexpr/parallel-eval.cc index 50e91e6ee3ec..a2955961e063 100644 --- a/src/libexpr/parallel-eval.cc +++ b/src/libexpr/parallel-eval.cc @@ -289,8 +289,9 @@ static void prim_parallel(EvalState & state, const PosIdx pos, Value ** args, Va Executor::WorkItems work; for (auto value : args[0]->listView()) if (!value->isFinished()) - state.addWork( - work, 0, [value(allocRootValue(value)), &state, pos]() { state.forceValue(**value, pos); }); + state.addWork(work, 0, [value(std::make_shared(value)), &state, pos]() { + state.forceValue(***value, pos); + }); state.executor->spawn(std::move(work)); } diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index 8ea3ac525bac..943d87027b07 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -786,9 +786,9 @@ struct CompareValues static void prim_genericClosure(EvalState & state, const PosIdx pos, Value ** args, Value & v) { - /* The values are rooted via UniqueRootValue, so the lists themselves + /* The values are rooted via RootValue, so the lists themselves don't need to be visible to the GC. */ - using ValueList = std::list; + using ValueList = std::list; state.forceAttrs(*args[0], noPos, "while evaluating the first argument passed to builtins.genericClosure"); @@ -803,7 +803,7 @@ static void prim_genericClosure(EvalState & state, const PosIdx pos, Value ** ar ValueList workSet; for (auto elem : startSet->value->listView()) - workSet.push_back(UniqueRootValue(elem)); + workSet.push_back(RootValue(elem)); if (startSet->value->listSize() == 0) { v = *startSet->value; @@ -869,7 +869,7 @@ static void prim_genericClosure(EvalState & state, const PosIdx pos, Value ** ar } throw; } - res.push_back(UniqueRootValue(e)); + res.push_back(RootValue(e)); /* Call the `operator' function with `e' as argument. */ Value newElements; @@ -884,7 +884,7 @@ static void prim_genericClosure(EvalState & state, const PosIdx pos, Value ** ar 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(UniqueRootValue(elem)); + workSet.push_back(RootValue(elem)); } } catch (Error & err) { err.addTrace( diff --git a/src/libexpr/root-value.cc b/src/libexpr/root-value.cc index 3fa209517e22..7ad2c645db60 100644 --- a/src/libexpr/root-value.cc +++ b/src/libexpr/root-value.cc @@ -36,7 +36,7 @@ static auto & freeSlots = *new Sync{nullptr}; #endif -Value ** allocRootValueSlot(Value * v) +RootValue::RootValue(Value * v) { #if NIX_USE_BOEHMGC Slot * slot; @@ -58,13 +58,13 @@ Value ** allocRootValueSlot(Value * v) slot->value = v; - return &slot->value; + this->slot = &slot->value; #else - return new Value *(v); + this->slot = new Value *(v); #endif } -void freeRootValueSlot(Value ** slot) +void RootValue::freeRootValueSlot() { #if NIX_USE_BOEHMGC /* Note: writing `nextFree` overwrites the `Value *`, so this also @@ -76,11 +76,7 @@ void freeRootValueSlot(Value ** slot) #else delete slot; #endif -} - -RootValue allocRootValue(Value * v) -{ - return RootValue(allocRootValueSlot(v), freeRootValueSlot); + slot = nullptr; } } // namespace nix diff --git a/src/libexpr/value-to-json.cc b/src/libexpr/value-to-json.cc index 58e3b3e53993..b25f8fb759b0 100644 --- a/src/libexpr/value-to-json.cc +++ b/src/libexpr/value-to-json.cc @@ -29,8 +29,8 @@ static void parallelForceDeep(EvalState & state, Value & v, PosIdx pos) if (v.attrs()->get(state.s.outPath)) return; for (auto & a : *v.attrs()) - state.addWork(work, 0, [value(allocRootValue(a.value)), pos(a.pos), &state]() { - parallelForceDeep(state, **value, pos); + state.addWork(work, 0, [value(std::make_shared(a.value)), pos(a.pos), &state]() { + parallelForceDeep(state, ***value, pos); }); break; }