From a88e580186a6a67f78da7ec71908ae108bb0908d Mon Sep 17 00:00:00 2001 From: Peter Jaszkowiak Date: Thu, 13 Oct 2022 23:38:20 -0600 Subject: [PATCH 1/4] impl `Map::retain` and `Set::retain` test examples and benches in CI --- .github/workflows/ci.yml | 8 +++ examples/basic.rs | 7 +++ examples/map_feature.rs | 47 +++++++++++++++++ fixed-map-derive/src/any_variants.rs | 20 ++++++++ fixed-map-derive/src/unit_variants.rs | 17 +++++++ src/map.rs | 70 +++++++++++++++++++++++++ src/set.rs | 73 +++++++++++++++++++++++++++ src/storage.rs | 5 ++ src/storage/boolean.rs | 17 +++++++ src/storage/map.rs | 8 +++ src/storage/option.rs | 13 +++++ src/storage/singleton.rs | 12 +++++ 12 files changed, 297 insertions(+) create mode 100644 examples/map_feature.rs diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index a7bef01..a552186 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -14,7 +14,15 @@ jobs: profile: minimal components: clippy, rustfmt + # Run library and proc macro unit tests and doc tests + # also checks that those compile ok - run: cargo test --all-features - run: cargo test --no-default-features + # Run example and bench tests + # also checks that those compile ok + - run: cargo test --all-features --examples --benches + - run: cargo test --no-default-features --examples --benches + # Run clippy on everything, denying warnings - run: cargo clippy --workspace --all-features --tests --examples --benches -- --deny warnings + # Check that formatting is correct - run: cargo fmt --check diff --git a/examples/basic.rs b/examples/basic.rs index cde4bf3..08a0792 100644 --- a/examples/basic.rs +++ b/examples/basic.rs @@ -12,3 +12,10 @@ fn main() { assert_eq!(map.get(Key::First), Some(&42)); assert_eq!(map.get(Key::Second), None); } + +// Execute this during testing as well. +#[cfg(test)] +#[test] +fn test_main() { + main(); +} diff --git a/examples/map_feature.rs b/examples/map_feature.rs new file mode 100644 index 0000000..fd1e5d6 --- /dev/null +++ b/examples/map_feature.rs @@ -0,0 +1,47 @@ +// This file is very useful for debugging the derive macro. +// Doctests don't work well for that purpose. + +fn main() { + #[cfg(feature = "map")] + { + use fixed_map::{Key, Map}; + + #[derive(Clone, Copy, Key)] + enum Part { + One, + Two, + } + + #[derive(Clone, Copy, Key)] + enum Key { + Simple, + Composite(Part), + String(&'static str), + Number(u32), + Singleton(()), + } + let mut map = Map::new(); + + map.insert(Key::Simple, 1); + map.insert(Key::Composite(Part::One), 2); + map.insert(Key::String("foo"), 3); + map.insert(Key::Number(1), 4); + map.insert(Key::Singleton(()), 5); + + assert_eq!(map.get(Key::Simple), Some(&1)); + assert_eq!(map.get(Key::Composite(Part::One)), Some(&2)); + assert_eq!(map.get(Key::Composite(Part::Two)), None); + assert_eq!(map.get(Key::String("foo")), Some(&3)); + assert_eq!(map.get(Key::String("bar")), None); + assert_eq!(map.get(Key::Number(1)), Some(&4)); + assert_eq!(map.get(Key::Number(2)), None); + assert_eq!(map.get(Key::Singleton(())), Some(&5)); + } +} + +// Execute this during testing as well. +#[cfg(test)] +#[test] +fn test_main() { + main(); +} diff --git a/fixed-map-derive/src/any_variants.rs b/fixed-map-derive/src/any_variants.rs index 9f93900..cdb0c01 100644 --- a/fixed-map-derive/src/any_variants.rs +++ b/fixed-map-derive/src/any_variants.rs @@ -36,6 +36,7 @@ pub(crate) fn implement(cx: &Ctxt, en: &DataEnum) -> Result { let mut get_mut = Vec::new(); let mut insert = Vec::new(); let mut remove = Vec::new(); + let mut retain = Vec::new(); let mut clear = Vec::new(); let mut copy_bounds = Vec::new(); let mut field_specs = Vec::new(); @@ -64,6 +65,14 @@ pub(crate) fn implement(cx: &Ctxt, en: &DataEnum) -> Result { get_mut.push(quote!(#option::as_mut(&mut self.#name))); insert.push(quote!(#mem::replace(&mut self.#name, #option::Some(value)))); remove.push(quote!(#mem::replace(&mut self.#name, #option::None))); + retain.push(quote! { + if let Some(val) = #option::as_mut(&mut self.#name) { + if !func(#ident::#var, val) { + self.#name = None; + } + } + }); + FieldKind::Simple } Fields::Unnamed(unnamed) => { @@ -91,6 +100,9 @@ pub(crate) fn implement(cx: &Ctxt, en: &DataEnum) -> Result { get_mut.push(quote!(#as_storage::get_mut(&mut self.#name, v))); insert.push(quote!(#as_storage::insert(&mut self.#name, v, value))); remove.push(quote!(#as_storage::remove(&mut self.#name, v))); + retain.push(quote! { + #as_storage::retain(&mut self.#name, |k, v| func(#ident::#var(k), v)); + }); copy_bounds.push(quote!(#storage: #copy)); @@ -220,6 +232,14 @@ pub(crate) fn implement(cx: &Ctxt, en: &DataEnum) -> Result { } } + #[inline] + fn retain(&mut self, mut func: F) + where + F: FnMut(#ident, &mut V) -> bool + { + #(#retain)* + } + #[inline] fn clear(&mut self) { #(#clear;)* diff --git a/fixed-map-derive/src/unit_variants.rs b/fixed-map-derive/src/unit_variants.rs index 63987d5..8d4a6d3 100644 --- a/fixed-map-derive/src/unit_variants.rs +++ b/fixed-map-derive/src/unit_variants.rs @@ -42,6 +42,7 @@ pub(crate) fn implement(cx: &Ctxt, en: &DataEnum) -> Result { let mut get_mut = Vec::new(); let mut insert = Vec::new(); let mut remove = Vec::new(); + let mut retain = Vec::new(); let mut keys_iter_init = Vec::new(); let mut iter_init = Vec::new(); @@ -59,6 +60,13 @@ pub(crate) fn implement(cx: &Ctxt, en: &DataEnum) -> Result { get_mut.push(quote!(#option::as_mut(#name))); insert.push(quote!(#mem::replace(#name, #option::Some(value)))); remove.push(quote!(#mem::take(#name))); + retain.push(quote! { + if let Some(val) = #option::as_mut(#name) { + if !func(#ident::#var, val) { + *#name = None; + } + } + }); keys_iter_init.push(quote!(if #name.is_some() { Some(#ident::#var) } else { None })); iter_init.push(quote!((#ident::#var, #name))); names.push(name.clone()); @@ -180,6 +188,15 @@ pub(crate) fn implement(cx: &Ctxt, en: &DataEnum) -> Result { } } + #[inline] + fn retain(&mut self, mut func: F) + where + F: FnMut(#ident, &mut V) -> bool + { + let [#(#names),*] = &mut self.data; + #(#retain)* + } + #[inline] fn clear(&mut self) { self.data = [#(#field_inits),*]; diff --git a/src/map.rs b/src/map.rs index ead1a9f..3b3357e 100644 --- a/src/map.rs +++ b/src/map.rs @@ -590,6 +590,76 @@ where self.storage.remove(key) } + /// Retains only the elements specified by the predicate. + /// + /// In other words, remove all pairs (k, v) for which f(k, &mut v) returns false. + /// The elements are visited in unsorted (and unspecified) order. + /// + /// # Examples + /// + /// ``` + /// use fixed_map::{Key, Map}; + /// + /// #[derive(Clone, Copy, Key)] + /// enum Key { + /// First, + /// Second, + /// } + /// + /// let mut map: Map = Map::new(); + /// + /// map.insert(Key::First, 42); + /// map.insert(Key::Second, -10); + /// + /// map.retain(|k, v| *v > 0); + /// + /// assert_eq!(map.len(), 1); + /// assert_eq!(map.get(Key::First), Some(&42)); + /// assert_eq!(map.get(Key::Second), None); + /// ``` + /// + /// Using a composite key: + /// + /// ``` + /// use fixed_map::{Key, Map}; + /// + /// #[derive(Clone, Copy, Key)] + /// enum Key { + /// First(bool), + /// Second, + /// } + /// + /// let mut map: Map = Map::new(); + /// + /// map.insert(Key::First(true), 42); + /// map.insert(Key::First(false), -31); + /// map.insert(Key::Second, 100); + /// + /// let mut other = map.clone(); + /// assert_eq!(map.len(), 3); + /// + /// map.retain(|k, v| *v > 0); + /// + /// assert_eq!(map.len(), 2); + /// assert_eq!(map.get(Key::First(true)), Some(&42)); + /// assert_eq!(map.get(Key::First(false)), None); + /// assert_eq!(map.get(Key::Second), Some(&100)); + /// + /// other.retain(|k, v| matches!(k, Key::First(_))); + /// + /// assert_eq!(other.len(), 2); + /// assert_eq!(other.get(Key::First(true)), Some(&42)); + /// assert_eq!(other.get(Key::First(false)), Some(&-31)); + /// assert_eq!(other.get(Key::Second), None); + /// ``` + #[inline] + pub fn retain(&mut self, f: F) + where + F: FnMut(K, &mut V) -> bool, + { + self.storage.retain(f); + } + /// Clears the map, removing all key-value pairs. Keeps the allocated memory /// for reuse. /// diff --git a/src/set.rs b/src/set.rs index 8c721c2..3943636 100644 --- a/src/set.rs +++ b/src/set.rs @@ -250,6 +250,79 @@ where self.storage.remove(key).is_some() } + /// Retains only the elements specified by the predicate. + /// + /// In other words, remove all elements e for which f(e) returns false. + /// The elements are visited in unsorted (and unspecified) order. + /// + /// # Examples + /// + /// ``` + /// use fixed_map::{Key, Set}; + /// + /// #[derive(Clone, Copy, Key)] + /// enum Key { + /// First, + /// Second, + /// } + /// + /// let mut set = Set::new(); + /// + /// set.insert(Key::First); + /// set.insert(Key::Second); + /// + /// set.retain(|k| matches!(k, Key::First)); + /// + /// assert_eq!(set.len(), 1); + /// assert_eq!(set.contains(Key::First), true); + /// assert_eq!(set.contains(Key::Second), false); + /// ``` + /// + /// Using a composite key: + /// + /// ``` + /// use fixed_map::{Key, Set}; + /// + /// #[derive(Clone, Copy, Key)] + /// enum Key { + /// First(bool), + /// Second(bool), + /// } + /// + /// let mut set = Set::new(); + /// + /// set.insert(Key::First(true)); + /// set.insert(Key::First(false)); + /// set.insert(Key::Second(true)); + /// set.insert(Key::Second(false)); + /// + /// let mut other = set.clone(); + /// assert_eq!(set.len(), 4); + /// + /// set.retain(|k| matches!(k, Key::First(true) | Key::Second(true))); + /// + /// assert_eq!(set.len(), 2); + /// assert_eq!(set.contains(Key::First(true)), true); + /// assert_eq!(set.contains(Key::First(false)), false); + /// assert_eq!(set.contains(Key::Second(true)), true); + /// assert_eq!(set.contains(Key::Second(false)), false); + /// + /// other.retain(|k| matches!(k, Key::First(_))); + /// + /// assert_eq!(other.len(), 2); + /// assert_eq!(other.contains(Key::First(true)), true); + /// assert_eq!(other.contains(Key::First(false)), true); + /// assert_eq!(other.contains(Key::Second(true)), false); + /// assert_eq!(other.contains(Key::Second(false)), false); + /// ``` + #[inline] + pub fn retain(&mut self, mut f: F) + where + F: FnMut(K) -> bool, + { + self.storage.retain(|k, _| f(k)); + } + /// Clears the set, removing all values. /// /// # Examples diff --git a/src/storage.rs b/src/storage.rs index 4a0af26..384c187 100644 --- a/src/storage.rs +++ b/src/storage.rs @@ -72,6 +72,11 @@ pub trait Storage: Default { /// This is the storage abstraction for [`Map::remove`][crate::Map::remove]. fn remove(&mut self, key: K) -> Option; + /// This is the storage abstraction for [`Map::retain`][crate::Map::retain]. + fn retain(&mut self, f: F) + where + F: FnMut(K, &mut V) -> bool; + /// This is the storage abstraction for [`Map::clear`][crate::Map::clear]. fn clear(&mut self); diff --git a/src/storage/boolean.rs b/src/storage/boolean.rs index 0c5b6d2..fd0341b 100644 --- a/src/storage/boolean.rs +++ b/src/storage/boolean.rs @@ -208,6 +208,23 @@ impl Storage for BooleanStorage { } } + #[inline] + fn retain(&mut self, mut func: F) + where + F: FnMut(bool, &mut V) -> bool, + { + if let Some(t) = self.t.as_mut() { + if !func(true, t) { + self.t = None; + } + } + if let Some(f) = self.f.as_mut() { + if !func(false, f) { + self.f = None; + } + } + } + #[inline] fn clear(&mut self) { self.t = None; diff --git a/src/storage/map.rs b/src/storage/map.rs index e63fb36..782eaa4 100644 --- a/src/storage/map.rs +++ b/src/storage/map.rs @@ -163,6 +163,14 @@ where self.inner.remove(&key) } + #[inline] + fn retain(&mut self, mut func: F) + where + F: FnMut(K, &mut V) -> bool, + { + self.inner.retain(|&k, v| func(k, v)); + } + #[inline] fn clear(&mut self) { self.inner.clear(); diff --git a/src/storage/option.rs b/src/storage/option.rs index dae304e..9997858 100644 --- a/src/storage/option.rs +++ b/src/storage/option.rs @@ -187,6 +187,19 @@ where } } + #[inline] + fn retain(&mut self, mut func: F) + where + F: FnMut(Option, &mut V) -> bool, + { + self.some.retain(|k, v| func(Some(k), v)); + if let Some(none) = self.none.as_mut() { + if !func(None, none) { + self.none = None; + } + } + } + #[inline] fn clear(&mut self) { self.some.clear(); diff --git a/src/storage/singleton.rs b/src/storage/singleton.rs index 459dcdb..5744c0b 100644 --- a/src/storage/singleton.rs +++ b/src/storage/singleton.rs @@ -76,6 +76,18 @@ where mem::replace(&mut self.inner, None) } + #[inline] + fn retain(&mut self, mut func: F) + where + F: FnMut(K, &mut V) -> bool, + { + if let Some(val) = self.inner.as_mut() { + if !func(K::default(), val) { + self.inner = None; + } + } + } + #[inline] fn clear(&mut self) { self.inner = None; From 8709505c0d6bde59924dd8dbf781947de06acce9 Mon Sep 17 00:00:00 2001 From: John-John Tedro Date: Fri, 14 Oct 2022 17:39:31 +0200 Subject: [PATCH 2/4] Revert examples and introduce more granular builds --- .github/workflows/ci.yml | 52 ++++++++++++++++++++++++++++++---------- examples/basic.rs | 7 ------ examples/map_feature.rs | 47 ------------------------------------ 3 files changed, 40 insertions(+), 66 deletions(-) delete mode 100644 examples/map_feature.rs diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index a552186..41cba37 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -3,26 +3,54 @@ name: CI on: [push, pull_request] jobs: + clippy: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v2 + - uses: actions-rs/toolchain@v1 + with: + toolchain: beta + override: true + profile: minimal + components: clippy + - run: cargo clippy --workspace --all-features --tests --examples --benches -- --deny warnings + + rustfmt: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v2 + - uses: actions-rs/toolchain@v1 + with: + toolchain: beta + override: true + profile: minimal + components: rustfmt + - run: cargo fmt --check + build: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v1 + - uses: actions/checkout@v2 - uses: actions-rs/toolchain@v1 with: toolchain: beta override: true profile: minimal - components: clippy, rustfmt + - run: cargo build --all-features --examples --tests --benches - # Run library and proc macro unit tests and doc tests - # also checks that those compile ok + test: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v2 + - uses: actions-rs/toolchain@v1 + with: + toolchain: beta + override: true + profile: minimal + # Test all relevant feature combos: + # features: std, map, serde - run: cargo test --all-features + # features: -std, -map, -serde - run: cargo test --no-default-features - # Run example and bench tests - # also checks that those compile ok - - run: cargo test --all-features --examples --benches - - run: cargo test --no-default-features --examples --benches - # Run clippy on everything, denying warnings - - run: cargo clippy --workspace --all-features --tests --examples --benches -- --deny warnings - # Check that formatting is correct - - run: cargo fmt --check + # features: -std, -map, serde + - run: cargo test --no-default-features --features serde diff --git a/examples/basic.rs b/examples/basic.rs index 08a0792..cde4bf3 100644 --- a/examples/basic.rs +++ b/examples/basic.rs @@ -12,10 +12,3 @@ fn main() { assert_eq!(map.get(Key::First), Some(&42)); assert_eq!(map.get(Key::Second), None); } - -// Execute this during testing as well. -#[cfg(test)] -#[test] -fn test_main() { - main(); -} diff --git a/examples/map_feature.rs b/examples/map_feature.rs deleted file mode 100644 index fd1e5d6..0000000 --- a/examples/map_feature.rs +++ /dev/null @@ -1,47 +0,0 @@ -// This file is very useful for debugging the derive macro. -// Doctests don't work well for that purpose. - -fn main() { - #[cfg(feature = "map")] - { - use fixed_map::{Key, Map}; - - #[derive(Clone, Copy, Key)] - enum Part { - One, - Two, - } - - #[derive(Clone, Copy, Key)] - enum Key { - Simple, - Composite(Part), - String(&'static str), - Number(u32), - Singleton(()), - } - let mut map = Map::new(); - - map.insert(Key::Simple, 1); - map.insert(Key::Composite(Part::One), 2); - map.insert(Key::String("foo"), 3); - map.insert(Key::Number(1), 4); - map.insert(Key::Singleton(()), 5); - - assert_eq!(map.get(Key::Simple), Some(&1)); - assert_eq!(map.get(Key::Composite(Part::One)), Some(&2)); - assert_eq!(map.get(Key::Composite(Part::Two)), None); - assert_eq!(map.get(Key::String("foo")), Some(&3)); - assert_eq!(map.get(Key::String("bar")), None); - assert_eq!(map.get(Key::Number(1)), Some(&4)); - assert_eq!(map.get(Key::Number(2)), None); - assert_eq!(map.get(Key::Singleton(())), Some(&5)); - } -} - -// Execute this during testing as well. -#[cfg(test)] -#[test] -fn test_main() { - main(); -} From b0eee22a51982a5fbbbd78855c07d924022730a0 Mon Sep 17 00:00:00 2001 From: John-John Tedro Date: Fri, 14 Oct 2022 17:44:51 +0200 Subject: [PATCH 3/4] More compact CI config --- .github/workflows/ci.yml | 22 ++++------------------ 1 file changed, 4 insertions(+), 18 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 41cba37..9b20027 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -8,11 +8,7 @@ jobs: steps: - uses: actions/checkout@v2 - uses: actions-rs/toolchain@v1 - with: - toolchain: beta - override: true - profile: minimal - components: clippy + with: { toolchain: beta, override: true, profile: minimal, components: clippy } - run: cargo clippy --workspace --all-features --tests --examples --benches -- --deny warnings rustfmt: @@ -20,11 +16,7 @@ jobs: steps: - uses: actions/checkout@v2 - uses: actions-rs/toolchain@v1 - with: - toolchain: beta - override: true - profile: minimal - components: rustfmt + with: { toolchain: beta, override: true, profile: minimal, components: rustfmt } - run: cargo fmt --check build: @@ -32,10 +24,7 @@ jobs: steps: - uses: actions/checkout@v2 - uses: actions-rs/toolchain@v1 - with: - toolchain: beta - override: true - profile: minimal + with: { toolchain: beta, override: true, profile: minimal } - run: cargo build --all-features --examples --tests --benches test: @@ -43,10 +32,7 @@ jobs: steps: - uses: actions/checkout@v2 - uses: actions-rs/toolchain@v1 - with: - toolchain: beta - override: true - profile: minimal + with: { toolchain: beta, override: true, profile: minimal } # Test all relevant feature combos: # features: std, map, serde - run: cargo test --all-features From c0a17abd6a30d9b9cc21a8d67fc5d1fb39269ecf Mon Sep 17 00:00:00 2001 From: John-John Tedro Date: Fri, 14 Oct 2022 17:55:26 +0200 Subject: [PATCH 4/4] Reintroduce map_feature and move empty --- {examples => tests}/empty.rs | 3 ++- tests/map_feature.rs | 38 ++++++++++++++++++++++++++++++++++++ 2 files changed, 40 insertions(+), 1 deletion(-) rename {examples => tests}/empty.rs (84%) create mode 100644 tests/map_feature.rs diff --git a/examples/empty.rs b/tests/empty.rs similarity index 84% rename from examples/empty.rs rename to tests/empty.rs index 23ca852..c3586de 100644 --- a/examples/empty.rs +++ b/tests/empty.rs @@ -3,6 +3,7 @@ use fixed_map::{Key, Map}; #[derive(Debug, Clone, Copy, Key)] enum Key {} -fn main() { +#[test] +fn empty() { let _ = Map::::new(); } diff --git a/tests/map_feature.rs b/tests/map_feature.rs new file mode 100644 index 0000000..b7cc173 --- /dev/null +++ b/tests/map_feature.rs @@ -0,0 +1,38 @@ +#![cfg(feature = "map")] + +use fixed_map::{Key, Map}; + +#[derive(Clone, Copy, Key)] +enum Part { + One, + Two, +} + +#[derive(Clone, Copy, Key)] +enum Key { + Simple, + Composite(Part), + String(&'static str), + Number(u32), + Singleton(()), +} + +#[test] +fn map_feature() { + let mut map = Map::new(); + + map.insert(Key::Simple, 1); + map.insert(Key::Composite(Part::One), 2); + map.insert(Key::String("foo"), 3); + map.insert(Key::Number(1), 4); + map.insert(Key::Singleton(()), 5); + + assert_eq!(map.get(Key::Simple), Some(&1)); + assert_eq!(map.get(Key::Composite(Part::One)), Some(&2)); + assert_eq!(map.get(Key::Composite(Part::Two)), None); + assert_eq!(map.get(Key::String("foo")), Some(&3)); + assert_eq!(map.get(Key::String("bar")), None); + assert_eq!(map.get(Key::Number(1)), Some(&4)); + assert_eq!(map.get(Key::Number(2)), None); + assert_eq!(map.get(Key::Singleton(())), Some(&5)); +}