From 1b919f52e928e8a72d5acba9175eae32ed4b0c90 Mon Sep 17 00:00:00 2001
From: Andrew Noyes <andrew.noyes@snowflake.com>
Date: Wed, 30 Mar 2022 16:29:35 -0700
Subject: [PATCH] Combine vector_like_traits::{insert,reserve} (#6689)

* Combine vector_like_traits::{insert,reserve}

and explain semantics better. This should make it more clear what
implementers need to do when implementing the vector_like_traits
concept.

* Update std::unordered_set vector_like_traits impl
---
 flow/Arena.h                  | 14 ++++---
 flow/ObjectSerializerTraits.h | 10 +++--
 flow/flat_buffers.h           | 77 ++++++++++++++++++++---------------
 3 files changed, 61 insertions(+), 40 deletions(-)

diff --git a/flow/Arena.h b/flow/Arena.h
index 59923d67a8..88e6ee6a0c 100644
--- a/flow/Arena.h
+++ b/flow/Arena.h
@@ -1470,15 +1470,19 @@ struct vector_like_traits<VectorRef<T, VecSerStrategy::FlatBuffers>> : std::true
 	static size_t num_entries(const VectorRef<T>& v, Context&) {
 		return v.size();
 	}
-	template <class Context>
-	static void reserve(VectorRef<T>& v, size_t s, Context& context) {
-		v.resize(context.arena(), s);
-	}
 
+	// Return an insert_iterator starting with an empty vector. |size| is the
+	// number of elements to be inserted. Implementations may want to allocate
+	// enough memory up front to hold |size| elements.
 	template <class Context>
-	static insert_iterator insert(Vec& v, Context&) {
+	static insert_iterator insert(Vec& v, size_t s, Context& context) {
+		// Logically v should be empty after this function returns, but since we're going to
+		// insert s times into the raw pointer insert_iterator it will end up
+		// with the correct size after deserialization finishes.
+		v.resize(context.arena(), s);
 		return v.begin();
 	}
+
 	template <class Context>
 	static iterator begin(const Vec& v, Context&) {
 		return v.begin();
diff --git a/flow/ObjectSerializerTraits.h b/flow/ObjectSerializerTraits.h
index 979db897e3..d396bd5808 100644
--- a/flow/ObjectSerializerTraits.h
+++ b/flow/ObjectSerializerTraits.h
@@ -108,13 +108,17 @@ struct vector_like_traits : std::false_type {
 	using iterator = void;
 	using insert_iterator = void;
 
+	// The number of entries in this vector
 	template <class Context>
 	static size_t num_entries(VectorLike&, Context&);
-	template <class Context>
-	static void reserve(VectorLike&, size_t, Context&);
 
+	// Return an insert_iterator starting with an empty vector. |size| is the
+	// number of elements to be inserted. Implementations may want to allocate
+	// enough memory up front to hold |size| elements.
 	template <class Context>
-	static insert_iterator insert(VectorLike&, Context&);
+	static insert_iterator insert(VectorLike&, size_t size, Context&);
+
+	// Return an iterator to read from this vector.
 	template <class Context>
 	static iterator begin(const VectorLike&, Context&);
 };
diff --git a/flow/flat_buffers.h b/flow/flat_buffers.h
index 5b98f74536..c1445d92dd 100644
--- a/flow/flat_buffers.h
+++ b/flow/flat_buffers.h
@@ -115,16 +115,17 @@ struct vector_like_traits<std::vector<T, Alloc>> : std::true_type {
 	static size_t num_entries(const Vec& v, Context&) {
 		return v.size();
 	}
+
+	// Return an insert_iterator starting with an empty vector. |size| is the
+	// number of elements to be inserted. Implementations may want to allocate
+	// enough memory up front to hold |size| elements.
 	template <class Context>
-	static void reserve(Vec& v, size_t size, Context&) {
+	static insert_iterator insert(Vec& v, size_t size, Context&) {
 		v.clear();
 		v.reserve(size);
-	}
-
-	template <class Context>
-	static insert_iterator insert(Vec& v, Context&) {
 		return std::back_inserter(v);
 	}
+
 	template <class Context>
 	static iterator begin(const Vec& v, Context&) {
 		return v.begin();
@@ -142,16 +143,16 @@ struct vector_like_traits<std::deque<T, Alloc>> : std::true_type {
 	static size_t num_entries(const Deq& v, Context&) {
 		return v.size();
 	}
-	template <class Context>
-	static void reserve(Deq& v, size_t size, Context&) {
-		v.resize(size);
-		v.clear();
-	}
 
+	// Return an insert_iterator starting with an empty vector. |size| is the
+	// number of elements to be inserted. Implementations may want to allocate
+	// enough memory up front to hold |size| elements.
 	template <class Context>
-	static insert_iterator insert(Deq& v, Context&) {
+	static insert_iterator insert(Deq& v, size_t size, Context&) {
+		v.clear();
 		return std::back_inserter(v);
 	}
+
 	template <class Context>
 	static iterator begin(const Deq& v, Context&) {
 		return v.begin();
@@ -169,12 +170,15 @@ struct vector_like_traits<std::array<T, N>> : std::true_type {
 	static size_t num_entries(const Vec& v, Context&) {
 		return N;
 	}
+
+	// Return an insert_iterator starting with an empty vector. |size| is the
+	// number of elements to be inserted. Implementations may want to allocate
+	// enough memory up front to hold |size| elements.
 	template <class Context>
-	static void reserve(Vec& v, size_t size, Context&) {}
-	template <class Context>
-	static insert_iterator insert(Vec& v, Context&) {
+	static insert_iterator insert(Vec& v, size_t s, Context&) {
 		return v.begin();
 	}
+
 	template <class Context>
 	static iterator begin(const Vec& v, Context&) {
 		return v.begin();
@@ -192,13 +196,16 @@ struct vector_like_traits<std::map<Key, T, Compare, Allocator>> : std::true_type
 	static size_t num_entries(const Vec& v, Context&) {
 		return v.size();
 	}
-	template <class Context>
-	static void reserve(Vec& v, size_t size, Context&) {}
 
+	// Return an insert_iterator starting with an empty vector. |size| is the
+	// number of elements to be inserted. Implementations may want to allocate
+	// enough memory up front to hold |size| elements.
 	template <class Context>
-	static insert_iterator insert(Vec& v, Context&) {
+	static insert_iterator insert(Vec& v, size_t s, Context&) {
+		v.clear();
 		return std::inserter(v, v.end());
 	}
+
 	template <class Context>
 	static iterator begin(const Vec& v, Context&) {
 		return v.begin();
@@ -215,13 +222,17 @@ struct vector_like_traits<std::unordered_map<Key, T, Hash, Pred, Allocator>> : s
 	static size_t num_entries(const Vec& v, Context&) {
 		return v.size();
 	}
-	template <class Context>
-	static void reserve(Vec& v, size_t size, Context&) {}
 
+	// Return an insert_iterator starting with an empty vector. |size| is the
+	// number of elements to be inserted. Implementations may want to allocate
+	// enough memory up front to hold |size| elements.
 	template <class Context>
-	static insert_iterator insert(Vec& v, Context&) {
+	static insert_iterator insert(Vec& v, size_t size, Context&) {
+		v.clear();
+		v.reserve(size);
 		return std::inserter(v, v.end());
 	}
+
 	template <class Context>
 	static iterator begin(const Vec& v, Context&) {
 		return v.begin();
@@ -239,13 +250,16 @@ struct vector_like_traits<std::set<Key, Compare, Allocator>> : std::true_type {
 	static size_t num_entries(const Vec& v, Context&) {
 		return v.size();
 	}
-	template <class Context>
-	static void reserve(Vec&, size_t, Context&) {}
 
+	// Return an insert_iterator starting with an empty vector. |size| is the
+	// number of elements to be inserted. Implementations may want to allocate
+	// enough memory up front to hold |size| elements.
 	template <class Context>
-	static insert_iterator insert(Vec& v, Context&) {
+	static insert_iterator insert(Vec& v, size_t size, Context&) {
+		v.clear();
 		return std::inserter(v, v.end());
 	}
+
 	template <class Context>
 	static iterator begin(const Vec& v, Context&) {
 		return v.begin();
@@ -262,15 +276,16 @@ struct vector_like_traits<std::unordered_set<Key, Hash, KeyEqual, Allocator>> :
 	static size_t num_entries(const Vec& v, Context&) {
 		return v.size();
 	}
-	template <class Context>
-	static void reserve(Vec& v, size_t size, Context&) {
-		v.reserve(size);
-	}
 
+	// Return an insert_iterator starting with an empty vector. |size| is the
+	// number of elements to be inserted. Implementations may want to allocate
+	// enough memory up front to hold |size| elements.
 	template <class Context>
-	static insert_iterator insert(Vec& v, Context&) {
+	static insert_iterator insert(Vec& v, size_t size, Context&) {
+		v.reserve(size);
 		return std::inserter(v, v.end());
 	}
+
 	template <class Context>
 	static iterator begin(const Vec& v, Context&) {
 		return v.begin();
@@ -946,8 +961,7 @@ struct LoadMember {
 			current += current_offset;
 			uint32_t numEntries = interpret_as<uint32_t>(current);
 			current += sizeof(uint32_t);
-			VectorTraits::reserve(member, numEntries, context);
-			auto inserter = VectorTraits::insert(member, context);
+			auto inserter = VectorTraits::insert(member, numEntries, context);
 			for (int i = 0; i < numEntries; ++i) {
 				T value;
 				if (types_current[i] > 0) {
@@ -1082,8 +1096,7 @@ struct LoadSaveHelper : Context {
 		current += current_offset;
 		uint32_t numEntries = interpret_as<uint32_t>(current);
 		current += sizeof(uint32_t);
-		VectorTraits::reserve(member, numEntries, this->context());
-		auto inserter = VectorTraits::insert(member, this->context());
+		auto inserter = VectorTraits::insert(member, numEntries, this->context());
 		for (uint32_t i = 0; i < numEntries; ++i) {
 			T value;
 			load_helper(value, current, this->context());