From 1fc9ba4885f5a16e263bcc5e58bef68e3aa32cea Mon Sep 17 00:00:00 2001
From: Kane York <kanepyork@gmail.com>
Date: Thu, 13 Aug 2020 16:40:27 -0700
Subject: refactor(tvix): always pass Bindings by ptr, use shared/unique_ptr

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>
---
 third_party/nix/src/libexpr/attr-set.cc | 46 +++++++++++++++++++++++++--------
 1 file changed, 35 insertions(+), 11 deletions(-)

(limited to 'third_party/nix/src/libexpr/attr-set.cc')

diff --git a/third_party/nix/src/libexpr/attr-set.cc b/third_party/nix/src/libexpr/attr-set.cc
index e9e2a2baded0..808ffce176ee 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;
 }
-- 
cgit 1.4.1