From d80e3f876ed589f842340b48761f70149df655b2 Mon Sep 17 00:00:00 2001 From: Gwilym Inzani Date: Sun, 23 Apr 2023 17:39:00 +0100 Subject: [PATCH 01/18] Extract agb hashmap to its own crate --- .vscode/agb.code-workspace | 5 +- agb-hashmap/Cargo.toml | 13 ++ agb/src/hash_map.rs => agb-hashmap/src/lib.rs | 212 +++++------------- agb/Cargo.toml | 1 + agb/src/lib.rs | 2 +- 5 files changed, 80 insertions(+), 153 deletions(-) create mode 100644 agb-hashmap/Cargo.toml rename agb/src/hash_map.rs => agb-hashmap/src/lib.rs (89%) diff --git a/.vscode/agb.code-workspace b/.vscode/agb.code-workspace index f7b0fd40..7c029667 100644 --- a/.vscode/agb.code-workspace +++ b/.vscode/agb.code-workspace @@ -38,6 +38,9 @@ }, { "path": "../tools" + }, + { + "path": "../agb-hashmap" } ] -} +} \ No newline at end of file diff --git a/agb-hashmap/Cargo.toml b/agb-hashmap/Cargo.toml new file mode 100644 index 00000000..3a1a7875 --- /dev/null +++ b/agb-hashmap/Cargo.toml @@ -0,0 +1,13 @@ +[package] +name = "agb_hashmap" +version = "0.14.0" +edition = "2021" +license = "MPL-2.0" +description = "A simple no_std hashmap implementation intended for use in the `agb` library" +repository = "https://github.com/agbrs/agb" + +[dependencies] +rustc-hash = { version = "1", default-features = false } + +[dev-dependencies] +rand = { version = "0.8", default-features = false, features = ["small_rng"] } \ No newline at end of file diff --git a/agb/src/hash_map.rs b/agb-hashmap/src/lib.rs similarity index 89% rename from agb/src/hash_map.rs rename to agb-hashmap/src/lib.rs index 59b71ad8..f8260efd 100644 --- a/agb/src/hash_map.rs +++ b/agb-hashmap/src/lib.rs @@ -1,6 +1,12 @@ -#![deny(missing_docs)] //! A lot of the documentation for this module was copied straight out of the rust //! standard library. The implementation however is not. +#![no_std] +#![feature(allocator_api)] +#![deny(clippy::all)] +#![deny(missing_docs)] + +extern crate alloc; + use alloc::{alloc::Global, vec::Vec}; use core::{ alloc::Allocator, @@ -993,10 +999,9 @@ mod test { use core::cell::RefCell; use super::*; - use crate::{rng::RandomNumberGenerator, Gba}; - #[test_case] - fn can_store_and_retrieve_8_elements(_gba: &mut Gba) { + #[test] + fn can_store_and_retrieve_8_elements() { let mut map = HashMap::new(); for i in 0..8 { @@ -1008,8 +1013,8 @@ mod test { } } - #[test_case] - fn can_get_the_length(_gba: &mut Gba) { + #[test] + fn can_get_the_length() { let mut map = HashMap::new(); for i in 0..8 { @@ -1019,8 +1024,8 @@ mod test { assert_eq!(map.len(), 4); } - #[test_case] - fn returns_none_if_element_does_not_exist(_gba: &mut Gba) { + #[test] + fn returns_none_if_element_does_not_exist() { let mut map = HashMap::new(); for i in 0..8 { @@ -1030,8 +1035,8 @@ mod test { assert_eq!(map.get(&12), None); } - #[test_case] - fn can_delete_entries(_gba: &mut Gba) { + #[test] + fn can_delete_entries() { let mut map = HashMap::new(); for i in 0..8 { @@ -1047,8 +1052,8 @@ mod test { assert_eq!(map.get(&7), Some(&1)); } - #[test_case] - fn can_iterate_through_all_entries(_gba: &mut Gba) { + #[test] + fn can_iterate_through_all_entries() { let mut map = HashMap::new(); for i in 0..8 { @@ -1067,8 +1072,8 @@ mod test { assert_eq!(max_found, 7); } - #[test_case] - fn can_insert_more_than_initial_capacity(_gba: &mut Gba) { + #[test] + fn can_insert_more_than_initial_capacity() { let mut map = HashMap::new(); for i in 0..65 { @@ -1115,17 +1120,32 @@ mod test { } } - #[test_case] - fn extreme_case(_gba: &mut Gba) { + trait RngNextI32 { + fn next_i32(&mut self) -> i32; + } + + impl RngNextI32 for T + where + T: rand::RngCore, + { + fn next_i32(&mut self) -> i32 { + self.next_u32() as i32 + } + } + + #[test] + fn extreme_case() { + use rand::SeedableRng; + let mut map = HashMap::new(); - let mut rng = RandomNumberGenerator::new(); + let mut rng = rand::rngs::SmallRng::seed_from_u64(20); let mut answers: [Option; 128] = [None; 128]; for _ in 0..5_000 { - let command = rng.gen().rem_euclid(2); - let key = rng.gen().rem_euclid(answers.len() as i32); - let value = rng.gen(); + let command = rng.next_i32().rem_euclid(2); + let key = rng.next_i32().rem_euclid(answers.len() as i32); + let value = rng.next_i32(); match command { 0 => { @@ -1212,8 +1232,8 @@ mod test { } } - #[test_case] - fn correctly_drops_on_remove_and_overall_drop(_gba: &mut Gba) { + #[test] + fn correctly_drops_on_remove_and_overall_drop() { let drop_registry = DropRegistry::new(); let droppable1 = drop_registry.new_droppable(); @@ -1239,8 +1259,8 @@ mod test { drop_registry.assert_dropped_once(id2); } - #[test_case] - fn correctly_drop_on_override(_gba: &mut Gba) { + #[test] + fn correctly_drop_on_override() { let drop_registry = DropRegistry::new(); let droppable1 = drop_registry.new_droppable(); @@ -1263,8 +1283,8 @@ mod test { drop_registry.assert_dropped_once(id2); } - #[test_case] - fn correctly_drops_key_on_override(_gba: &mut Gba) { + #[test] + fn correctly_drops_key_on_override() { let drop_registry = DropRegistry::new(); let droppable1 = drop_registry.new_droppable(); @@ -1285,8 +1305,8 @@ mod test { drop_registry.assert_dropped_n_times(id1, 2); } - #[test_case] - fn test_retain(_gba: &mut Gba) { + #[test] + fn test_retain() { let mut map = HashMap::new(); for i in 0..100 { @@ -1301,8 +1321,8 @@ mod test { assert_eq!(map.iter().count(), 50); // force full iteration } - #[test_case] - fn test_size_hint_iter(_gba: &mut Gba) { + #[test] + fn test_size_hint_iter() { let mut map = HashMap::new(); for i in 0..100 { @@ -1317,8 +1337,8 @@ mod test { assert_eq!(iter.size_hint(), (99, Some(99))); } - #[test_case] - fn test_size_hint_into_iter(_gba: &mut Gba) { + #[test] + fn test_size_hint_into_iter() { let mut map = HashMap::new(); for i in 0..100 { @@ -1336,13 +1356,10 @@ mod test { // Following test cases copied from the rust source // https://github.com/rust-lang/rust/blob/master/library/std/src/collections/hash/map/tests.rs mod rust_std_tests { - use crate::{ - hash_map::{Entry::*, HashMap}, - Gba, - }; + use crate::{Entry::*, HashMap}; - #[test_case] - fn test_entry(_gba: &mut Gba) { + #[test] + fn test_entry() { let xs = [(1, 10), (2, 20), (3, 30), (4, 40), (5, 50), (6, 60)]; let mut map: HashMap<_, _> = xs.iter().copied().collect(); @@ -1391,8 +1408,8 @@ mod test { assert_eq!(map.len(), 6); } - #[test_case] - fn test_occupied_entry_key(_gba: &mut Gba) { + #[test] + fn test_occupied_entry_key() { let mut a = HashMap::new(); let key = "hello there"; let value = "value goes here"; @@ -1409,8 +1426,8 @@ mod test { assert_eq!(a[key], value); } - #[test_case] - fn test_vacant_entry_key(_gba: &mut Gba) { + #[test] + fn test_vacant_entry_key() { let mut a = HashMap::new(); let key = "hello there"; let value = "value goes here"; @@ -1427,8 +1444,8 @@ mod test { assert_eq!(a[key], value); } - #[test_case] - fn test_index(_gba: &mut Gba) { + #[test] + fn test_index() { let mut map = HashMap::new(); map.insert(1, 2); @@ -1438,111 +1455,4 @@ mod test { assert_eq!(map[&2], 1); } } - - mod rust_std_tests_custom_allocator { - use crate::{ - hash_map::{Entry::*, HashMap}, - Gba, InternalAllocator, - }; - - #[test_case] - fn test_entry(_gba: &mut Gba) { - let xs = [(1, 10), (2, 20), (3, 30), (4, 40), (5, 50), (6, 60)]; - - let mut map = HashMap::new_in(InternalAllocator); - for (k, v) in xs { - map.insert(k, v); - } - - // Existing key (insert) - match map.entry(1) { - Vacant(_) => unreachable!(), - Occupied(mut view) => { - assert_eq!(view.get(), &10); - assert_eq!(view.insert(100), 10); - } - } - assert_eq!(map.get(&1).unwrap(), &100); - assert_eq!(map.len(), 6); - - // Existing key (update) - match map.entry(2) { - Vacant(_) => unreachable!(), - Occupied(mut view) => { - let v = view.get_mut(); - let new_v = (*v) * 10; - *v = new_v; - } - } - assert_eq!(map.get(&2).unwrap(), &200); - assert_eq!(map.len(), 6); - - // Existing key (take) - match map.entry(3) { - Vacant(_) => unreachable!(), - Occupied(view) => { - assert_eq!(view.remove(), 30); - } - } - assert_eq!(map.get(&3), None); - assert_eq!(map.len(), 5); - - // Inexistent key (insert) - match map.entry(10) { - Occupied(_) => unreachable!(), - Vacant(view) => { - assert_eq!(*view.insert(1000), 1000); - } - } - assert_eq!(map.get(&10).unwrap(), &1000); - assert_eq!(map.len(), 6); - } - - #[test_case] - fn test_occupied_entry_key(_gba: &mut Gba) { - let mut a = HashMap::new_in(InternalAllocator); - let key = "hello there"; - let value = "value goes here"; - assert!(a.is_empty()); - a.insert(key, value); - assert_eq!(a.len(), 1); - assert_eq!(a[key], value); - - match a.entry(key) { - Vacant(_) => panic!(), - Occupied(e) => assert_eq!(key, *e.key()), - } - assert_eq!(a.len(), 1); - assert_eq!(a[key], value); - } - - #[test_case] - fn test_vacant_entry_key(_gba: &mut Gba) { - let mut a = HashMap::new_in(InternalAllocator); - let key = "hello there"; - let value = "value goes here"; - - assert!(a.is_empty()); - match a.entry(key) { - Occupied(_) => panic!(), - Vacant(e) => { - assert_eq!(key, *e.key()); - e.insert(value); - } - } - assert_eq!(a.len(), 1); - assert_eq!(a[key], value); - } - - #[test_case] - fn test_index(_gba: &mut Gba) { - let mut map = HashMap::new_in(InternalAllocator); - - map.insert(1, 2); - map.insert(2, 1); - map.insert(3, 4); - - assert_eq!(map[&2], 1); - } - } } diff --git a/agb/Cargo.toml b/agb/Cargo.toml index 583033a5..1f572e9a 100644 --- a/agb/Cargo.toml +++ b/agb/Cargo.toml @@ -27,6 +27,7 @@ agb_image_converter = { version = "0.14.0", path = "../agb-image-converter" } agb_sound_converter = { version = "0.14.0", path = "../agb-sound-converter" } agb_macros = { version = "0.14.0", path = "../agb-macros" } agb_fixnum = { version = "0.14.0", path = "../agb-fixnum" } +agb_hashmap = { version = "0.14.0", path = "../agb-hashmap" } bare-metal = "1" modular-bitfield = "0.11" rustc-hash = { version = "1", default-features = false } diff --git a/agb/src/lib.rs b/agb/src/lib.rs index 72926375..14aed6e9 100644 --- a/agb/src/lib.rs +++ b/agb/src/lib.rs @@ -152,7 +152,7 @@ pub mod mgba; #[doc(inline)] pub use agb_fixnum as fixnum; /// Contains an implementation of a hashmap which suits the gameboy advance's hardware. -pub mod hash_map; +pub use agb_hashmap as hash_map; /// Simple random number generator pub mod rng; pub mod save; From 4ade408d3056847b821597ba0b2cd5c09f682d97 Mon Sep 17 00:00:00 2001 From: Gwilym Inzani Date: Sun, 23 Apr 2023 17:43:30 +0100 Subject: [PATCH 02/18] Avoid the really slow test under miri --- agb-hashmap/src/lib.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/agb-hashmap/src/lib.rs b/agb-hashmap/src/lib.rs index f8260efd..350f2206 100644 --- a/agb-hashmap/src/lib.rs +++ b/agb-hashmap/src/lib.rs @@ -1133,6 +1133,7 @@ mod test { } } + #[cfg(not(miri))] // takes way too long to run under miri #[test] fn extreme_case() { use rand::SeedableRng; From db0eab7589d8fe847fd8935318cdc19698d56772 Mon Sep 17 00:00:00 2001 From: Gwilym Inzani Date: Sun, 23 Apr 2023 17:46:00 +0100 Subject: [PATCH 03/18] Add miri test step --- .github/workflows/build-and-test.yml | 4 ++++ justfile | 5 ++++- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/.github/workflows/build-and-test.yml b/.github/workflows/build-and-test.yml index f92a6c08..12e125b0 100644 --- a/.github/workflows/build-and-test.yml +++ b/.github/workflows/build-and-test.yml @@ -18,6 +18,10 @@ jobs: steps: - name: Install build tools run: sudo apt-get update && sudo apt-get install build-essential binutils-arm-none-eabi libelf-dev zip -y + - name: Install Miri + run: | + rustup component add miri + cargo miri setup - uses: actions/checkout@v3 - name: Cache uses: actions/cache@v3 diff --git a/justfile b/justfile index 4a0b9e7d..9c6f9c7d 100644 --- a/justfile +++ b/justfile @@ -59,7 +59,7 @@ check-linker-script-consistency: find -type f -name gba.ld -print0 | xargs -0 -n1 cmp -- agb/gba.ld find -type f -name gba_mb.ld -print0 | xargs -0 -n1 cmp -- agb/gba_mb.ld -ci: check-linker-script-consistency build-debug clippy fmt-check test build-release test-release doctest-agb build-roms build-book check-docs +ci: check-linker-script-consistency build-debug clippy fmt-check test miri build-release test-release doctest-agb build-roms build-book check-docs build-roms: just _build-rom "examples/the-purple-night" "PURPLENIGHT" @@ -85,6 +85,9 @@ publish *args: (_run-tool "publish" args) release +args: (_run-tool "release" args) +miri: + (cd agb-hashmap && cargo miri test) + _run-tool +tool: (cd tools && cargo build) "$CARGO_TARGET_DIR/debug/tools" {{tool}} From f667804cf85b075c02490f531837ecfb3524fd3e Mon Sep 17 00:00:00 2001 From: Gwilym Inzani Date: Sun, 23 Apr 2023 17:47:15 +0100 Subject: [PATCH 04/18] Add section in readme about miri --- README.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/README.md b/README.md index ed82fb7b..78f99a73 100644 --- a/README.md +++ b/README.md @@ -66,6 +66,8 @@ to just write games for the Game Boy Advance using this library: * Install with `cargo install just` * [mdbook](https://rust-lang.github.io/mdBook/index.html) * Install with `cargo install mdbook` +* [miri](https://github.com/rust-lang/miri) + * Some of the unsafe code is tested using miri, install with `rustup component add miri` With all of this installed, you should be able to run a full build of agb using by running ```sh From 9c27e9ca4d1263854c7da6129b9263dbe17c1413 Mon Sep 17 00:00:00 2001 From: Gwilym Inzani Date: Sun, 23 Apr 2023 17:48:55 +0100 Subject: [PATCH 05/18] Add section about agb-hashmap in the readme --- README.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/README.md b/README.md index 78f99a73..4cf9e327 100644 --- a/README.md +++ b/README.md @@ -87,6 +87,8 @@ for performant decimals. `agb-sound-converter` - a crate which converts wav files into a format supported by the game boy advance +`agb-hashmap` - an no_std hashmap implementation tuned for use on the game boy advance + `agb` - the main library code `agb/examples` - basic examples often targeting 1 feature, you can run these using `just run-example ` From 69687aa51954ac7fb9b412789680837a63b14ddc Mon Sep 17 00:00:00 2001 From: Gwilym Inzani Date: Sun, 23 Apr 2023 17:58:45 +0100 Subject: [PATCH 06/18] Import all the denys from the main crate --- agb-hashmap/src/lib.rs | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/agb-hashmap/src/lib.rs b/agb-hashmap/src/lib.rs index 350f2206..5283605e 100644 --- a/agb-hashmap/src/lib.rs +++ b/agb-hashmap/src/lib.rs @@ -3,7 +3,17 @@ #![no_std] #![feature(allocator_api)] #![deny(clippy::all)] +#![deny(clippy::must_use_candidate)] #![deny(missing_docs)] +#![deny(clippy::trivially_copy_pass_by_ref)] +#![deny(clippy::semicolon_if_nothing_returned)] +#![deny(clippy::map_unwrap_or)] +#![deny(clippy::needless_pass_by_value)] +#![deny(clippy::redundant_closure_for_method_calls)] +#![deny(clippy::cloned_instead_of_copied)] +#![deny(rustdoc::broken_intra_doc_links)] +#![deny(rustdoc::private_intra_doc_links)] +#![deny(rustdoc::invalid_html_tags)] extern crate alloc; @@ -1091,6 +1101,7 @@ mod test { } impl NoisyDrop { + #[cfg(not(miri))] fn new(i: i32) -> Self { Self { i, dropped: false } } From 0841759c8d50493e196980fa4fcdda4e86f97eb3 Mon Sep 17 00:00:00 2001 From: Gwilym Inzani Date: Sun, 23 Apr 2023 17:58:59 +0100 Subject: [PATCH 07/18] Fix publish test --- tools/src/publish.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tools/src/publish.rs b/tools/src/publish.rs index be68209f..48d2d616 100644 --- a/tools/src/publish.rs +++ b/tools/src/publish.rs @@ -170,7 +170,8 @@ mod test { "agb-image-converter", "agb-sound-converter", "agb-macros", - "agb-fixnum" + "agb-fixnum", + "agb-hashmap", ] ); Ok(()) From 7029b66c625fb9a17df9148ee690d0ae26a8d407 Mon Sep 17 00:00:00 2001 From: Gwilym Inzani Date: Sun, 23 Apr 2023 18:00:31 +0100 Subject: [PATCH 08/18] Update lockfiles --- book/games/pong/Cargo.lock | 8 ++++++++ examples/combo/Cargo.lock | 8 ++++++++ examples/hyperspace-roll/Cargo.lock | 8 ++++++++ examples/the-hat-chooses-the-wizard/Cargo.lock | 8 ++++++++ examples/the-purple-night/Cargo.lock | 8 ++++++++ 5 files changed, 40 insertions(+) diff --git a/book/games/pong/Cargo.lock b/book/games/pong/Cargo.lock index 1597215a..cff08e4b 100644 --- a/book/games/pong/Cargo.lock +++ b/book/games/pong/Cargo.lock @@ -19,6 +19,7 @@ name = "agb" version = "0.14.0" dependencies = [ "agb_fixnum", + "agb_hashmap", "agb_image_converter", "agb_macros", "agb_sound_converter", @@ -35,6 +36,13 @@ dependencies = [ "agb_macros", ] +[[package]] +name = "agb_hashmap" +version = "0.14.0" +dependencies = [ + "rustc-hash", +] + [[package]] name = "agb_image_converter" version = "0.14.0" diff --git a/examples/combo/Cargo.lock b/examples/combo/Cargo.lock index ac40b044..84f65f39 100644 --- a/examples/combo/Cargo.lock +++ b/examples/combo/Cargo.lock @@ -19,6 +19,7 @@ name = "agb" version = "0.14.0" dependencies = [ "agb_fixnum", + "agb_hashmap", "agb_image_converter", "agb_macros", "agb_sound_converter", @@ -35,6 +36,13 @@ dependencies = [ "agb_macros", ] +[[package]] +name = "agb_hashmap" +version = "0.14.0" +dependencies = [ + "rustc-hash", +] + [[package]] name = "agb_image_converter" version = "0.14.0" diff --git a/examples/hyperspace-roll/Cargo.lock b/examples/hyperspace-roll/Cargo.lock index 753b5b84..651a3d7a 100644 --- a/examples/hyperspace-roll/Cargo.lock +++ b/examples/hyperspace-roll/Cargo.lock @@ -19,6 +19,7 @@ name = "agb" version = "0.14.0" dependencies = [ "agb_fixnum", + "agb_hashmap", "agb_image_converter", "agb_macros", "agb_sound_converter", @@ -35,6 +36,13 @@ dependencies = [ "agb_macros", ] +[[package]] +name = "agb_hashmap" +version = "0.14.0" +dependencies = [ + "rustc-hash", +] + [[package]] name = "agb_image_converter" version = "0.14.0" diff --git a/examples/the-hat-chooses-the-wizard/Cargo.lock b/examples/the-hat-chooses-the-wizard/Cargo.lock index 4de4793d..cecf9caf 100644 --- a/examples/the-hat-chooses-the-wizard/Cargo.lock +++ b/examples/the-hat-chooses-the-wizard/Cargo.lock @@ -19,6 +19,7 @@ name = "agb" version = "0.14.0" dependencies = [ "agb_fixnum", + "agb_hashmap", "agb_image_converter", "agb_macros", "agb_sound_converter", @@ -35,6 +36,13 @@ dependencies = [ "agb_macros", ] +[[package]] +name = "agb_hashmap" +version = "0.14.0" +dependencies = [ + "rustc-hash", +] + [[package]] name = "agb_image_converter" version = "0.14.0" diff --git a/examples/the-purple-night/Cargo.lock b/examples/the-purple-night/Cargo.lock index 8b6264da..bd9ad872 100644 --- a/examples/the-purple-night/Cargo.lock +++ b/examples/the-purple-night/Cargo.lock @@ -19,6 +19,7 @@ name = "agb" version = "0.14.0" dependencies = [ "agb_fixnum", + "agb_hashmap", "agb_image_converter", "agb_macros", "agb_sound_converter", @@ -35,6 +36,13 @@ dependencies = [ "agb_macros", ] +[[package]] +name = "agb_hashmap" +version = "0.14.0" +dependencies = [ + "rustc-hash", +] + [[package]] name = "agb_image_converter" version = "0.14.0" From 6d7801788ee4f35ac8a77dc8649161da07a20e96 Mon Sep 17 00:00:00 2001 From: Gwilym Inzani Date: Sun, 23 Apr 2023 18:02:00 +0100 Subject: [PATCH 09/18] Add changelog entry --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index eee11ee8..e8e37174 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed - Importing background tiles has been improved. You no longer need to use `include_gfx!` with the toml file. Instead, use `include_background_gfx`. See the documentation for usage. +- The hashmap implementation is now it its own crate, `agb-hashmap`. There is no change in API, but you can now use this for interop between non-agb code and agb code ## [0.14.0] - 2023/04/11 From 6d0bd187a2e357c24e205f6589ed763361031293 Mon Sep 17 00:00:00 2001 From: Gwilym Inzani Date: Sun, 23 Apr 2023 18:04:42 +0100 Subject: [PATCH 10/18] Explicitly state nightly --- .github/workflows/build-and-test.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/build-and-test.yml b/.github/workflows/build-and-test.yml index 12e125b0..5d1741dd 100644 --- a/.github/workflows/build-and-test.yml +++ b/.github/workflows/build-and-test.yml @@ -20,7 +20,7 @@ jobs: run: sudo apt-get update && sudo apt-get install build-essential binutils-arm-none-eabi libelf-dev zip -y - name: Install Miri run: | - rustup component add miri + rustup component add --toolchain nightly miri cargo miri setup - uses: actions/checkout@v3 - name: Cache From a053c8f0c3786b6ed972dbc0a752cd7cdb32711f Mon Sep 17 00:00:00 2001 From: Gwilym Inzani Date: Sun, 23 Apr 2023 18:06:52 +0100 Subject: [PATCH 11/18] Seems nightly isn't installed by default --- .github/workflows/build-and-test.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/build-and-test.yml b/.github/workflows/build-and-test.yml index 5d1741dd..4b334d24 100644 --- a/.github/workflows/build-and-test.yml +++ b/.github/workflows/build-and-test.yml @@ -20,8 +20,8 @@ jobs: run: sudo apt-get update && sudo apt-get install build-essential binutils-arm-none-eabi libelf-dev zip -y - name: Install Miri run: | - rustup component add --toolchain nightly miri - cargo miri setup + rustup toolchain install nightly --component miri + cargo miri setup - uses: actions/checkout@v3 - name: Cache uses: actions/cache@v3 From 88d3d027b73ccb74de93ca87de9f3688409b5718 Mon Sep 17 00:00:00 2001 From: Gwilym Inzani Date: Sun, 23 Apr 2023 18:08:59 +0100 Subject: [PATCH 12/18] Maybe we need to set nightly as default? --- .github/workflows/build-and-test.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/build-and-test.yml b/.github/workflows/build-and-test.yml index 4b334d24..2f873f9a 100644 --- a/.github/workflows/build-and-test.yml +++ b/.github/workflows/build-and-test.yml @@ -21,6 +21,7 @@ jobs: - name: Install Miri run: | rustup toolchain install nightly --component miri + rustup override set nightly cargo miri setup - uses: actions/checkout@v3 - name: Cache From d594e9e078dc2cd2e0abda15b72c830e04de19ac Mon Sep 17 00:00:00 2001 From: Gwilym Inzani Date: Sun, 23 Apr 2023 18:37:22 +0100 Subject: [PATCH 13/18] Also build docs for agb-hashmap --- justfile | 1 + 1 file changed, 1 insertion(+) diff --git a/justfile b/justfile index 9c6f9c7d..cde49acb 100644 --- a/justfile +++ b/justfile @@ -27,6 +27,7 @@ doctest-agb: check-docs: (cd agb && cargo doc --target=thumbv6m-none-eabi --no-deps) just _build_docs agb-fixnum + just _build_docs agb-hashmap _build_docs crate: (cd "{{crate}}" && cargo doc --no-deps) From 150b1a3078ef3307582afd06c2525fd971ad336c Mon Sep 17 00:00:00 2001 From: Gwilym Inzani Date: Sun, 23 Apr 2023 18:37:30 +0100 Subject: [PATCH 14/18] Add documentation example and add Borrow stuff for easier contains_key --- agb-hashmap/src/lib.rs | 52 ++++++++++++++++++++++++++++++++++++++---- 1 file changed, 47 insertions(+), 5 deletions(-) diff --git a/agb-hashmap/src/lib.rs b/agb-hashmap/src/lib.rs index 5283605e..9602cb28 100644 --- a/agb-hashmap/src/lib.rs +++ b/agb-hashmap/src/lib.rs @@ -20,6 +20,7 @@ extern crate alloc; use alloc::{alloc::Global, vec::Vec}; use core::{ alloc::Allocator, + borrow::Borrow, hash::{BuildHasher, BuildHasherDefault, Hash, Hasher}, iter::FromIterator, mem::{self, MaybeUninit}, @@ -99,6 +100,38 @@ type HashType = u32; /// /// [`Eq`]: https://doc.rust-lang.org/core/cmp/trait.Eq.html /// [`Hash`]: https://doc.rust-lang.org/core/hash/trait.Hash.html +/// +/// # Example +/// ``` +/// use agb_hashmap::HashMap; +/// +/// // Type inference lets you omit the type signature (which would be HashMap in this example) +/// let mut game_reviews = HashMap::new(); +/// +/// // Review some games +/// game_reviews.insert( +/// "Pokemon Emerald".to_string(), +/// "Best post-game battle experience of any generation.".to_string(), +/// ); +/// game_reviews.insert( +/// "Golden Sun".to_string(), +/// "Some of the best music on the console".to_string(), +/// ); +/// game_reviews.insert( +/// "Super Dodge Ball Advance".to_string(), +/// "Really great launch title".to_string(), +/// ); +/// +/// // Check for a specific entry +/// if !game_reviews.contains_key("Legend of Zelda: The Minish Cap") { +/// println!("We've got {} reviews, but The Minish Cap ain't one", game_reviews.len()); +/// } +/// +/// // Iterate over everything +/// for (game, review) in &game_reviews { +/// println!("{game}: \"{review}\""); +/// } +/// ``` pub struct HashMap { nodes: NodeStorage, @@ -299,7 +332,11 @@ where } /// Returns `true` if the map contains a value for the specified key. - pub fn contains_key(&self, k: &K) -> bool { + pub fn contains_key(&self, k: &Q) -> bool + where + K: Borrow, + Q: Hash + Eq + ?Sized, + { let hash = self.hash(k); self.nodes.location(k, hash).is_some() } @@ -346,7 +383,11 @@ impl HashMap where K: Hash, { - fn hash(&self, key: &K) -> HashType { + fn hash(&self, key: &Q) -> HashType + where + K: Borrow, + Q: Hash + ?Sized, + { let mut hasher = self.hasher.build_hasher(); key.hash(&mut hasher); hasher.finish() as HashType @@ -828,9 +869,10 @@ impl NodeStorage { } } - fn location(&self, key: &K, hash: HashType) -> Option + fn location(&self, key: &Q, hash: HashType) -> Option where - K: Eq, + K: Borrow, + Q: Eq + ?Sized, { for distance_to_initial_bucket in 0..(self.max_distance_to_initial_bucket + 1) { let location = fast_mod( @@ -840,7 +882,7 @@ impl NodeStorage { let node = &self.nodes[location]; if let Some(node_key_ref) = node.key_ref() { - if node_key_ref == key { + if node_key_ref.borrow() == key { return Some(location); } } else { From 39edc4ab360bc74028e60369ac10d36bdb5d818e Mon Sep 17 00:00:00 2001 From: Gwilym Inzani Date: Sun, 23 Apr 2023 18:40:13 +0100 Subject: [PATCH 15/18] Also do Borrow shenanigans for get and get_key_value --- agb-hashmap/src/lib.rs | 22 ++++++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/agb-hashmap/src/lib.rs b/agb-hashmap/src/lib.rs index 9602cb28..6a77a571 100644 --- a/agb-hashmap/src/lib.rs +++ b/agb-hashmap/src/lib.rs @@ -342,7 +342,11 @@ where } /// Returns the key-value pair corresponding to the supplied key - pub fn get_key_value(&self, key: &K) -> Option<(&K, &V)> { + pub fn get_key_value(&self, key: &Q) -> Option<(&K, &V)> + where + K: Borrow, + Q: Hash + Eq + ?Sized, + { let hash = self.hash(key); self.nodes @@ -352,7 +356,21 @@ where /// Returns a reference to the value corresponding to the key. Returns [`None`] if there is /// no element in the map with the given key. - pub fn get(&self, key: &K) -> Option<&V> { + /// + /// # Example + /// ``` + /// use agb_hashmap::HashMap; + /// + /// let mut map = HashMap::new(); + /// map.insert("a".to_string(), "A"); + /// assert_eq!(map.get("a"), Some(&"A")); + /// assert_eq!(map.get("b"), None); + /// ``` + pub fn get(&self, key: &Q) -> Option<&V> + where + K: Borrow, + Q: Hash + Eq + ?Sized, + { self.get_key_value(key).map(|(_, v)| v) } From 11b98eab2970c4a0e3755d9e72fabaa0a2581640 Mon Sep 17 00:00:00 2001 From: Gwilym Inzani Date: Sun, 23 Apr 2023 18:44:03 +0100 Subject: [PATCH 16/18] Give index and get_mut the Borrow treatment --- agb-hashmap/src/lib.rs | 38 +++++++++++++++++++++++--------------- 1 file changed, 23 insertions(+), 15 deletions(-) diff --git a/agb-hashmap/src/lib.rs b/agb-hashmap/src/lib.rs index 6a77a571..c2b6ddca 100644 --- a/agb-hashmap/src/lib.rs +++ b/agb-hashmap/src/lib.rs @@ -376,7 +376,25 @@ where /// Returns a mutable reference to the value corresponding to the key. Return [`None`] if /// there is no element in the map with the given key. - pub fn get_mut(&mut self, key: &K) -> Option<&mut V> { + /// + /// # Example + /// ``` + /// use agb_hashmap::HashMap; + /// + /// let mut map = HashMap::new(); + /// map.insert("a".to_string(), "A"); + /// + /// if let Some(x) = map.get_mut("a") { + /// *x = "b"; + /// } + /// + /// assert_eq!(map["a"], "b"); + /// ``` + pub fn get_mut(&mut self, key: &Q) -> Option<&mut V> + where + K: Borrow, + Q: Hash + Eq + ?Sized, + { let hash = self.hash(key); if let Some(location) = self.nodes.location(key, hash) { @@ -732,28 +750,18 @@ where } } -impl Index<&K> for HashMap +impl Index<&Q> for HashMap where - K: Eq + Hash, + K: Eq + Hash + Borrow, + Q: Eq + Hash + ?Sized, { type Output = V; - fn index(&self, key: &K) -> &V { + fn index(&self, key: &Q) -> &V { self.get(key).expect("no entry found for key") } } -impl Index for HashMap -where - K: Eq + Hash, -{ - type Output = V; - - fn index(&self, key: K) -> &V { - self.get(&key).expect("no entry found for key") - } -} - const fn number_before_resize(capacity: usize) -> usize { capacity * 85 / 100 } From 4cace7b01dc537d0283ce8a7e1f7e5c08c81fc36 Mon Sep 17 00:00:00 2001 From: Gwilym Inzani Date: Sun, 23 Apr 2023 18:45:39 +0100 Subject: [PATCH 17/18] Also for remove --- agb-hashmap/src/lib.rs | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/agb-hashmap/src/lib.rs b/agb-hashmap/src/lib.rs index c2b6ddca..147af353 100644 --- a/agb-hashmap/src/lib.rs +++ b/agb-hashmap/src/lib.rs @@ -406,7 +406,21 @@ where /// Removes the given key from the map. Returns the current value if it existed, or [`None`] /// if it did not. - pub fn remove(&mut self, key: &K) -> Option { + /// + /// # Example + /// ``` + /// use agb_hashmap::HashMap; + /// + /// let mut map = HashMap::new(); + /// map.insert(1, "a"); + /// assert_eq!(map.remove(&1), Some("a")); + /// assert_eq!(map.remove(&1), None); + /// ``` + pub fn remove(&mut self, key: &Q) -> Option + where + K: Borrow, + Q: Hash + Eq + ?Sized, + { let hash = self.hash(key); self.nodes From cd369f212e3b1fa6de5a6c4a4e621b2f8b37549b Mon Sep 17 00:00:00 2001 From: Gwilym Inzani Date: Sun, 23 Apr 2023 18:48:16 +0100 Subject: [PATCH 18/18] Ensure tests run on hashmap too --- justfile | 1 + 1 file changed, 1 insertion(+) diff --git a/justfile b/justfile index cde49acb..0c3326fd 100644 --- a/justfile +++ b/justfile @@ -14,6 +14,7 @@ clippy: test: just _test-debug agb just _test-debug agb-fixnum + just _test-debug agb-hashmap just _test-debug-arm agb just _test-debug tools