Bump version to 1.6.2 while tweaking code, comments & readme

- Noticed that the regression test for `greedyClone(true)` does not
  fail when the bug is present. I fixed that.
- Tweaked the fix to `mergeSibling` and rewrote the comment on it.
- Updated the readme.

Author: qwertie

b+tree.d.ts

@@ -309,8 +309,8 @@ export default class BTree<K = any, V = any> implements ISortedMapF<K, V>, ISort
      *  This is known as copy-on-write behavior, or "lazy copying". */
     clone(): BTree<K, V>;
     /** Performs a greedy clone, immediately duplicating any nodes that are
-     *  not currently marked as shared, in order to avoid marking any nodes
-     *  as shared.
+     *  not currently marked as shared, in order to avoid marking any
+     *  additional nodes as shared.
      *  @param force Clone all nodes, even shared ones.
      */
     greedyClone(force?: boolean): BTree<K, V>;

b+tree.js

@@ -779,8 +779,8 @@ var BTree = /** @class */ (function () {
         return result;
     };
     /** Performs a greedy clone, immediately duplicating any nodes that are
-     *  not currently marked as shared, in order to avoid marking any nodes
-     *  as shared.
+     *  not currently marked as shared, in order to avoid marking any
+     *  additional nodes as shared.
      *  @param force Clone all nodes, even shared ones.
      */
     BTree.prototype.greedyClone = function (force) {
@@ -1392,7 +1392,7 @@ var BNodeInternal = /** @class */ (function (_super) {
     __extends(BNodeInternal, _super);
     /**
      * This does not mark `children` as shared, so it is the responsibility of the caller
-     * to ensure that either children are marked shared, or it are not included in another tree.
+     * to ensure children are either marked shared, or aren't included in another tree.
      */
     function BNodeInternal(children, keys) {
         var _this = this;
@@ -1635,10 +1635,9 @@ var BNodeInternal = /** @class */ (function (_super) {
         this.keys.push.apply(this.keys, rhs.keys);
         var rhsChildren = rhs.children;
         this.children.push.apply(this.children, rhsChildren);
-        if (rhs.isShared) {
-            // Because rhs might continue to be used in another tree since it is shared,
-            // this is adding a parent to its children instead of just changing what their parent is.
-            // Thus they need to be marked as shared.
+        if (rhs.isShared && !this.isShared) {
+            // All children of a shared node are implicitly shared, and since their new
+            // parent is not shared, they must now be explicitly marked as shared.
             for (var i = 0; i < rhsChildren.length; i++)
                 rhsChildren[i].isShared = true;
         }

b+tree.test.ts

@@ -430,6 +430,10 @@ describe("cloning and sharing tests", () => {
     }
     // Leaf nodes don't count, so this is depth 2
     expect(tree.height).toBe(2);
+
+    // To trigger the bug, mark children of the root node as shared (not just the root)
+    tree.clone().set(1, 1);
+    
     const clone = tree.greedyClone(true);
 
     // The bug was that `force` was not passed down. This meant that non-shared nodes below the second layer would not be cloned.

b+tree.ts

@@ -875,8 +875,8 @@ export default class BTree<K=any, V=any> implements ISortedMapF<K,V>, ISortedMap
   }
 
   /** Performs a greedy clone, immediately duplicating any nodes that are 
-   *  not currently marked as shared, in order to avoid marking any nodes
-   *  as shared.
+   *  not currently marked as shared, in order to avoid marking any 
+   *  additional nodes as shared.
    *  @param force Clone all nodes, even shared ones.
    */
   greedyClone(force?: boolean): BTree<K,V> {
@@ -1206,8 +1206,10 @@ class BNode<K,V> {
   // If this is an internal node, _keys[i] is the highest key in children[i].
   keys: K[];
   values: V[];
-  // True if this node might multiple parents (equivalently: could be in multiple b-trees).
-  // This means it must be cloned before being mutated to avoid changing an unrelated tree.
+  // True if this node might be within multiple `BTree`s (or have multiple parents).
+  // If so, it must be cloned before being mutated to avoid changing an unrelated tree.
+  // This is transitive: if it's true, children are also shared even if `isShared!=true`
+  // in those children. (Certain operations will propagate isShared=true to children.)
   isShared: true | undefined;
   get isLeaf() { return (this as any).children === undefined; }
 
@@ -1532,7 +1534,7 @@ class BNodeInternal<K,V> extends BNode<K,V> {
 
   /** 
    * This does not mark `children` as shared, so it is the responsibility of the caller
-   * to ensure that either children are marked shared, or it are not included in another tree.
+   * to ensure children are either marked shared, or aren't included in another tree.
    */
   constructor(children: BNode<K,V>[], keys?: K[]) {
     if (!keys) {
@@ -1793,10 +1795,9 @@ class BNodeInternal<K,V> extends BNode<K,V> {
     const rhsChildren = (rhs as any as BNodeInternal<K,V>).children;
     this.children.push.apply(this.children, rhsChildren);
 
-    if (rhs.isShared) {
-      // Because rhs might continue to be used in another tree since it is shared,
-      // this is adding a parent to its children instead of just changing what their parent is.
-      // Thus they need to be marked as shared.
+    if (rhs.isShared && !this.isShared) {
+      // All children of a shared node are implicitly shared, and since their new
+      // parent is not shared, they must now be explicitly marked as shared.
       for (var i = 0; i < rhsChildren.length; i++)
         rhsChildren[i].isShared = true;
     }

package.json

@@ -1,6 +1,6 @@
 {
   "name": "sorted-btree",
-  "version": "1.6.1",
+  "version": "1.6.2",
   "description": "A sorted list of key-value pairs in a fast, typed in-memory B+ tree with a powerful API.",
   "main": "b+tree.js",
   "typings": "b+tree",

readme.md

@@ -23,10 +23,10 @@ Features
   constructor argument).
 - Supports O(1) fast cloning with subtree sharing. This works by marking the
   root node as "shared between instances". This makes the tree read-only 
-  with copy-on-edit behavior; both copies of the tree remain mutable.
-  I call this category of data structure "dynamically persistent" because 
-  AFAIK no one else has given it a name; it walks the line between mutating 
-  and [persistent](https://en.wikipedia.org/wiki/Persistent_data_structure).
+  with copy-on-edit behavior; both copies of the tree remain mutable. I call 
+  this category of data structure "dynamically persistent" or "mutably 
+  persistent" because AFAIK no one else has given it a name; it walks the line 
+  between mutating and [persistent](https://en.wikipedia.org/wiki/Persistent_data_structure).
 - Includes persistent methods such as `with` and `without`, which return a
   modified tree without changing the original (in O(log(size)) time).
 - When a node fills up, items are shifted to siblings when possible to 
@@ -375,6 +375,11 @@ Benchmarks (in milliseconds for integer keys/values)
 Version history
 ---------------
 
+### v1.6.2 ###
+
+- Bug fixes: two rare situations were discovered in which shared nodes could fail to be marked as shared, and as a result, mutations could affect copies that should have been completely separate.
+- Bug fix: greedyClone(true) did not clone shared nodes recursively.
+
 ### v1.6.0 ###
 
 - Added `BTree.getPairOrNextLower` and `BTree.getPairOrNextHigher` methods (PR #23)