From 2aa3133c530b40ea0448d3c6a5507aeb40aaa558 Mon Sep 17 00:00:00 2001
From: Stein Somers <git@steinsomers.be>
Date: Mon, 13 Jul 2020 16:50:27 +0200
Subject: [PATCH] Add and fix BTreeMap comments

---
 src/liballoc/collections/btree/map.rs      |  2 ++
 src/liballoc/collections/btree/navigate.rs | 26 ++++++++++++----------
 src/liballoc/collections/btree/node.rs     | 15 ++++++++-----
 3 files changed, 25 insertions(+), 18 deletions(-)

diff --git a/src/liballoc/collections/btree/map.rs b/src/liballoc/collections/btree/map.rs
index bb9091a66594b..f3781db1cf784 100644
--- a/src/liballoc/collections/btree/map.rs
+++ b/src/liballoc/collections/btree/map.rs
@@ -1697,6 +1697,8 @@ where
     pred: F,
     inner: DrainFilterInner<'a, K, V>,
 }
+/// Most of the implementation of DrainFilter, independent of the type
+/// of the predicate, thus also serving for BTreeSet::DrainFilter.
 pub(super) struct DrainFilterInner<'a, K: 'a, V: 'a> {
     length: &'a mut usize,
     cur_leaf_edge: Option<Handle<NodeRef<marker::Mut<'a>, K, V, marker::Leaf>, marker::Edge>>,
diff --git a/src/liballoc/collections/btree/navigate.rs b/src/liballoc/collections/btree/navigate.rs
index 5478d822438b1..44f0e25bbd798 100644
--- a/src/liballoc/collections/btree/navigate.rs
+++ b/src/liballoc/collections/btree/navigate.rs
@@ -161,15 +161,16 @@ impl<'a, K, V> Handle<NodeRef<marker::Mut<'a>, K, V, marker::Leaf>, marker::Edge
 impl<K, V> Handle<NodeRef<marker::Owned, K, V, marker::Leaf>, marker::Edge> {
     /// Moves the leaf edge handle to the next leaf edge and returns the key and value
     /// in between, while deallocating any node left behind.
-    /// Unsafe for three reasons:
+    /// Unsafe for two reasons:
     /// - The caller must ensure that the leaf edge is not the last one in the tree
     ///   and is not a handle previously resulting from counterpart `next_back_unchecked`.
-    /// - If the leaf edge is the last edge of a node, that node and possibly ancestors
+    /// - Further use of the updated leaf edge handle is very dangerous. In particular,
+    ///   if the leaf edge is the last edge of a node, that node and possibly ancestors
     ///   will be deallocated, while the reference to those nodes in the surviving ancestor
-    ///   is left dangling; thus further use of the leaf edge handle is dangerous.
-    ///   It is, however, safe to call this method again on the updated handle.
-    ///   if the two preconditions above hold.
-    /// - Using the updated handle may well invalidate the returned references.
+    ///   is left dangling.
+    ///   The only safe way to proceed with the updated handle is to compare it, drop it,
+    ///   call this method again subject to both preconditions listed in the first point,
+    ///   or call counterpart `next_back_unchecked` subject to its preconditions.
     pub unsafe fn next_unchecked(&mut self) -> (K, V) {
         unsafe {
             replace(self, |leaf_edge| {
@@ -183,15 +184,16 @@ impl<K, V> Handle<NodeRef<marker::Owned, K, V, marker::Leaf>, marker::Edge> {
 
     /// Moves the leaf edge handle to the previous leaf edge and returns the key
     /// and value in between, while deallocating any node left behind.
-    /// Unsafe for three reasons:
+    /// Unsafe for two reasons:
     /// - The caller must ensure that the leaf edge is not the first one in the tree
     ///   and is not a handle previously resulting from counterpart `next_unchecked`.
-    /// - If the lead edge is the first edge of a node, that node and possibly ancestors
+    /// - Further use of the updated leaf edge handle is very dangerous. In particular,
+    ///   if the leaf edge is the first edge of a node, that node and possibly ancestors
     ///   will be deallocated, while the reference to those nodes in the surviving ancestor
-    ///   is left dangling; thus further use of the leaf edge handle is dangerous.
-    ///   It is, however, safe to call this method again on the updated handle.
-    ///   if the two preconditions above hold.
-    /// - Using the updated handle may well invalidate the returned references.
+    ///   is left dangling.
+    ///   The only safe way to proceed with the updated handle is to compare it, drop it,
+    ///   call this method again subject to both preconditions listed in the first point,
+    ///   or call counterpart `next_unchecked` subject to its preconditions.
     pub unsafe fn next_back_unchecked(&mut self) -> (K, V) {
         unsafe {
             replace(self, |leaf_edge| {
diff --git a/src/liballoc/collections/btree/node.rs b/src/liballoc/collections/btree/node.rs
index a4b6cf12a23bd..ce74d4f8ee688 100644
--- a/src/liballoc/collections/btree/node.rs
+++ b/src/liballoc/collections/btree/node.rs
@@ -94,7 +94,8 @@ struct InternalNode<K, V> {
     data: LeafNode<K, V>,
 
     /// The pointers to the children of this node. `len + 1` of these are considered
-    /// initialized and valid.
+    /// initialized and valid. Although during the process of `into_iter` or `drop`,
+    /// some pointers are dangling while others still need to be traversed.
     edges: [MaybeUninit<BoxedNode<K, V>>; 2 * B],
 }
 
@@ -408,7 +409,7 @@ impl<K, V> NodeRef<marker::Owned, K, V, marker::LeafOrInternal> {
 
 impl<'a, K, V, Type> NodeRef<marker::Mut<'a>, K, V, Type> {
     /// Unsafely asserts to the compiler some static information about whether this
-    /// node is a `Leaf`.
+    /// node is a `Leaf` or an `Internal`.
     unsafe fn cast_unchecked<NewType>(&mut self) -> NodeRef<marker::Mut<'_>, K, V, NewType> {
         NodeRef { height: self.height, node: self.node, root: self.root, _marker: PhantomData }
     }
@@ -515,7 +516,7 @@ impl<'a, K: 'a, V: 'a, Type> NodeRef<marker::Mut<'a>, K, V, Type> {
 }
 
 impl<'a, K, V> NodeRef<marker::Mut<'a>, K, V, marker::Leaf> {
-    /// Adds a key/value pair the end of the node.
+    /// Adds a key/value pair to the end of the node.
     pub fn push(&mut self, key: K, val: V) {
         assert!(self.len() < CAPACITY);
 
@@ -602,8 +603,10 @@ impl<'a, K, V> NodeRef<marker::Mut<'a>, K, V, marker::Internal> {
 }
 
 impl<'a, K, V> NodeRef<marker::Mut<'a>, K, V, marker::LeafOrInternal> {
-    /// Removes a key/value pair from the end of this node. If this is an internal node,
-    /// also removes the edge that was to the right of that pair.
+    /// Removes a key/value pair from the end of this node and returns the pair.
+    /// If this is an internal node, also removes the edge that was to the right
+    /// of that pair and returns the orphaned node that this edge owned with its
+    /// parent erased.
     pub fn pop(&mut self) -> (K, V, Option<Root<K, V>>) {
         assert!(self.len() > 0);
 
@@ -883,7 +886,7 @@ impl<'a, K, V> Handle<NodeRef<marker::Mut<'a>, K, V, marker::Internal>, marker::
     }
 
     /// Unsafely asserts to the compiler some static information about whether the underlying
-    /// node of this handle is a `Leaf`.
+    /// node of this handle is a `Leaf` or an `Internal`.
     unsafe fn cast_unchecked<NewType>(
         &mut self,
     ) -> Handle<NodeRef<marker::Mut<'_>, K, V, NewType>, marker::Edge> {