Rollup merge of #145628 - tinnamchoi:fix-btree-append, r=Amanieu

[std][BTree] Fix behavior of `::append` to match documentation, `::insert`, and `::extend`

Resolves #145614

Author: Zalathar

library/alloc/src/collections/btree/append.rs

@@ -104,9 +104,14 @@ where
 {
     type Item = (K, V);
 
-    /// If two keys are equal, returns the key-value pair from the right source.
+    /// If two keys are equal, returns the key from the left and the value from the right.
     fn next(&mut self) -> Option<(K, V)> {
         let (a_next, b_next) = self.0.nexts(|a: &(K, V), b: &(K, V)| K::cmp(&a.0, &b.0));
-        b_next.or(a_next)
+        match (a_next, b_next) {
+            (Some((a_k, _)), Some((_, b_v))) => Some((a_k, b_v)),
+            (Some(a), None) => Some(a),
+            (None, Some(b)) => Some(b),
+            (None, None) => None,
+        }
     }
 }

library/alloc/src/collections/btree/map.rs

@@ -1181,6 +1181,10 @@ impl<K, V, A: Allocator + Clone> BTreeMap<K, V, A> {
     ///
     /// If a key from `other` is already present in `self`, the respective
     /// value from `self` will be overwritten with the respective value from `other`.
+    /// Similar to [`insert`], though, the key is not overwritten,
+    /// which matters for types that can be `==` without being identical.
+    ///
+    /// [`insert`]: BTreeMap::insert
     ///
     /// # Examples
     ///

library/alloc/src/collections/btree/map/tests.rs

@@ -11,7 +11,7 @@ use crate::fmt::Debug;
 use crate::rc::Rc;
 use crate::string::{String, ToString};
 use crate::testing::crash_test::{CrashTestDummy, Panic};
-use crate::testing::ord_chaos::{Cyclic3, Governed, Governor};
+use crate::testing::ord_chaos::{Cyclic3, Governed, Governor, IdBased};
 use crate::testing::rng::DeterministicRng;
 
 // Minimum number of elements to insert, to guarantee a tree with 2 levels,
@@ -2137,9 +2137,9 @@ fn test_append_drop_leak() {
     let mut left = BTreeMap::new();
     let mut right = BTreeMap::new();
     left.insert(a.spawn(Panic::Never), ());
-    left.insert(b.spawn(Panic::InDrop), ()); // first duplicate key, dropped during append
+    left.insert(b.spawn(Panic::Never), ());
     left.insert(c.spawn(Panic::Never), ());
-    right.insert(b.spawn(Panic::Never), ());
+    right.insert(b.spawn(Panic::InDrop), ()); // first duplicate key, dropped during append
     right.insert(c.spawn(Panic::Never), ());
 
     catch_unwind(move || left.append(&mut right)).unwrap_err();
@@ -2587,3 +2587,31 @@ fn cursor_peek_prev_agrees_with_cursor_mut() {
     let prev = cursor.peek_prev();
     assert_matches!(prev, Some((&3, _)));
 }
+
+#[test]
+fn test_id_based_insert() {
+    let mut lhs = BTreeMap::new();
+    let mut rhs = BTreeMap::new();
+
+    lhs.insert(IdBased { id: 0, name: "lhs_k".to_string() }, "lhs_v".to_string());
+    rhs.insert(IdBased { id: 0, name: "rhs_k".to_string() }, "rhs_v".to_string());
+
+    for (k, v) in rhs.into_iter() {
+        lhs.insert(k, v);
+    }
+
+    assert_eq!(lhs.pop_first().unwrap().0.name, "lhs_k".to_string());
+}
+
+#[test]
+fn test_id_based_append() {
+    let mut lhs = BTreeMap::new();
+    let mut rhs = BTreeMap::new();
+
+    lhs.insert(IdBased { id: 0, name: "lhs_k".to_string() }, "lhs_v".to_string());
+    rhs.insert(IdBased { id: 0, name: "rhs_k".to_string() }, "rhs_v".to_string());
+
+    lhs.append(&mut rhs);
+
+    assert_eq!(lhs.pop_first().unwrap().0.name, "lhs_k".to_string());
+}

library/alloctests/testing/ord_chaos.rs

@@ -2,6 +2,8 @@ use std::cell::Cell;
 use std::cmp::Ordering::{self, *};
 use std::ptr;
 
+use crate::string::String;
+
 // Minimal type with an `Ord` implementation violating transitivity.
 #[derive(Debug)]
 pub(crate) enum Cyclic3 {
@@ -79,3 +81,31 @@ impl<T: PartialEq> PartialEq for Governed<'_, T> {
 }
 
 impl<T: Eq> Eq for Governed<'_, T> {}
+
+// Comparison based only on the ID, the name is ignored.
+#[derive(Debug)]
+pub(crate) struct IdBased {
+    pub id: u32,
+    #[allow(dead_code)]
+    pub name: String,
+}
+
+impl PartialEq for IdBased {
+    fn eq(&self, other: &Self) -> bool {
+        self.id == other.id
+    }
+}
+
+impl Eq for IdBased {}
+
+impl PartialOrd for IdBased {
+    fn partial_cmp(&self, other: &Self) -> Option<Ordering> {
+        Some(self.cmp(other))
+    }
+}
+
+impl Ord for IdBased {
+    fn cmp(&self, other: &Self) -> Ordering {
+        self.id.cmp(&other.id)
+    }
+}