Merge pull request #402 from rust-embedded/fix-indexmap

Fix IndexMap entry API, fix CI.

Author: Dirbaio

.github/workflows/build.yml

@@ -49,6 +49,42 @@ jobs:
       - name: Run miri
         run: MIRIFLAGS=-Zmiri-ignore-leaks cargo miri test
 
+  # Run cargo test
+  test:
+    name: test
+    runs-on: ubuntu-latest
+    steps:
+      - name: Checkout
+        uses: actions/checkout@v4
+
+      - name: Cache cargo dependencies
+        uses: actions/cache@v3
+        with:
+          path: |
+            - ~/.cargo/bin/
+            - ~/.cargo/registry/index/
+            - ~/.cargo/registry/cache/
+            - ~/.cargo/git/db/
+          key: ${{ runner.OS }}-cargo-${{ hashFiles('**/Cargo.lock') }}
+          restore-keys: |
+            ${{ runner.OS }}-cargo-
+
+      - name: Cache build output dependencies
+        uses: actions/cache@v3
+        with:
+          path: target
+          key: ${{ runner.OS }}-build-${{ hashFiles('**/Cargo.lock') }}
+          restore-keys: |
+            ${{ runner.OS }}-build-
+
+      - name: Install Rust
+        uses: dtolnay/rust-toolchain@master
+        with:
+          toolchain: stable
+
+      - name: Run cargo test
+        run: cargo test
+
   # Run cargo fmt --check
   style:
     name: style
@@ -169,11 +205,6 @@ jobs:
         target:
           - x86_64-unknown-linux-gnu
           - i686-unknown-linux-musl
-        toolchain:
-          - stable
-          - nightly
-        features:
-          - serde
         buildtype:
           - ""
           - "--release"
@@ -203,14 +234,14 @@ jobs:
             ${{ runner.OS }}-build-${{ hashFiles('**/Cargo.lock') }}
             ${{ runner.OS }}-build-
 
-      - name: Install Rust ${{ matrix.toolchain }} with target (${{ matrix.target }})
+      - name: Install Rust with target (${{ matrix.target }})
         uses: dtolnay/rust-toolchain@master
         with:
-          toolchain: ${{ matrix.toolchain }}
+          toolchain: stable
           targets: ${{ matrix.target }}
 
       - name: cargo test
-        run: cargo test --test cpass --target=${{ matrix.target }} --features=${{ matrix.features }} ${{ matrix.buildtype }}
+        run: cargo test --test cpass --target=${{ matrix.target }} --features=serde ${{ matrix.buildtype }}
 
   # Run test suite for UI
   testtsan:
@@ -220,8 +251,6 @@ jobs:
       matrix:
         target:
           - x86_64-unknown-linux-gnu
-        toolchain:
-          - nightly
         buildtype:
           - ""
           - "--release"
@@ -249,10 +278,10 @@ jobs:
           restore-keys: |
             ${{ runner.OS }}-build-
 
-      - name: Install Rust ${{ matrix.toolchain }} with target (${{ matrix.target }})
+      - name: Install Rust nightly with target (${{ matrix.target }})
         uses: dtolnay/rust-toolchain@master
         with:
-          toolchain: ${{ matrix.toolchain }}
+          toolchain: nightly
           target: ${{ matrix.target }}
           components: rust-src
 
@@ -302,23 +331,3 @@ jobs:
 
       - name: Run cargo
         run: cargo run
-
-  # Refs: https://github.com/rust-lang/crater/blob/9ab6f9697c901c4a44025cf0a39b73ad5b37d198/.github/workflows/bors.yml#L125-L149
-  #
-  # ALL THE PREVIOUS JOBS NEEDS TO BE ADDED TO THE `needs` SECTION OF THIS JOB!
-
-  ci-success:
-    name: ci
-    if: github.event_name == 'push' && success()
-    needs:
-      - style
-      - check
-      - doc
-      - testcpass
-      - testtsan
-      - testcfail
-      - testmiri
-    runs-on: ubuntu-latest
-    steps:
-      - name: Mark the job as a success
-        run: exit 0

CHANGELOG.md

@@ -43,6 +43,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 ### Fixed
 
 - Fixed a `dropping_references` warning in `LinearMap`.
+- Fixed IndexMap entry API returning wrong slot after an insert on vacant entry. (#360)
 
 ### Removed
 

src/de.rs

@@ -1,5 +1,6 @@
 use crate::{
-    binary_heap::Kind as BinaryHeapKind, BinaryHeap, Deque, IndexMap, IndexSet, LinearMap, String, Vec,
+    binary_heap::Kind as BinaryHeapKind, BinaryHeap, Deque, IndexMap, IndexSet, LinearMap, String,
+    Vec,
 };
 use core::{
     fmt,

src/indexmap.rs

@@ -209,12 +209,9 @@ where
                     // robin hood: steal the spot if it's better for us
                     let index = self.entries.len();
                     unsafe { self.entries.push_unchecked(Bucket { hash, key, value }) };
+                    Self::insert_phase_2(&mut self.indices, probe, Pos::new(index, hash));
                     return Insert::Success(Inserted {
-                        index: Self::insert_phase_2(
-                            &mut self.indices,
-                            probe,
-                            Pos::new(index, hash),
-                        ),
+                        index,
                         old_value: None,
                     });
                 } else if entry_hash == hash && unsafe { self.entries.get_unchecked(i).key == key }
@@ -1372,7 +1369,7 @@ mod tests {
                 panic!("Entry found when empty");
             }
             Entry::Vacant(v) => {
-                v.insert(value).unwrap();
+                assert_eq!(value, *v.insert(value).unwrap());
             }
         };
         assert_eq!(value, *src.get(&key).unwrap())
@@ -1472,7 +1469,7 @@ mod tests {
                     panic!("Entry found before insert");
                 }
                 Entry::Vacant(v) => {
-                    v.insert(i).unwrap();
+                    assert_eq!(i, *v.insert(i).unwrap());
                 }
             }
         }

src/ser.rs

@@ -1,7 +1,8 @@
 use core::hash::{BuildHasher, Hash};
 
 use crate::{
-    binary_heap::Kind as BinaryHeapKind, BinaryHeap, Deque, IndexMap, IndexSet, LinearMap, String, Vec,
+    binary_heap::Kind as BinaryHeapKind, BinaryHeap, Deque, IndexMap, IndexSet, LinearMap, String,
+    Vec,
 };
 use serde::ser::{Serialize, SerializeMap, SerializeSeq, Serializer};
 

src/string.rs

@@ -350,7 +350,7 @@ impl<const N: usize> String<N> {
     /// ```
     /// use heapless::String;
     ///
-    /// let mut s: String<8> = String::from("foo");
+    /// let mut s: String<8> = String::try_from("foo").unwrap();
     ///
     /// assert_eq!(s.remove(0), 'f');
     /// assert_eq!(s.remove(1), 'o');
@@ -365,12 +365,9 @@ impl<const N: usize> String<N> {
 
         let next = index + ch.len_utf8();
         let len = self.len();
+        let ptr = self.vec.as_mut_ptr();
         unsafe {
-            core::ptr::copy(
-                self.vec.as_ptr().add(next),
-                self.vec.as_mut_ptr().add(index),
-                len - next,
-            );
+            core::ptr::copy(ptr.add(next), ptr.add(index), len - next);
             self.vec.set_len(len - (next - index));
         }
         ch
@@ -842,14 +839,14 @@ mod tests {
 
     #[test]
     fn remove() {
-        let mut s: String<8> = String::from("foo");
+        let mut s: String<8> = String::try_from("foo").unwrap();
         assert_eq!(s.remove(0), 'f');
         assert_eq!(s.as_str(), "oo");
     }
 
     #[test]
     fn remove_uenc() {
-        let mut s: String<8> = String::from("ĝėēƶ");
+        let mut s: String<8> = String::try_from("ĝėēƶ").unwrap();
         assert_eq!(s.remove(2), 'ė');
         assert_eq!(s.remove(2), 'ē');
         assert_eq!(s.remove(2), 'ƶ');
@@ -858,7 +855,7 @@ mod tests {
 
     #[test]
     fn remove_uenc_combo_characters() {
-        let mut s: String<8> = String::from("héy");
+        let mut s: String<8> = String::try_from("héy").unwrap();
         assert_eq!(s.remove(2), '\u{0301}');
         assert_eq!(s.as_str(), "hey");
     }