feat(value): introduce Set storage abstraction#740
Open
anakrish wants to merge 1 commit into
Open
Conversation
There was a problem hiding this comment.
Pull request overview
Introduces a new opaque value::Set container (mirroring value::Object) to abstract set storage behind a curated API, while freeing the Set identifier by renaming the internal lib.rs collection alias to MapSet.
Changes:
- Added
src/value/set/(Set type, iterator wrappers, and serde support) and re-exportedSet(andSetCursorbehindrvm) fromsrc/value/mod.rs. - Updated the single in-tree consumer of the old crate-internal
Setalias (CompiledPolicyData::rule_paths) to useMapSet. - Added unit tests for
Setbehavior and serde round-trips; addeddocs/value/set.mddescribing the design.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/value/tests.rs | Adds test coverage for the new Set API (iteration, algebra, cursor, serde, COW behavior via Value::Set). |
| src/value/set/mod.rs | Implements the new Set abstraction, including cursor iteration and set algebra helpers. |
| src/value/set/iter.rs | Adds opaque iterator newtypes and IntoIterator impls for Set and &Set. |
| src/value/set/serde.rs | Implements Serialize/Deserialize for Set, with per-element memory-limit checks during deserialization. |
| src/value/mod.rs | Wires the new set module and re-exports Set (and SetCursor behind rvm). |
| src/lib.rs | Renames the crate-internal Set collection alias to MapSet to avoid colliding with the new public Set type. |
| src/compiled_policy.rs | Updates rule_paths storage type from Set<String> to MapSet<String>. |
| docs/value/set.md | Documents the new Set abstraction and intended backend-swappability. |
Collaborator
Author
|
@copilot review this PR using all the review skills in this repo. Use a separate agent for each skill. |
2dd9c43 to
39fff42
Compare
Add an opaque `Set` newtype paralleling `Object`, living under `src/value/set/` with the same module structure (`mod.rs` / `iter.rs` / `serde.rs`). `Set` wraps `BTreeSet<Value>` today but exposes only a curated surface: `contains`, `insert`, `remove`, `iter`, `iter_sorted`, `cursor` (resumable), `is_subset`, `intersection`, `difference`, serde, and a hand-written `Ord`. The cursor types are re-exported behind the `rvm` feature so the follow-up `IterationState::Set` swap can land additively. To free the `Set` name for the new public type, the crate-internal `BTreeSet as Set` / `HashSet as Set` aliases in `lib.rs` are renamed to `MapSet`. All in-tree consumers of the old alias are updated in lockstep. `Value::Set` is unchanged in this commit (still wraps `Rc<BTreeSet<Value>>`); the payload swap and call-site migration ship in the next PR. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
39fff42 to
923da90
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds an opaque
Setnewtype parallelingObject, living undersrc/value/set/with the same module structure assrc/value/object/(mod.rs/iter.rs/serde.rs).SetwrapsBTreeSet<Value>today but exposes only a curated surface:contains,insert,remove,iter/iter_sorted,cursor(resumable),is_subset,intersection,difference, serde, and a hand-writtenOrd. The cursor type is re-exported behind thervmfeature so the follow-upIterationState::Setswap can land additively.To free the
Setname for the new public type, the crate-internalBTreeSet as Set/HashSet as Setalias inlib.rsis renamed toMapSet. Only one in-tree consumer existed (compiled_policy::CompiledPolicy::rule_paths) and is updated in lockstep.Value::Setis unchanged in this PR (stillRc<BTreeSet<Value>>); the payload swap and call-site migration ship in a follow-up PR.Tests live in
src/value/tests.rsalongside the Object tests.