refactor(tvix): always pass Bindings by ptr, use shared/unique_ptr r/1658

Value now carries a shared_ptr<Bindings>, and all Bindings constructors return a unique_ptr<Bindings>.

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 <grfn@gws.fyi>
Reviewed-by: tazjin <mail@tazj.in>

22 files changed, 125 insertions, 103 deletions

diff --git a/third_party/nix/src/libexpr/attr-path.cc b/third_party/nix/src/libexpr/attr-path.cc
index a267e82b6e..86ebeec2fb 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 a71ceb9a06..97170be840 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 e9e2a2bade..808ffce176 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(); }
+bool Bindings::Equal(const Bindings* other, EvalState& state) const {
+  if (this == other) {
+    return true;
+  }
+
+  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::iterator Bindings::begin() { return attributes_.begin(); }
 Bindings::iterator Bindings::end() { return attributes_.end(); }
 
-Bindings* Bindings::NewGC(size_t capacity) {
+Bindings::const_iterator Bindings::cbegin() const {
+  return attributes_.cbegin();
+}
+
+Bindings::const_iterator Bindings::cend() const { return attributes_.cend(); }
+
+std::unique_ptr<Bindings> Bindings::New(size_t capacity) {
   if (capacity == 0) {
-    return &ZERO_BINDINGS;
+    // TODO(tazjin): A lot of 0-capacity Bindings are allocated.
+    // It would be nice to optimize that.
   }
 
-  return new Bindings;
+  return std::make_unique<Bindings>();
 }
 
-Bindings* Bindings::Merge(const Bindings& lhs, const Bindings& rhs) {
-  auto bindings = NewGC(lhs.size() + rhs.size());
+std::unique_ptr<Bindings> 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 6397008517..c02f3930fd 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<Symbol, Attr>;
 
 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<Bindings> 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<Bindings> 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 b3e3782732..e36457f4f8 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<Bindings> 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 be7126c039..9b0a77c6f7 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<Bindings> getAutoArgs(EvalState& state);
 
   Strings searchPath;
 
diff --git a/third_party/nix/src/libexpr/eval.cc b/third_party/nix/src/libexpr/eval.cc
index 4ae3661b0b..569e04e3a7 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 f3e38cfb3c..aa7af777a1 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 02ddae1f88..d81fb5dfad 100644
--- a/third_party/nix/src/libexpr/get-drvs.cc
+++ b/third_party/nix/src/libexpr/get-drvs.cc
@@ -4,6 +4,7 @@
 #include <regex>
 #include <utility>
 
+#include <absl/container/flat_hash_set.h>
 #include <absl/strings/numbers.h>
 #include <glog/logging.h>
 
@@ -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<Bindings> attrs)
     : state(&state), attrs(attrs), attrPath(std::move(attrPath)) {}
 
 DrvInfo::DrvInfo(EvalState& state, const ref<Store>& 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<Bindings> old = meta;
+  meta = std::shared_ptr<Bindings>(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<Bindings*>;
+using Done = absl::flat_hash_set<std::shared_ptr<Bindings>>;
 
 /* 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 608713ed1b..b17efe8b33 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<Bindings> attrs = nullptr;
+  std::shared_ptr<Bindings> 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<Bindings> attrs);
   DrvInfo(EvalState& state, const ref<Store>& store,
           const std::string& drvPathWithOutputs);
 
@@ -75,7 +77,7 @@ std::optional<DrvInfo> 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 85ce9e7a1b..82360c5668 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<Bindings> 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 85b8a32462..1fb8a2f3ad 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<EvalState>(myArgs.searchPath, store);
   state->repair = repair;
 
-  Bindings& autoArgs = *myArgs.getAutoArgs(*state);
+  std::unique_ptr<Bindings> 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 396f65d0d6..7502a648d8 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<Bindings> 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 0cb921c824..dac0c52a81 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(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 7b8d6dd3d6..236037299d 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<Bindings> 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 5454c6cd10..66e7cff810 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<EvalState>(myArgs.searchPath, store);
 
-    Bindings& autoArgs = *myArgs.getAutoArgs(*state);
+    std::unique_ptr<Bindings> 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 a18cba292e..644caa7cbe 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 b257a26f23..dd1202586a 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<Bindings> 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<Bindings> 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 85750b08ce..2607655f9b 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<Bindings> 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 06d5c207e5..9db5b3d103 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 158e3cd1c2..eff51ba158 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<EvalState>(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(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 d52956f6ac..84756c6094 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