From 1fc9ba4885f5a16e263bcc5e58bef68e3aa32cea Mon Sep 17 00:00:00 2001 From: Kane York Date: Thu, 13 Aug 2020 16:40:27 -0700 Subject: [PATCH] refactor(tvix): always pass Bindings by ptr, use shared/unique_ptr Value now carries a shared_ptr, and all Bindings constructors return a unique_ptr. The test that wanted to compare two Bindings by putting them into Values has been modified to use the new Equal() method on Bindings (extracted from EvalState). Change-Id: I8dfb60e65fdabb717e3b3e5d56d5b3fc82f70883 Reviewed-on: https://cl.tvl.fyi/c/depot/+/1744 Tested-by: BuildkiteCI Reviewed-by: glittershark Reviewed-by: tazjin --- third_party/nix/src/libexpr/attr-path.cc | 2 +- third_party/nix/src/libexpr/attr-path.hh | 2 +- third_party/nix/src/libexpr/attr-set.cc | 54 +++++++++++++------ third_party/nix/src/libexpr/attr-set.hh | 15 ++++-- .../nix/src/libexpr/common-eval-args.cc | 4 +- .../nix/src/libexpr/common-eval-args.hh | 2 +- third_party/nix/src/libexpr/eval.cc | 27 +++------- third_party/nix/src/libexpr/eval.hh | 5 +- third_party/nix/src/libexpr/get-drvs.cc | 18 ++++--- third_party/nix/src/libexpr/get-drvs.hh | 8 +-- third_party/nix/src/libexpr/value.hh | 2 +- third_party/nix/src/nix-build/nix-build.cc | 6 +-- third_party/nix/src/nix-env/nix-env.cc | 15 +++--- third_party/nix/src/nix-env/user-env.cc | 4 +- .../src/nix-instantiate/nix-instantiate.cc | 14 ++--- .../src/nix-prefetch-url/nix-prefetch-url.cc | 4 +- third_party/nix/src/nix/edit.cc | 4 +- third_party/nix/src/nix/installables.cc | 8 +-- third_party/nix/src/nix/repl.cc | 4 +- third_party/nix/src/nix/search.cc | 3 +- third_party/nix/src/nix/upgrade-nix.cc | 5 +- third_party/nix/src/tests/attr-set.cc | 30 +++++------ 22 files changed, 129 insertions(+), 107 deletions(-) diff --git a/third_party/nix/src/libexpr/attr-path.cc b/third_party/nix/src/libexpr/attr-path.cc index a267e82b6..86ebeec2f 100644 --- a/third_party/nix/src/libexpr/attr-path.cc +++ b/third_party/nix/src/libexpr/attr-path.cc @@ -39,7 +39,7 @@ static Strings parseAttrPath(const std::string& s) { } Value* findAlongAttrPath(EvalState& state, const std::string& attrPath, - Bindings& autoArgs, Value& vIn) { + Bindings* autoArgs, Value& vIn) { Strings tokens = parseAttrPath(attrPath); Error attrError = diff --git a/third_party/nix/src/libexpr/attr-path.hh b/third_party/nix/src/libexpr/attr-path.hh index a71ceb9a0..97170be84 100644 --- a/third_party/nix/src/libexpr/attr-path.hh +++ b/third_party/nix/src/libexpr/attr-path.hh @@ -8,6 +8,6 @@ namespace nix { Value* findAlongAttrPath(EvalState& state, const std::string& attrPath, - Bindings& autoArgs, Value& vIn); + Bindings* autoArgs, Value& vIn); } diff --git a/third_party/nix/src/libexpr/attr-set.cc b/third_party/nix/src/libexpr/attr-set.cc index e9e2a2bad..808ffce17 100644 --- a/third_party/nix/src/libexpr/attr-set.cc +++ b/third_party/nix/src/libexpr/attr-set.cc @@ -9,8 +9,6 @@ namespace nix { -static Bindings ZERO_BINDINGS; - // This function inherits its name from previous implementations, in // which Bindings was backed by an array of elements which was scanned // linearly. @@ -22,8 +20,6 @@ static Bindings ZERO_BINDINGS; // This behaviour is mimicked by using .insert(), which will *not* // override existing values. void Bindings::push_back(const Attr& attr) { - assert(this != &ZERO_BINDINGS); - auto [_, inserted] = attributes_.insert({attr.name, attr}); if (!inserted) { @@ -51,20 +47,48 @@ Bindings::iterator Bindings::find(const Symbol& name) { return attributes_.find(name); } -Bindings::iterator Bindings::begin() { return attributes_.begin(); } - -Bindings::iterator Bindings::end() { return attributes_.end(); } - -Bindings* Bindings::NewGC(size_t capacity) { - if (capacity == 0) { - return &ZERO_BINDINGS; +bool Bindings::Equal(const Bindings* other, EvalState& state) const { + if (this == other) { + return true; } - return new Bindings; + if (this->attributes_.size() != other->attributes_.size()) { + return false; + } + + Bindings::const_iterator i; + Bindings::const_iterator j; + for (i = this->cbegin(), j = other->cbegin(); i != this->cend(); ++i, ++j) { + if (i->second.name != j->second.name || + !state.eqValues(*i->second.value, *j->second.value)) { + return false; + } + } + + return true; } -Bindings* Bindings::Merge(const Bindings& lhs, const Bindings& rhs) { - auto bindings = NewGC(lhs.size() + rhs.size()); +Bindings::iterator Bindings::begin() { return attributes_.begin(); } +Bindings::iterator Bindings::end() { return attributes_.end(); } + +Bindings::const_iterator Bindings::cbegin() const { + return attributes_.cbegin(); +} + +Bindings::const_iterator Bindings::cend() const { return attributes_.cend(); } + +std::unique_ptr Bindings::New(size_t capacity) { + if (capacity == 0) { + // TODO(tazjin): A lot of 0-capacity Bindings are allocated. + // It would be nice to optimize that. + } + + return std::make_unique(); +} + +std::unique_ptr Bindings::Merge(const Bindings& lhs, + const Bindings& rhs) { + auto bindings = New(lhs.size() + rhs.size()); // Values are merged by inserting the entire iterator range of both // input sets. The right-hand set (the values of which take @@ -81,7 +105,7 @@ Bindings* Bindings::Merge(const Bindings& lhs, const Bindings& rhs) { void EvalState::mkAttrs(Value& v, size_t capacity) { clearValue(v); v.type = tAttrs; - v.attrs = Bindings::NewGC(capacity); + v.attrs = Bindings::New(capacity); nrAttrsets++; nrAttrsInAttrsets += capacity; } diff --git a/third_party/nix/src/libexpr/attr-set.hh b/third_party/nix/src/libexpr/attr-set.hh index 639700851..c02f3930f 100644 --- a/third_party/nix/src/libexpr/attr-set.hh +++ b/third_party/nix/src/libexpr/attr-set.hh @@ -25,15 +25,17 @@ using AttributeMap = absl::btree_map; class Bindings { public: - typedef AttributeMap::iterator iterator; + using iterator = AttributeMap::iterator; + using const_iterator = AttributeMap::const_iterator; // Allocate a new attribute set that is visible to the garbage // collector. - static Bindings* NewGC(size_t capacity = 0); + static std::unique_ptr New(size_t capacity = 0); // Create a new attribute set by merging two others. This is used to // implement the `//` operator in Nix. - static Bindings* Merge(const Bindings& lhs, const Bindings& rhs); + static std::unique_ptr Merge(const Bindings& lhs, + const Bindings& rhs); // Return the number of contained elements. size_t size() const; @@ -44,11 +46,18 @@ class Bindings { // Insert, but do not replace, values in the attribute set. void push_back(const Attr& attr); + // Are these two attribute sets deeply equal? + // Note: Does not special-case derivations. Use state.eqValues() to check + // attrsets that may be derivations. + bool Equal(const Bindings* other, EvalState& state) const; + // Look up a specific element of the attribute set. iterator find(const Symbol& name); iterator begin(); + const_iterator cbegin() const; iterator end(); + const_iterator cend() const; // Returns the elements of the attribute set as a vector, sorted // lexicographically by keys. diff --git a/third_party/nix/src/libexpr/common-eval-args.cc b/third_party/nix/src/libexpr/common-eval-args.cc index b3e378273..e36457f4f 100644 --- a/third_party/nix/src/libexpr/common-eval-args.cc +++ b/third_party/nix/src/libexpr/common-eval-args.cc @@ -32,8 +32,8 @@ MixEvalArgs::MixEvalArgs() { .handler([&](const std::string& s) { searchPath.push_back(s); }); } -Bindings* MixEvalArgs::getAutoArgs(EvalState& state) { - Bindings* res = Bindings::NewGC(autoArgs.size()); +std::unique_ptr MixEvalArgs::getAutoArgs(EvalState& state) { + auto res = Bindings::New(autoArgs.size()); for (auto& i : autoArgs) { Value* v = state.allocValue(); if (i.second[0] == 'E') { diff --git a/third_party/nix/src/libexpr/common-eval-args.hh b/third_party/nix/src/libexpr/common-eval-args.hh index be7126c03..9b0a77c6f 100644 --- a/third_party/nix/src/libexpr/common-eval-args.hh +++ b/third_party/nix/src/libexpr/common-eval-args.hh @@ -11,7 +11,7 @@ class Bindings; struct MixEvalArgs : virtual Args { MixEvalArgs(); - Bindings* getAutoArgs(EvalState& state); + std::unique_ptr getAutoArgs(EvalState& state); Strings searchPath; diff --git a/third_party/nix/src/libexpr/eval.cc b/third_party/nix/src/libexpr/eval.cc index 4ae3661b0..569e04e3a 100644 --- a/third_party/nix/src/libexpr/eval.cc +++ b/third_party/nix/src/libexpr/eval.cc @@ -1096,7 +1096,7 @@ void EvalState::callFunction(Value& fun, Value& arg, Value& v, const Pos& pos) { // prevents tail-call optimisation. void EvalState::incrFunctionCall(ExprLambda* fun) { functionCalls[fun]++; } -void EvalState::autoCallFunction(Bindings& args, Value& fun, Value& res) { +void EvalState::autoCallFunction(Bindings* args, Value& fun, Value& res) { forceValue(fun); if (fun.type == tAttrs) { @@ -1118,8 +1118,8 @@ void EvalState::autoCallFunction(Bindings& args, Value& fun, Value& res) { mkAttrs(*actualArgs, fun.lambda.fun->formals->formals.size()); for (auto& i : fun.lambda.fun->formals->formals) { - Bindings::iterator j = args.find(i.name); - if (j != args.end()) { + Bindings::iterator j = args->find(i.name); + if (j != args->end()) { actualArgs->attrs->push_back(j->second); } else if (i.def == nullptr) { throwTypeError( @@ -1615,22 +1615,7 @@ bool EvalState::eqValues(Value& v1, Value& v2) { } } - if (v1.attrs->size() != v2.attrs->size()) { - return false; - } - - /* Otherwise, compare the attributes one by one. */ - Bindings::iterator i; - Bindings::iterator j; - for (i = v1.attrs->begin(), j = v2.attrs->begin(); i != v1.attrs->end(); - ++i, ++j) { - if (i->second.name != j->second.name || - !eqValues(*i->second.value, *j->second.value)) { - return false; - } - } - - return true; + return v1.attrs->Equal(v2.attrs.get(), *this); } /* Functions are incomparable. */ @@ -1811,8 +1796,8 @@ size_t valueSize(const Value& v) { sz += doString(v.path); break; case tAttrs: - if (seenBindings.find(v.attrs) == seenBindings.end()) { - seenBindings.insert(v.attrs); + if (seenBindings.find(v.attrs.get()) == seenBindings.end()) { + seenBindings.insert(v.attrs.get()); sz += sizeof(Bindings); for (const auto& i : *v.attrs) { sz += doValue(*i.second.value); diff --git a/third_party/nix/src/libexpr/eval.hh b/third_party/nix/src/libexpr/eval.hh index f3e38cfb3..aa7af777a 100644 --- a/third_party/nix/src/libexpr/eval.hh +++ b/third_party/nix/src/libexpr/eval.hh @@ -247,8 +247,9 @@ class EvalState { void callPrimOp(Value& fun, Value& arg, Value& v, const Pos& pos); /* Automatically call a function for which each argument has a - default value or has a binding in the `args' map. */ - void autoCallFunction(Bindings& args, Value& fun, Value& res); + default value or has a binding in the `args' map. 'args' need + not live past the end of the call. */ + void autoCallFunction(Bindings* args, Value& fun, Value& res); /* Allocation primitives. */ Value* allocValue(); diff --git a/third_party/nix/src/libexpr/get-drvs.cc b/third_party/nix/src/libexpr/get-drvs.cc index 02ddae1f8..d81fb5dfa 100644 --- a/third_party/nix/src/libexpr/get-drvs.cc +++ b/third_party/nix/src/libexpr/get-drvs.cc @@ -4,6 +4,7 @@ #include #include +#include #include #include @@ -13,7 +14,8 @@ namespace nix { -DrvInfo::DrvInfo(EvalState& state, std::string attrPath, Bindings* attrs) +DrvInfo::DrvInfo(EvalState& state, std::string attrPath, + std::shared_ptr attrs) : state(&state), attrs(attrs), attrPath(std::move(attrPath)) {} DrvInfo::DrvInfo(EvalState& state, const ref& store, @@ -161,7 +163,7 @@ std::string DrvInfo::queryOutputName() const { Bindings* DrvInfo::getMeta() { if (meta != nullptr) { - return meta; + return meta.get(); } if (attrs == nullptr) { return nullptr; @@ -172,7 +174,7 @@ Bindings* DrvInfo::getMeta() { } state->forceAttrs(*a->second.value, *a->second.pos); meta = a->second.value->attrs; - return meta; + return meta.get(); } StringSet DrvInfo::queryMetaNames() { @@ -292,8 +294,8 @@ bool DrvInfo::queryMetaBool(const std::string& name, bool def) { } void DrvInfo::setMeta(const std::string& name, Value* v) { - Bindings* old = getMeta(); - meta = Bindings::NewGC(old->size() + 1); + std::shared_ptr old = meta; + meta = std::shared_ptr(Bindings::New(old->size() + 1).release()); Symbol sym = state->symbols.Create(name); if (old != nullptr) { for (auto i : *old) { @@ -308,7 +310,7 @@ void DrvInfo::setMeta(const std::string& name, Value* v) { } /* Cache for already considered attrsets. */ -using Done = std::set; +using Done = absl::flat_hash_set>; /* Evaluate value `v'. If it evaluates to a set of type `derivation', then put information about it in `drvs' (unless it's already in `done'). @@ -364,7 +366,7 @@ static std::string addToPath(const std::string& s1, const std::string& s2) { static std::regex attrRegex("[A-Za-z_][A-Za-z0-9-_+]*"); static void getDerivations(EvalState& state, Value& vIn, - const std::string& pathPrefix, Bindings& autoArgs, + const std::string& pathPrefix, Bindings* autoArgs, DrvInfos& drvs, Done& done, bool ignoreAssertionFailures) { Value v; @@ -434,7 +436,7 @@ static void getDerivations(EvalState& state, Value& vIn, } void getDerivations(EvalState& state, Value& v, const std::string& pathPrefix, - Bindings& autoArgs, DrvInfos& drvs, + Bindings* autoArgs, DrvInfos& drvs, bool ignoreAssertionFailures) { Done done; getDerivations(state, v, pathPrefix, autoArgs, drvs, done, diff --git a/third_party/nix/src/libexpr/get-drvs.hh b/third_party/nix/src/libexpr/get-drvs.hh index 608713ed1..b17efe8b3 100644 --- a/third_party/nix/src/libexpr/get-drvs.hh +++ b/third_party/nix/src/libexpr/get-drvs.hh @@ -23,7 +23,8 @@ struct DrvInfo { bool failed = false; // set if we get an AssertionError - Bindings *attrs = nullptr, *meta = nullptr; + std::shared_ptr attrs = nullptr; + std::shared_ptr meta = nullptr; Bindings* getMeta(); @@ -33,7 +34,8 @@ struct DrvInfo { std::string attrPath; /* path towards the derivation */ DrvInfo(EvalState& state) : state(&state){}; - DrvInfo(EvalState& state, std::string attrPath, Bindings* attrs); + DrvInfo(EvalState& state, std::string attrPath, + std::shared_ptr attrs); DrvInfo(EvalState& state, const ref& store, const std::string& drvPathWithOutputs); @@ -75,7 +77,7 @@ std::optional getDerivation(EvalState& state, Value& v, bool ignoreAssertionFailures); void getDerivations(EvalState& state, Value& v, const std::string& pathPrefix, - Bindings& autoArgs, DrvInfos& drvs, + Bindings* autoArgs, DrvInfos& drvs, bool ignoreAssertionFailures); } // namespace nix diff --git a/third_party/nix/src/libexpr/value.hh b/third_party/nix/src/libexpr/value.hh index 85ce9e7a1..82360c566 100644 --- a/third_party/nix/src/libexpr/value.hh +++ b/third_party/nix/src/libexpr/value.hh @@ -95,7 +95,7 @@ struct Value { bool boolean; NixString string; const char* path; - Bindings* attrs; + std::shared_ptr attrs; NixList* list; NixThunk thunk; NixApp app; // TODO(tazjin): "app"? diff --git a/third_party/nix/src/nix-build/nix-build.cc b/third_party/nix/src/nix-build/nix-build.cc index 85b8a3246..1fb8a2f3a 100644 --- a/third_party/nix/src/nix-build/nix-build.cc +++ b/third_party/nix/src/nix-build/nix-build.cc @@ -266,7 +266,7 @@ static void _main(int argc, char** argv) { auto state = std::make_unique(myArgs.searchPath, store); state->repair = repair; - Bindings& autoArgs = *myArgs.getAutoArgs(*state); + std::unique_ptr autoArgs = myArgs.getAutoArgs(*state); if (packages) { std::ostringstream joined; @@ -334,9 +334,9 @@ static void _main(int argc, char** argv) { state->eval(e, vRoot); for (auto& i : attrPaths) { - Value& v(*findAlongAttrPath(*state, i, autoArgs, vRoot)); + Value& v(*findAlongAttrPath(*state, i, autoArgs.get(), vRoot)); state->forceValue(v); - getDerivations(*state, v, "", autoArgs, drvs, false); + getDerivations(*state, v, "", autoArgs.get(), drvs, false); } } diff --git a/third_party/nix/src/nix-env/nix-env.cc b/third_party/nix/src/nix-env/nix-env.cc index 396f65d0d..7502a648d 100644 --- a/third_party/nix/src/nix-env/nix-env.cc +++ b/third_party/nix/src/nix-env/nix-env.cc @@ -46,7 +46,7 @@ struct InstallSourceInfo { Path nixExprPath; /* for srcNixExprDrvs, srcNixExprs */ Path profile; /* for srcProfile */ std::string systemFilter; /* for srcNixExprDrvs */ - Bindings* autoArgs; + std::unique_ptr autoArgs; }; struct Globals { @@ -170,7 +170,7 @@ static void loadSourceExpr(EvalState& state, const Path& path, Value& v) { } static void loadDerivations(EvalState& state, const Path& nixExprPath, - const std::string& systemFilter, Bindings& autoArgs, + const std::string& systemFilter, Bindings* autoArgs, const std::string& pathPrefix, DrvInfos& elems) { Value vRoot; loadSourceExpr(state, nixExprPath, vRoot); @@ -333,7 +333,7 @@ static void queryInstSources(EvalState& state, InstallSourceInfo& instSource, Nix expression. */ DrvInfos allElems; loadDerivations(state, instSource.nixExprPath, instSource.systemFilter, - *instSource.autoArgs, "", allElems); + instSource.autoArgs.get(), "", allElems); elems = filterBySelector(state, allElems, args, newestOnly); @@ -356,7 +356,7 @@ static void queryInstSources(EvalState& state, InstallSourceInfo& instSource, Value vTmp; state.eval(eFun, vFun); mkApp(vTmp, vFun, vArg); - getDerivations(state, vTmp, "", *instSource.autoArgs, elems, true); + getDerivations(state, vTmp, "", instSource.autoArgs.get(), elems, true); } break; @@ -410,8 +410,9 @@ static void queryInstSources(EvalState& state, InstallSourceInfo& instSource, Value vRoot; loadSourceExpr(state, instSource.nixExprPath, vRoot); for (auto& i : args) { - Value& v(*findAlongAttrPath(state, i, *instSource.autoArgs, vRoot)); - getDerivations(state, v, "", *instSource.autoArgs, elems, true); + Value& v( + *findAlongAttrPath(state, i, instSource.autoArgs.get(), vRoot)); + getDerivations(state, v, "", instSource.autoArgs.get(), elems, true); } break; } @@ -959,7 +960,7 @@ static void opQuery(Globals& globals, Strings opFlags, Strings opArgs) { if (source == sAvailable || compareVersions) { loadDerivations(*globals.state, globals.instSource.nixExprPath, globals.instSource.systemFilter, - *globals.instSource.autoArgs, attrPath, availElems); + globals.instSource.autoArgs.get(), attrPath, availElems); } DrvInfos elems_ = filterBySelector( diff --git a/third_party/nix/src/nix-env/user-env.cc b/third_party/nix/src/nix-env/user-env.cc index 0cb921c82..dac0c52a8 100644 --- a/third_party/nix/src/nix-env/user-env.cc +++ b/third_party/nix/src/nix-env/user-env.cc @@ -20,8 +20,8 @@ DrvInfos queryInstalled(EvalState& state, const Path& userEnv) { if (pathExists(manifestFile)) { Value v; state.evalFile(manifestFile, v); - Bindings& bindings(*Bindings::NewGC()); - getDerivations(state, v, "", bindings, elems, false); + std::unique_ptr bindings(Bindings::New()); + getDerivations(state, v, "", bindings.get(), elems, false); } return elems; } diff --git a/third_party/nix/src/nix-instantiate/nix-instantiate.cc b/third_party/nix/src/nix-instantiate/nix-instantiate.cc index 7b8d6dd3d..236037299 100644 --- a/third_party/nix/src/nix-instantiate/nix-instantiate.cc +++ b/third_party/nix/src/nix-instantiate/nix-instantiate.cc @@ -23,7 +23,7 @@ static bool indirectRoot = false; enum OutputKind { okPlain, okXML, okJSON }; void processExpr(EvalState& state, const Strings& attrPaths, bool parseOnly, - bool strict, Bindings& autoArgs, bool evalOnly, + bool strict, Bindings* autoArgs, bool evalOnly, OutputKind output, bool location, Expr* e) { if (parseOnly) { std::cout << format("%1%\n") % *e; @@ -40,7 +40,7 @@ void processExpr(EvalState& state, const Strings& attrPaths, bool parseOnly, PathSet context; if (evalOnly) { Value vRes; - if (autoArgs.empty()) { + if (autoArgs->empty()) { vRes = v; } else { state.autoCallFunction(autoArgs, v, vRes); @@ -176,7 +176,7 @@ static int _main(int argc, char** argv) { }); } - Bindings& autoArgs = *myArgs.getAutoArgs(*state); + std::unique_ptr autoArgs = myArgs.getAutoArgs(*state); if (attrPaths.empty()) { attrPaths = {""}; @@ -195,8 +195,8 @@ static int _main(int argc, char** argv) { if (readStdin) { Expr* e = state->parseStdin(); - processExpr(*state, attrPaths, parseOnly, strict, autoArgs, evalOnly, - outputKind, xmlOutputSourceLocation, e); + processExpr(*state, attrPaths, parseOnly, strict, autoArgs.get(), + evalOnly, outputKind, xmlOutputSourceLocation, e); } else if (files.empty() && !fromArgs) { files.push_back("./default.nix"); } @@ -206,8 +206,8 @@ static int _main(int argc, char** argv) { ? state->parseExprFromString(i, absPath(".")) : state->parseExprFromFile(resolveExprPath( state->checkSourcePath(lookupFileArg(*state, i)))); - processExpr(*state, attrPaths, parseOnly, strict, autoArgs, evalOnly, - outputKind, xmlOutputSourceLocation, e); + processExpr(*state, attrPaths, parseOnly, strict, autoArgs.get(), + evalOnly, outputKind, xmlOutputSourceLocation, e); } state->printStats(); diff --git a/third_party/nix/src/nix-prefetch-url/nix-prefetch-url.cc b/third_party/nix/src/nix-prefetch-url/nix-prefetch-url.cc index 5454c6cd1..66e7cff81 100644 --- a/third_party/nix/src/nix-prefetch-url/nix-prefetch-url.cc +++ b/third_party/nix/src/nix-prefetch-url/nix-prefetch-url.cc @@ -107,7 +107,7 @@ static int _main(int argc, char** argv) { auto store = openStore(); auto state = std::make_unique(myArgs.searchPath, store); - Bindings& autoArgs = *myArgs.getAutoArgs(*state); + std::unique_ptr autoArgs = myArgs.getAutoArgs(*state); /* If -A is given, get the URI from the specified Nix expression. */ @@ -122,7 +122,7 @@ static int _main(int argc, char** argv) { resolveExprPath(lookupFileArg(*state, args.empty() ? "." : args[0])); Value vRoot; state->evalFile(path, vRoot); - Value& v(*findAlongAttrPath(*state, attrPath, autoArgs, vRoot)); + Value& v(*findAlongAttrPath(*state, attrPath, autoArgs.get(), vRoot)); state->forceAttrs(v); /* Extract the URI. */ diff --git a/third_party/nix/src/nix/edit.cc b/third_party/nix/src/nix/edit.cc index a18cba292..644caa7cb 100644 --- a/third_party/nix/src/nix/edit.cc +++ b/third_party/nix/src/nix/edit.cc @@ -30,8 +30,8 @@ struct CmdEdit final : InstallableCommand { Value* v2; try { - auto dummyArgs = Bindings::NewGC(); - v2 = findAlongAttrPath(*state, "meta.position", *dummyArgs, *v); + auto dummyArgs = Bindings::New(); + v2 = findAlongAttrPath(*state, "meta.position", dummyArgs.get(), *v); } catch (Error&) { throw Error("package '%s' has no source location information", installable->what()); diff --git a/third_party/nix/src/nix/installables.cc b/third_party/nix/src/nix/installables.cc index b257a26f2..dd1202586 100644 --- a/third_party/nix/src/nix/installables.cc +++ b/third_party/nix/src/nix/installables.cc @@ -122,10 +122,10 @@ struct InstallableValue : Installable { auto v = toValue(*state); - Bindings& autoArgs = *cmd.getAutoArgs(*state); + std::unique_ptr autoArgs = cmd.getAutoArgs(*state); DrvInfos drvs; - getDerivations(*state, *v, "", autoArgs, drvs, false); + getDerivations(*state, *v, "", autoArgs.get(), drvs, false); Buildables res; @@ -185,9 +185,9 @@ struct InstallableAttrPath final : InstallableValue { Value* toValue(EvalState& state) override { auto source = cmd.getSourceExpr(state); - Bindings& autoArgs = *cmd.getAutoArgs(state); + std::unique_ptr autoArgs = cmd.getAutoArgs(state); - Value* v = findAlongAttrPath(state, attrPath, autoArgs, *source); + Value* v = findAlongAttrPath(state, attrPath, autoArgs.get(), *source); state.forceValue(*v); return v; diff --git a/third_party/nix/src/nix/repl.cc b/third_party/nix/src/nix/repl.cc index 85750b08c..2607655f9 100644 --- a/third_party/nix/src/nix/repl.cc +++ b/third_party/nix/src/nix/repl.cc @@ -35,7 +35,7 @@ namespace nix { struct NixRepl { std::string curDir; EvalState state; - Bindings* autoArgs; + std::unique_ptr autoArgs; Strings loadedFiles; @@ -575,7 +575,7 @@ void NixRepl::loadFile(const Path& path) { Value v; Value v2; state.evalFile(lookupFileArg(state, path), v); - state.autoCallFunction(*autoArgs, v, v2); + state.autoCallFunction(autoArgs.get(), v, v2); addAttrsToScope(v2); } diff --git a/third_party/nix/src/nix/search.cc b/third_party/nix/src/nix/search.cc index 06d5c207e..9db5b3d10 100644 --- a/third_party/nix/src/nix/search.cc +++ b/third_party/nix/src/nix/search.cc @@ -110,7 +110,8 @@ struct CmdSearch final : SourceExprCommand, MixJSON { if (v->type == tLambda && toplevel) { Value* v2 = state->allocValue(); - state->autoCallFunction(*Bindings::NewGC(), *v, *v2); + auto dummyArgs = Bindings::New(); + state->autoCallFunction(dummyArgs.get(), *v, *v2); v = v2; state->forceValue(*v); } diff --git a/third_party/nix/src/nix/upgrade-nix.cc b/third_party/nix/src/nix/upgrade-nix.cc index 158e3cd1c..eff51ba15 100644 --- a/third_party/nix/src/nix/upgrade-nix.cc +++ b/third_party/nix/src/nix/upgrade-nix.cc @@ -156,8 +156,9 @@ struct CmdUpgradeNix final : MixDryRun, StoreCommand { auto state = std::make_unique(Strings(), store); auto v = state->allocValue(); state->eval(state->parseExprFromString(*res.data, "/no-such-path"), *v); - Bindings& bindings(*Bindings::NewGC()); - auto v2 = findAlongAttrPath(*state, settings.thisSystem, bindings, *v); + std::unique_ptr bindings(Bindings::New()); + auto v2 = + findAlongAttrPath(*state, settings.thisSystem, bindings.get(), *v); return state->forceString(*v2); } diff --git a/third_party/nix/src/tests/attr-set.cc b/third_party/nix/src/tests/attr-set.cc index d52956f6a..84756c609 100644 --- a/third_party/nix/src/tests/attr-set.cc +++ b/third_party/nix/src/tests/attr-set.cc @@ -121,14 +121,8 @@ class AttrSetTest : public ::testing::Test { symbol_table = &eval_state_->symbols; } - void assert_bindings_equal(nix::Bindings& lhs, nix::Bindings& rhs) { - Value lhs_val; - Value rhs_val; - lhs_val.type = rhs_val.type = ValueType::tAttrs; - lhs_val.attrs = &lhs; - rhs_val.attrs = &lhs; - - RC_ASSERT(eval_state_->eqValues(lhs_val, rhs_val)); + void assert_bindings_equal(nix::Bindings* lhs, nix::Bindings* rhs) { + RC_ASSERT(lhs->Equal(rhs, *eval_state_)); } }; @@ -136,24 +130,26 @@ class AttrSetMonoidTest : public AttrSetTest {}; RC_GTEST_FIXTURE_PROP(AttrSetMonoidTest, mergeLeftIdentity, (nix::Bindings && bindings)) { - auto empty_bindings = nix::Bindings::NewGC(); - auto result = *Bindings::Merge(*empty_bindings, bindings); - assert_bindings_equal(result, bindings); + auto empty_bindings = nix::Bindings::New(); + auto result = Bindings::Merge(*empty_bindings, bindings); + assert_bindings_equal(result.get(), &bindings); } RC_GTEST_FIXTURE_PROP(AttrSetMonoidTest, mergeRightIdentity, (nix::Bindings && bindings)) { - auto empty_bindings = nix::Bindings::NewGC(); - auto result = *Bindings::Merge(bindings, *empty_bindings); - assert_bindings_equal(result, bindings); + auto empty_bindings = nix::Bindings::New(); + auto result = Bindings::Merge(bindings, *empty_bindings); + assert_bindings_equal(result.get(), &bindings); } RC_GTEST_FIXTURE_PROP(AttrSetMonoidTest, mergeAssociative, (nix::Bindings && bindings_1, nix::Bindings&& bindings_2, nix::Bindings&& bindings_3)) { - assert_bindings_equal( - *Bindings::Merge(bindings_1, *Bindings::Merge(bindings_2, bindings_3)), - *Bindings::Merge(*Bindings::Merge(bindings_1, bindings_2), bindings_3)); + auto b231 = + Bindings::Merge(bindings_1, *Bindings::Merge(bindings_2, bindings_3)); + auto b123 = + Bindings::Merge(*Bindings::Merge(bindings_1, bindings_2), bindings_3); + assert_bindings_equal(b231.get(), b123.get()); } } // namespace nix