Skip to content

refactor: replace Range with a bounded implementation #112

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 13 commits into from
Jun 25, 2022
Merged

refactor: replace Range with a bounded implementation #112

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
c3a9567
refactor: replace Range with a bounded implementation
baszalmstra Apr 22, 2022
530cd9c
fix: rewrite range proptest strategy
baszalmstra May 28, 2022
bb2f2c6
fix: deserialize SmallVec without Vec alloc
baszalmstra May 28, 2022
e72a3f5
fix: remove not_equals
baszalmstra May 28, 2022
71fbe52
fix: re-add union and remove early out
baszalmstra May 28, 2022
a1ea900
fix: renamed V to VS in bench
baszalmstra May 28, 2022
d14f6fa
refactor: simpler intersection
baszalmstra Jun 7, 2022
c44639a
test: use deltas for range strategy
baszalmstra Jun 7, 2022
63277a0
docs(range): added comment about discrete values
baszalmstra Jun 10, 2022
83d8725
More restrictive for valid random range generation
mpizenberg Jun 25, 2022
3bc73f1
Remove duplicate check in check_invariants
mpizenberg Jun 25, 2022
f35c2e9
Add warning about non-unique ranges representations
mpizenberg Jun 25, 2022
ec8dfce
Rename start_bounded into start_unbounded
mpizenberg Jun 25, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 9 additions & 6 deletions benches/large_case.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,18 @@ extern crate criterion;
use self::criterion::*;

use pubgrub::package::Package;
use pubgrub::range::Range;
use pubgrub::solver::{resolve, OfflineDependencyProvider};
use pubgrub::version::{NumberVersion, SemanticVersion, Version};
use pubgrub::version::{NumberVersion, SemanticVersion};
use pubgrub::version_set::VersionSet;
use serde::de::Deserialize;
use std::hash::Hash;

fn bench<'a, P: Package + Deserialize<'a>, V: Version + Hash + Deserialize<'a>>(
fn bench<'a, P: Package + Deserialize<'a>, V: VersionSet + Deserialize<'a>>(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we call it VS: not V:, I think that will be more consistent with our usage in the rest of the package?
Then the where can become where VS::V: Deserialize<'a>.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

b: &mut Bencher,
case: &'a str,
) {
) where
<V as VersionSet>::V: Deserialize<'a>,
{
let dependency_provider: OfflineDependencyProvider<P, V> = ron::de::from_str(&case).unwrap();

b.iter(|| {
Expand All @@ -35,11 +38,11 @@ fn bench_nested(c: &mut Criterion) {
let data = std::fs::read_to_string(&case).unwrap();
if name.ends_with("u16_NumberVersion.ron") {
group.bench_function(name, |b| {
bench::<u16, NumberVersion>(b, &data);
bench::<u16, Range<NumberVersion>>(b, &data);
});
} else if name.ends_with("str_SemanticVersion.ron") {
group.bench_function(name, |b| {
bench::<&str, SemanticVersion>(b, &data);
bench::<&str, Range<SemanticVersion>>(b, &data);
});
}
}
Expand Down
6 changes: 3 additions & 3 deletions examples/doc_interface.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,10 @@ type NumVS = Range<NumberVersion>;
fn main() {
let mut dependency_provider = OfflineDependencyProvider::<&str, NumVS>::new();
dependency_provider.add_dependencies(
"root", 1, [("menu", Range::any()), ("icons", Range::any())],
"root", 1, [("menu", Range::full()), ("icons", Range::full())],
);
dependency_provider.add_dependencies("menu", 1, [("dropdown", Range::any())]);
dependency_provider.add_dependencies("dropdown", 1, [("icons", Range::any())]);
dependency_provider.add_dependencies("menu", 1, [("dropdown", Range::full())]);
dependency_provider.add_dependencies("dropdown", 1, [("icons", Range::full())]);
dependency_provider.add_dependencies("icons", 1, []);

// Run the algorithm.
Expand Down
16 changes: 8 additions & 8 deletions examples/doc_interface_error.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,9 @@ fn main() {
let mut dependency_provider = OfflineDependencyProvider::<&str, SemVS>::new();
// Direct dependencies: menu and icons.
dependency_provider.add_dependencies("root", (1, 0, 0), [
("menu", Range::any()),
("icons", Range::exact((1, 0, 0))),
("intl", Range::exact((5, 0, 0))),
("menu", Range::full()),
("icons", Range::singleton((1, 0, 0))),
("intl", Range::singleton((5, 0, 0))),
]);

// Dependencies of the menu lib.
Expand All @@ -47,19 +47,19 @@ fn main() {

// Dependencies of the dropdown lib.
dependency_provider.add_dependencies("dropdown", (1, 8, 0), [
("intl", Range::exact((3, 0, 0))),
("intl", Range::singleton((3, 0, 0))),
]);
dependency_provider.add_dependencies("dropdown", (2, 0, 0), [
("icons", Range::exact((2, 0, 0))),
("icons", Range::singleton((2, 0, 0))),
]);
dependency_provider.add_dependencies("dropdown", (2, 1, 0), [
("icons", Range::exact((2, 0, 0))),
("icons", Range::singleton((2, 0, 0))),
]);
dependency_provider.add_dependencies("dropdown", (2, 2, 0), [
("icons", Range::exact((2, 0, 0))),
("icons", Range::singleton((2, 0, 0))),
]);
dependency_provider.add_dependencies("dropdown", (2, 3, 0), [
("icons", Range::exact((2, 0, 0))),
("icons", Range::singleton((2, 0, 0))),
]);

// Icons have no dependencies.
Expand Down
12 changes: 6 additions & 6 deletions examples/doc_interface_semantic.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@ fn main() {
let mut dependency_provider = OfflineDependencyProvider::<&str, SemVS>::new();
// Direct dependencies: menu and icons.
dependency_provider.add_dependencies("root", (1, 0, 0), [
("menu", Range::any()),
("icons", Range::exact((1, 0, 0))),
("menu", Range::full()),
("icons", Range::singleton((1, 0, 0))),
]);

// Dependencies of the menu lib.
Expand All @@ -46,16 +46,16 @@ fn main() {
// Dependencies of the dropdown lib.
dependency_provider.add_dependencies("dropdown", (1, 8, 0), []);
dependency_provider.add_dependencies("dropdown", (2, 0, 0), [
("icons", Range::exact((2, 0, 0))),
("icons", Range::singleton((2, 0, 0))),
]);
dependency_provider.add_dependencies("dropdown", (2, 1, 0), [
("icons", Range::exact((2, 0, 0))),
("icons", Range::singleton((2, 0, 0))),
]);
dependency_provider.add_dependencies("dropdown", (2, 2, 0), [
("icons", Range::exact((2, 0, 0))),
("icons", Range::singleton((2, 0, 0))),
]);
dependency_provider.add_dependencies("dropdown", (2, 3, 0), [
("icons", Range::exact((2, 0, 0))),
("icons", Range::singleton((2, 0, 0))),
]);

// Icons has no dependency.
Expand Down
4 changes: 2 additions & 2 deletions src/internal/incompatibility.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -276,12 +276,12 @@ pub mod tests {
let mut store = Arena::new();
let i1 = store.alloc(Incompatibility {
package_terms: SmallMap::Two([("p1", t1.clone()), ("p2", t2.negate())]),
kind: Kind::UnavailableDependencies("0", Range::any())
kind: Kind::UnavailableDependencies("0", Range::full())
});

let i2 = store.alloc(Incompatibility {
package_terms: SmallMap::Two([("p2", t2), ("p3", t3.clone())]),
kind: Kind::UnavailableDependencies("0", Range::any())
kind: Kind::UnavailableDependencies("0", Range::full())
});

let mut i3 = Map::default();
Expand Down
42 changes: 42 additions & 0 deletions src/internal/small_vec.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use std::fmt;
use std::hash::{Hash, Hasher};
use std::ops::Deref;

#[derive(Clone)]
Expand Down Expand Up @@ -108,6 +109,13 @@ impl<T: fmt::Debug> fmt::Debug for SmallVec<T> {
}
}

impl<T: Hash> Hash for SmallVec<T> {
fn hash<H: Hasher>(&self, state: &mut H) {
self.len().hash(state);
Hash::hash_slice(self.as_slice(), state);
}
}

#[cfg(feature = "serde")]
impl<T: serde::Serialize> serde::Serialize for SmallVec<T> {
fn serialize<S: serde::Serializer>(&self, s: S) -> Result<S::Ok, S::Error> {
Expand All @@ -128,6 +136,40 @@ impl<'de, T: serde::Deserialize<'de>> serde::Deserialize<'de> for SmallVec<T> {
}
}

impl<T> IntoIterator for SmallVec<T> {
type Item = T;
type IntoIter = SmallVecIntoIter<T>;

fn into_iter(self) -> Self::IntoIter {
match self {
SmallVec::Empty => SmallVecIntoIter::Empty,
SmallVec::One(a) => SmallVecIntoIter::One(IntoIterator::into_iter(a)),
SmallVec::Two(a) => SmallVecIntoIter::Two(IntoIterator::into_iter(a)),
SmallVec::Flexible(v) => SmallVecIntoIter::Flexible(IntoIterator::into_iter(v)),
}
}
}

pub enum SmallVecIntoIter<T> {
Empty,
One(<[T; 1] as IntoIterator>::IntoIter),
Two(<[T; 2] as IntoIterator>::IntoIter),
Flexible(<Vec<T> as IntoIterator>::IntoIter),
}

impl<T> Iterator for SmallVecIntoIter<T> {
type Item = T;

fn next(&mut self) -> Option<Self::Item> {
match self {
SmallVecIntoIter::Empty => None,
SmallVecIntoIter::One(it) => it.next(),
SmallVecIntoIter::Two(it) => it.next(),
SmallVecIntoIter::Flexible(it) => it.next(),
}
}
}

// TESTS #######################################################################

#[cfg(test)]
Expand Down
6 changes: 3 additions & 3 deletions src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,10 +55,10 @@
//! let mut dependency_provider = OfflineDependencyProvider::<&str, NumVS>::new();
//!
//! dependency_provider.add_dependencies(
//! "root", 1, [("menu", Range::any()), ("icons", Range::any())],
//! "root", 1, [("menu", Range::full()), ("icons", Range::full())],
//! );
//! dependency_provider.add_dependencies("menu", 1, [("dropdown", Range::any())]);
//! dependency_provider.add_dependencies("dropdown", 1, [("icons", Range::any())]);
//! dependency_provider.add_dependencies("menu", 1, [("dropdown", Range::full())]);
//! dependency_provider.add_dependencies("dropdown", 1, [("icons", Range::full())]);
//! dependency_provider.add_dependencies("icons", 1, []);
//!
//! // Run the algorithm.
Expand Down
Loading