From e5f57005da683f454ee9f2dcb895c4cfac5be69e Mon Sep 17 00:00:00 2001
From: Hannes Janetzek <hannes.janetzek@gmail.com>
Date: Thu, 24 Nov 2016 16:31:18 +0100
Subject: [PATCH] remove shared_memory_holder indirection

- unnecessary level of indirection
---
 include/yaml-cpp/node/detail/impl.h      | 26 ++++++++++------------
 include/yaml-cpp/node/detail/iterator.h  |  4 ++--
 include/yaml-cpp/node/detail/memory.h    | 15 +------------
 include/yaml-cpp/node/detail/node.h      | 23 ++++++++++---------
 include/yaml-cpp/node/detail/node_data.h | 25 ++++++++++-----------
 include/yaml-cpp/node/impl.h             | 28 +++++++++++++++---------
 include/yaml-cpp/node/node.h             |  5 +++--
 src/memory.cpp                           | 15 +++----------
 src/node_data.cpp                        | 14 ++++++------
 src/nodebuilder.cpp                      |  2 +-
 src/nodebuilder.h                        |  2 +-
 src/nodeevents.h                         |  2 +-
 12 files changed, 72 insertions(+), 89 deletions(-)

diff --git a/include/yaml-cpp/node/detail/impl.h b/include/yaml-cpp/node/detail/impl.h
index 69f4476..e6a0dd0 100644
--- a/include/yaml-cpp/node/detail/impl.h
+++ b/include/yaml-cpp/node/detail/impl.h
@@ -16,7 +16,7 @@ namespace detail {
 template <typename Key, typename Enable = void>
 struct get_idx {
   static node* get(const std::vector<node*>& /* sequence */,
-                   const Key& /* key */, shared_memory_holder /* pMemory */) {
+                   const Key& /* key */, shared_memory /* pMemory */) {
     return 0;
   }
 };
@@ -26,12 +26,12 @@ struct get_idx<Key,
                typename std::enable_if<std::is_unsigned<Key>::value &&
                                        !std::is_same<Key, bool>::value>::type> {
   static node* get(const std::vector<node*>& sequence, const Key& key,
-                   shared_memory_holder /* pMemory */) {
+                   shared_memory /* pMemory */) {
     return key < sequence.size() ? sequence[key] : 0;
   }
 
   static node* get(std::vector<node*>& sequence, const Key& key,
-                   shared_memory_holder pMemory) {
+                   shared_memory pMemory) {
     if (key > sequence.size())
       return 0;
     if (key == sequence.size())
@@ -43,13 +43,13 @@ struct get_idx<Key,
 template <typename Key>
 struct get_idx<Key, typename std::enable_if<std::is_signed<Key>::value>::type> {
   static node* get(const std::vector<node*>& sequence, const Key& key,
-                   shared_memory_holder pMemory) {
+                   shared_memory pMemory) {
     return key >= 0 ? get_idx<std::size_t>::get(
                           sequence, static_cast<std::size_t>(key), pMemory)
                     : 0;
   }
   static node* get(std::vector<node*>& sequence, const Key& key,
-                   shared_memory_holder pMemory) {
+                   shared_memory pMemory) {
     return key >= 0 ? get_idx<std::size_t>::get(
                           sequence, static_cast<std::size_t>(key), pMemory)
                     : 0;
@@ -57,7 +57,7 @@ struct get_idx<Key, typename std::enable_if<std::is_signed<Key>::value>::type> {
 };
 
 template <typename T>
-inline bool node::equals(const T& rhs, shared_memory_holder pMemory) {
+inline bool node::equals(const T& rhs, shared_memory pMemory) {
   T lhs;
   if (convert<T>::decode(Node(*this, pMemory), lhs)) {
     return lhs == rhs;
@@ -65,14 +65,13 @@ inline bool node::equals(const T& rhs, shared_memory_holder pMemory) {
   return false;
 }
 
-inline bool node::equals(const char* rhs, shared_memory_holder pMemory) {
+inline bool node::equals(const char* rhs, shared_memory pMemory) {
   return equals<std::string>(rhs, pMemory);
 }
 
 // indexing
 template <typename Key>
-inline node* node_data::get(const Key& key,
-                            shared_memory_holder pMemory) const {
+inline node* node_data::get(const Key& key, shared_memory pMemory) const {
   switch (m_type) {
     case NodeType::Map:
       break;
@@ -97,7 +96,7 @@ inline node* node_data::get(const Key& key,
 }
 
 template <typename Key>
-inline node& node_data::get(const Key& key, shared_memory_holder pMemory) {
+inline node& node_data::get(const Key& key, shared_memory pMemory) {
   switch (m_type) {
     case NodeType::Map:
       break;
@@ -128,7 +127,7 @@ inline node& node_data::get(const Key& key, shared_memory_holder pMemory) {
 }
 
 template <typename Key>
-inline bool node_data::remove(const Key& key, shared_memory_holder pMemory) {
+inline bool node_data::remove(const Key& key, shared_memory pMemory) {
   if (m_type != NodeType::Map)
     return false;
 
@@ -145,7 +144,7 @@ inline bool node_data::remove(const Key& key, shared_memory_holder pMemory) {
 // map
 template <typename Key, typename Value>
 inline void node_data::force_insert(const Key& key, const Value& value,
-                                    shared_memory_holder pMemory) {
+                                    shared_memory pMemory) {
   switch (m_type) {
     case NodeType::Map:
       break;
@@ -164,8 +163,7 @@ inline void node_data::force_insert(const Key& key, const Value& value,
 }
 
 template <typename T>
-inline node& node_data::convert_to_node(const T& rhs,
-                                        shared_memory_holder pMemory) {
+inline node& node_data::convert_to_node(const T& rhs, shared_memory pMemory) {
   Node value = convert<T>::encode(rhs);
   value.EnsureNodeExists();
   pMemory->merge(*value.m_pMemory);
diff --git a/include/yaml-cpp/node/detail/iterator.h b/include/yaml-cpp/node/detail/iterator.h
index 82b00df..8c8e20a 100644
--- a/include/yaml-cpp/node/detail/iterator.h
+++ b/include/yaml-cpp/node/detail/iterator.h
@@ -39,7 +39,7 @@ class iterator_base : public std::iterator<std::forward_iterator_tag, V,
 
  public:
   iterator_base() : m_iterator(), m_pMemory(nullptr) {}
-  explicit iterator_base(base_type rhs, shared_memory_holder pMemory)
+  explicit iterator_base(base_type rhs, shared_memory pMemory)
       : m_iterator(rhs), m_pMemory(pMemory) {}
 
   template <class W>
@@ -82,7 +82,7 @@ class iterator_base : public std::iterator<std::forward_iterator_tag, V,
 
  private:
   base_type m_iterator;
-  shared_memory_holder m_pMemory;
+  shared_memory m_pMemory;
 };
 }
 }
diff --git a/include/yaml-cpp/node/detail/memory.h b/include/yaml-cpp/node/detail/memory.h
index 8189c54..4697d0c 100644
--- a/include/yaml-cpp/node/detail/memory.h
+++ b/include/yaml-cpp/node/detail/memory.h
@@ -25,7 +25,7 @@ namespace detail {
 class YAML_CPP_API memory : public ref_counted {
  public:
   node& create_node();
-  void merge(memory&& rhs);
+  void merge(memory& rhs);
 
   memory();
   ~memory();
@@ -37,19 +37,6 @@ class YAML_CPP_API memory : public ref_counted {
 
 typedef ref_holder<memory> shared_memory;
 
-class YAML_CPP_API memory_holder : public ref_counted {
- public:
-  memory_holder();
-  ~memory_holder();
-
-  node& create_node() { return m_pMemory->create_node(); }
-  void merge(memory_holder& rhs);
-
- private:
-  shared_memory m_pMemory;
-};
-
-typedef ref_holder<memory_holder> shared_memory_holder;
 }
 }
 
diff --git a/include/yaml-cpp/node/detail/node.h b/include/yaml-cpp/node/detail/node.h
index 3eae868..050b62b 100644
--- a/include/yaml-cpp/node/detail/node.h
+++ b/include/yaml-cpp/node/detail/node.h
@@ -39,8 +39,8 @@ class node {
   EmitterStyle::value style() const { return m_pRef->style(); }
 
   template <typename T>
-  bool equals(const T& rhs, shared_memory_holder pMemory);
-  bool equals(const char* rhs, shared_memory_holder pMemory);
+  bool equals(const T& rhs, shared_memory pMemory);
+  bool equals(const char* rhs, shared_memory pMemory);
 
   void set_ref(const node& rhs) {
     if (rhs.is_defined())
@@ -93,11 +93,11 @@ class node {
   node_iterator end() { return m_pRef->end(); }
 
   // sequence
-  void push_back(node& node, shared_memory_holder pMemory) {
+  void push_back(node& node, shared_memory pMemory) {
     m_pRef->push_back(node, pMemory);
     node.add_dependency(*this);
   }
-  void insert(node& key, node& value, shared_memory_holder pMemory) {
+  void insert(node& key, node& value, shared_memory pMemory) {
     m_pRef->insert(key, value, pMemory);
     key.add_dependency(*this);
     value.add_dependency(*this);
@@ -105,43 +105,42 @@ class node {
 
   // indexing
   template <typename Key>
-  node* get(const Key& key, shared_memory_holder pMemory) const {
+  node* get(const Key& key, shared_memory pMemory) const {
     // NOTE: this returns a non-const node so that the top-level Node can wrap
     // it, and returns a pointer so that it can be NULL (if there is no such
     // key).
     return static_cast<const node_data&>(*m_pRef).get(key, pMemory);
   }
   template <typename Key>
-  node& get(const Key& key, shared_memory_holder pMemory) {
+  node& get(const Key& key, shared_memory pMemory) {
     node& value = m_pRef->get(key, pMemory);
     value.add_dependency(*this);
     return value;
   }
   template <typename Key>
-  bool remove(const Key& key, shared_memory_holder pMemory) {
+  bool remove(const Key& key, shared_memory pMemory) {
     return m_pRef->remove(key, pMemory);
   }
 
-  node* get(node& key, shared_memory_holder pMemory) const {
+  node* get(node& key, shared_memory pMemory) const {
     // NOTE: this returns a non-const node so that the top-level Node can wrap
     // it, and returns a pointer so that it can be NULL (if there is no such
     // key).
     return static_cast<const node_data&>(*m_pRef).get(key, pMemory);
   }
-  node& get(node& key, shared_memory_holder pMemory) {
+  node& get(node& key, shared_memory pMemory) {
     node& value = m_pRef->get(key, pMemory);
     key.add_dependency(*this);
     value.add_dependency(*this);
     return value;
   }
-  bool remove(node& key, shared_memory_holder pMemory) {
+  bool remove(node& key, shared_memory pMemory) {
     return m_pRef->remove(key, pMemory);
   }
 
   // map
   template <typename Key, typename Value>
-  void force_insert(const Key& key, const Value& value,
-                    shared_memory_holder pMemory) {
+  void force_insert(const Key& key, const Value& value, shared_memory pMemory) {
     m_pRef->force_insert(key, value, pMemory);
   }
 
diff --git a/include/yaml-cpp/node/detail/node_data.h b/include/yaml-cpp/node/detail/node_data.h
index e6b4548..7d97b37 100644
--- a/include/yaml-cpp/node/detail/node_data.h
+++ b/include/yaml-cpp/node/detail/node_data.h
@@ -61,25 +61,24 @@ class YAML_CPP_API node_data : public ref_counted {
   node_iterator end();
 
   // sequence
-  void push_back(node& node, shared_memory_holder pMemory);
-  void insert(node& key, node& value, shared_memory_holder pMemory);
+  void push_back(node& node, shared_memory pMemory);
+  void insert(node& key, node& value, shared_memory pMemory);
 
   // indexing
   template <typename Key>
-  node* get(const Key& key, shared_memory_holder pMemory) const;
+  node* get(const Key& key, shared_memory pMemory) const;
   template <typename Key>
-  node& get(const Key& key, shared_memory_holder pMemory);
+  node& get(const Key& key, shared_memory pMemory);
   template <typename Key>
-  bool remove(const Key& key, shared_memory_holder pMemory);
+  bool remove(const Key& key, shared_memory pMemory);
 
-  node* get(node& key, shared_memory_holder pMemory) const;
-  node& get(node& key, shared_memory_holder pMemory);
-  bool remove(node& key, shared_memory_holder pMemory);
+  node* get(node& key, shared_memory pMemory) const;
+  node& get(node& key, shared_memory pMemory);
+  bool remove(node& key, shared_memory pMemory);
 
   // map
   template <typename Key, typename Value>
-  void force_insert(const Key& key, const Value& value,
-                    shared_memory_holder pMemory);
+  void force_insert(const Key& key, const Value& value, shared_memory pMemory);
 
  public:
   static std::string empty_scalar;
@@ -92,11 +91,11 @@ class YAML_CPP_API node_data : public ref_counted {
   void reset_map();
 
   void insert_map_pair(node& key, node& value);
-  void convert_to_map(shared_memory_holder pMemory);
-  void convert_sequence_to_map(shared_memory_holder pMemory);
+  void convert_to_map(shared_memory pMemory);
+  void convert_sequence_to_map(shared_memory pMemory);
 
   template <typename T>
-  static node& convert_to_node(const T& rhs, shared_memory_holder pMemory);
+  static node& convert_to_node(const T& rhs, shared_memory pMemory);
 
  private:
   bool m_isDefined;
diff --git a/include/yaml-cpp/node/impl.h b/include/yaml-cpp/node/impl.h
index 421f577..ddcf75c 100644
--- a/include/yaml-cpp/node/impl.h
+++ b/include/yaml-cpp/node/impl.h
@@ -15,17 +15,17 @@
 #include <string>
 
 namespace YAML {
-inline Node::Node() : m_pMemory(new detail::memory_holder), m_pNode(NULL) {}
+inline Node::Node() : m_pMemory(new detail::memory), m_pNode(NULL) {}
 
 inline Node::Node(NodeType::value type)
-    : m_pMemory(new detail::memory_holder),
+    : m_pMemory(new detail::memory),
       m_pNode(&(m_pMemory->create_node())) {
   m_pNode->set_type(type);
 }
 
 template <typename T>
 inline Node::Node(const T& rhs)
-    : m_pMemory(new detail::memory_holder),
+    : m_pMemory(new detail::memory),
       m_pNode(&(m_pMemory->create_node())) {
   Assign(rhs);
 }
@@ -43,7 +43,7 @@ inline Node::Node(Node&& rhs)
   rhs.m_pNode = nullptr;
 }
 
-inline Node::Node(detail::node& node, detail::shared_memory_holder pMemory)
+inline Node::Node(detail::node& node, detail::shared_memory pMemory)
     : m_pMemory(pMemory), m_pNode(&node) {}
 
 inline Node::~Node() {}
@@ -52,7 +52,7 @@ inline void Node::EnsureNodeExists() const {
   if (!isValid())
     throw InvalidNode();
   if (!m_pNode) {
-    m_pMemory.reset(new detail::memory_holder);
+    m_pMemory.reset(new detail::memory);
     m_pNode = &m_pMemory->create_node();
     m_pNode->set_null();
   }
@@ -257,6 +257,12 @@ inline Node& Node::operator=(Node&& rhs) {
   return *this;
 }
 
+inline void Node::mergeMemory(const Node& rhs) const {
+  if (m_pMemory != rhs.m_pMemory) {
+      m_pMemory->merge(*rhs.m_pMemory);
+      rhs.m_pMemory = m_pMemory;
+  }
+}
 inline void Node::AssignData(Node&& rhs) {
   if (!isValid() || !rhs.isValid())
     throw InvalidNode();
@@ -264,7 +270,7 @@ inline void Node::AssignData(Node&& rhs) {
   rhs.EnsureNodeExists();
 
   m_pNode->set_data(std::move(*rhs.m_pNode));
-  m_pMemory->merge(*rhs.m_pMemory);
+  mergeMemory(rhs);
 }
 
 inline void Node::AssignNode(const Node& rhs) {
@@ -279,7 +285,7 @@ inline void Node::AssignNode(const Node& rhs) {
   }
 
   m_pNode->set_ref(*rhs.m_pNode);
-  m_pMemory->merge(*rhs.m_pMemory);
+  mergeMemory(rhs);
   m_pNode = rhs.m_pNode;
 }
 
@@ -330,7 +336,7 @@ inline void Node::push_back(const Node& rhs) {
   rhs.EnsureNodeExists();
 
   m_pNode->push_back(*rhs.m_pNode, m_pMemory);
-  m_pMemory->merge(*rhs.m_pMemory);
+  mergeMemory(rhs);
 }
 
 // helpers for indexing
@@ -414,7 +420,8 @@ inline const Node Node::operator[](const Node& key) const {
     throw InvalidNode();
   EnsureNodeExists();
   key.EnsureNodeExists();
-  m_pMemory->merge(*key.m_pMemory);
+  mergeMemory(key);
+
   detail::node* value =
       static_cast<const detail::node&>(*m_pNode).get(*key.m_pNode, m_pMemory);
   if (!value) {
@@ -428,7 +435,8 @@ inline Node Node::operator[](const Node& key) {
     throw InvalidNode();
   EnsureNodeExists();
   key.EnsureNodeExists();
-  m_pMemory->merge(*key.m_pMemory);
+  mergeMemory(key);
+
   detail::node& value = m_pNode->get(*key.m_pNode, m_pMemory);
   return Node(value, m_pMemory);
 }
diff --git a/include/yaml-cpp/node/node.h b/include/yaml-cpp/node/node.h
index a42824b..4233893 100644
--- a/include/yaml-cpp/node/node.h
+++ b/include/yaml-cpp/node/node.h
@@ -119,7 +119,7 @@ class YAML_CPP_API Node {
  private:
   enum Zombie { ZombieNode };
   explicit Node(Zombie);
-  explicit Node(detail::node& node, detail::shared_memory_holder pMemory);
+  explicit Node(detail::node& node, detail::shared_memory pMemory);
 
   void EnsureNodeExists() const;
 
@@ -132,10 +132,11 @@ class YAML_CPP_API Node {
   void AssignNode(const Node& rhs);
 
  private:
-  mutable detail::shared_memory_holder m_pMemory;
+  mutable detail::shared_memory m_pMemory;
   mutable detail::node* m_pNode;
 
   bool isValid() const { return m_pMemory; }
+  void mergeMemory(const Node& rhs) const;
 };
 
 YAML_CPP_API bool operator==(const Node& lhs, const Node& rhs);
diff --git a/src/memory.cpp b/src/memory.cpp
index 59c87aa..7466b5d 100644
--- a/src/memory.cpp
+++ b/src/memory.cpp
@@ -7,23 +7,14 @@ namespace detail {
 
 typedef ref_holder<node> node_ref;
 
-void memory_holder::merge(memory_holder& rhs) {
-  if (m_pMemory == rhs.m_pMemory)
-    return;
-
-  m_pMemory->merge(std::move(*rhs.m_pMemory));
-  rhs.m_pMemory = m_pMemory;
-}
-
 node& memory::create_node() {
   m_nodes.emplace_back();
   return m_nodes.back();
 }
 
-void memory::merge(memory&& rhs) { m_nodes.splice(m_nodes.end(), rhs.m_nodes); }
-
-memory_holder::memory_holder() : m_pMemory(new memory) {}
-memory_holder::~memory_holder() {}
+void memory::merge(memory& rhs) {
+    m_nodes.splice(m_nodes.end(), rhs.m_nodes);
+}
 
 memory::memory() {}
 memory::~memory() {}
diff --git a/src/node_data.cpp b/src/node_data.cpp
index 77cd465..9a39ace 100644
--- a/src/node_data.cpp
+++ b/src/node_data.cpp
@@ -166,7 +166,7 @@ node_iterator node_data::end() {
 }
 
 // sequence
-void node_data::push_back(node& node, shared_memory_holder /* pMemory */) {
+void node_data::push_back(node& node, shared_memory /* pMemory */) {
   if (m_type == NodeType::Undefined || m_type == NodeType::Null) {
     m_type = NodeType::Sequence;
     reset_sequence();
@@ -178,7 +178,7 @@ void node_data::push_back(node& node, shared_memory_holder /* pMemory */) {
   m_sequence.push_back(&node);
 }
 
-void node_data::insert(node& key, node& value, shared_memory_holder pMemory) {
+void node_data::insert(node& key, node& value, shared_memory pMemory) {
   switch (m_type) {
     case NodeType::Map:
       break;
@@ -195,7 +195,7 @@ void node_data::insert(node& key, node& value, shared_memory_holder pMemory) {
 }
 
 // indexing
-node* node_data::get(node& key, shared_memory_holder /* pMemory */) const {
+node* node_data::get(node& key, shared_memory /* pMemory */) const {
   if (m_type != NodeType::Map) {
     return NULL;
   }
@@ -208,7 +208,7 @@ node* node_data::get(node& key, shared_memory_holder /* pMemory */) const {
   return NULL;
 }
 
-node& node_data::get(node& key, shared_memory_holder pMemory) {
+node& node_data::get(node& key, shared_memory pMemory) {
   switch (m_type) {
     case NodeType::Map:
       break;
@@ -231,7 +231,7 @@ node& node_data::get(node& key, shared_memory_holder pMemory) {
   return value;
 }
 
-bool node_data::remove(node& key, shared_memory_holder /* pMemory */) {
+bool node_data::remove(node& key, shared_memory /* pMemory */) {
   if (m_type != NodeType::Map)
     return false;
 
@@ -262,7 +262,7 @@ void node_data::insert_map_pair(node& key, node& value) {
     m_undefinedPairs.emplace_back(&key, &value);
 }
 
-void node_data::convert_to_map(shared_memory_holder pMemory) {
+void node_data::convert_to_map(shared_memory pMemory) {
   switch (m_type) {
     case NodeType::Undefined:
     case NodeType::Null:
@@ -280,7 +280,7 @@ void node_data::convert_to_map(shared_memory_holder pMemory) {
   }
 }
 
-void node_data::convert_sequence_to_map(shared_memory_holder pMemory) {
+void node_data::convert_sequence_to_map(shared_memory pMemory) {
   assert(m_type == NodeType::Sequence);
 
   reset_map();
diff --git a/src/nodebuilder.cpp b/src/nodebuilder.cpp
index 093d2ef..41f52e4 100644
--- a/src/nodebuilder.cpp
+++ b/src/nodebuilder.cpp
@@ -11,7 +11,7 @@ namespace YAML {
 struct Mark;
 
 NodeBuilder::NodeBuilder()
-    : m_pMemory(new detail::memory_holder), m_pRoot(0), m_mapDepth(0) {
+    : m_pMemory(new detail::memory), m_pRoot(0), m_mapDepth(0) {
   m_anchors.push_back(0);  // since the anchors start at 1
 }
 
diff --git a/src/nodebuilder.h b/src/nodebuilder.h
index e0eb0b8..87413ab 100644
--- a/src/nodebuilder.h
+++ b/src/nodebuilder.h
@@ -54,7 +54,7 @@ class NodeBuilder : public EventHandler {
   void RegisterAnchor(anchor_t anchor, detail::node& node);
 
  private:
-  detail::shared_memory_holder m_pMemory;
+  detail::shared_memory m_pMemory;
   detail::node* m_pRoot;
 
   typedef std::vector<detail::node*> Nodes;
diff --git a/src/nodeevents.h b/src/nodeevents.h
index 2bd5af0..665c3f2 100644
--- a/src/nodeevents.h
+++ b/src/nodeevents.h
@@ -54,7 +54,7 @@ class NodeEvents {
   bool IsAliased(const detail::node& node) const;
 
  private:
-  detail::shared_memory_holder m_pMemory;
+  detail::shared_memory m_pMemory;
   detail::node* m_root;
 
   typedef std::map<const detail::node_data*, int> RefCount;