refactor(3p/nix): Abstract away concrete bindings impl r/1283

To pave the way for the thing we want to do eventually which is use a
linear-time array for bindings (aka attribute sets) that are statically
known to be small enough to get a performance benefit from doing so,
make the Bindings class abstract, and define a BTreeBindings class that
inherits from it and is (currently always) returned from the static
initializer. The idea is that we'll have an ArrayBindings class as well
later that we can dispatch to conditionally based on an optional
"capacity" parameter or something like that.

There was some difficulty here in getting the iterator to work - the
approach we settled on ended up making a concrete BindingsIterator class
which will wrap a std::variant of either a btree iterator or something
else later, but right now just wraps a btree iterator.

Paired-With: Luke Granger-Brown <git@lukegb.com>
Paired-With: Vincent Ambo <mail@tazj.in>
Paired-With: Perry Lorier <isomer@tvl.fyi>
Change-Id: Ie02ca5a1c55e8ebf99ab1e957110bd9284278907
Reviewed-on: https://cl.tvl.fyi/c/depot/+/1121
Tested-by: BuildkiteCI
Reviewed-by: isomer <isomer@tvl.fyi>

1 files changed, 42 insertions, 10 deletions

diff --git a/third_party/nix/src/libexpr/attr-set.hh b/third_party/nix/src/libexpr/attr-set.hh
index d4823cb19712..06ae8f4b0723 100644
--- a/third_party/nix/src/libexpr/attr-set.hh
+++ b/third_party/nix/src/libexpr/attr-set.hh
@@ -1,6 +1,8 @@
 // This file implements the underlying structure of Nix attribute sets.
 #pragma once
 
+#include <cstddef>
+
 #include <absl/container/btree_map.h>
 #include <gc/gc_allocator.h>
 
@@ -28,38 +30,68 @@ using AttributeMap =
     absl::btree_map<Symbol, Attr, std::less<Symbol>,
                     gc_allocator<std::pair<const Symbol, Attr>>>;
 
+class BindingsIterator : public std::iterator<std::forward_iterator_tag,
+                                              std::pair<const Symbol, Attr>> {
+  friend class Bindings;
+  friend class BTreeBindings;
+
+ public:
+  BindingsIterator() : _iterator(){};
+  BindingsIterator& operator++();
+  BindingsIterator operator++(int);
+  bool operator==(const BindingsIterator& other) const;
+  bool operator!=(const BindingsIterator& other) const;
+  reference operator*() const;
+  pointer operator->() const { return &operator*(); }
+  BindingsIterator& operator=(const BindingsIterator& other) {
+    _iterator = other._iterator;
+    return *this;
+  }
+
+ protected:
+  explicit BindingsIterator(AttributeMap::iterator&& iterator)
+      : _iterator(iterator){};
+
+ private:
+  AttributeMap::iterator _iterator;
+};
+
 class Bindings {
  public:
-  typedef AttributeMap::iterator iterator;
+  typedef BindingsIterator iterator;
 
   // Allocate a new attribute set that is visible to the garbage
   // collector.
   static Bindings* NewGC();
 
+  // Allocate a new attribute set with a static capacity that is visible to the
+  // garbage collector.
+  // static Bindings* NewGC(size_t capacity);
+
   // Return the number of contained elements.
-  size_t size();
+  virtual size_t size() = 0;
 
   // Is this attribute set empty?
-  bool empty();
+  virtual bool empty() = 0;
 
   // Insert, but do not replace, values in the attribute set.
-  void push_back(const Attr& attr);
+  virtual void push_back(const Attr& attr) = 0;
 
   // Insert a value, or replace an existing one.
-  void insert_or_assign(const Attr& attr);
+  virtual void insert_or_assign(const Attr& attr) = 0;
 
   // Look up a specific element of the attribute set.
-  iterator find(const Symbol& name);
+  virtual iterator find(const Symbol& name) = 0;
 
   // TODO
-  iterator begin();
-  iterator end();
+  virtual iterator begin() = 0;
+  virtual iterator end() = 0;
 
   // Merge values from other into this attribute set.
-  void merge(const Bindings& other);
+  virtual void merge(Bindings& other) = 0;
 
   // TODO: can callers just iterate?
-  [[deprecated]] std::vector<const Attr*> lexicographicOrder();
+  [[deprecated]] virtual std::vector<const Attr*> lexicographicOrder() = 0;
 
   // oh no
   friend class EvalState;