Skip to content

Commit 4a94e7f

Browse files
baszalmstraEh2406mpizenberg
authored and
zanieb
committed
refactor: replace Range with a bounded implementation (pubgrub-rs#112)
* refactor: replace Range with a bounded implementation * fix: rewrite range proptest strategy * fix: deserialize SmallVec without Vec alloc * fix: remove not_equals * fix: re-add union and remove early out * fix: renamed V to VS in bench * refactor: simpler intersection Co-authored-by: Jacob Finkelman <[email protected]> * test: use deltas for range strategy Co-authored-by: Jacob Finkelman <[email protected]> * docs(range): added comment about discrete values * More restrictive for valid random range generation * Remove duplicate check in check_invariants * Add warning about non-unique ranges representations * Rename start_bounded into start_unbounded Co-authored-by: Jacob Finkelman <[email protected]> Co-authored-by: Matthieu Pizenberg <[email protected]>
1 parent 05fbc2f commit 4a94e7f

12 files changed

+529
-345
lines changed

‎benches/large_case.rs

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -5,16 +5,19 @@ extern crate criterion;
55
use self::criterion::*;
66

77
use pubgrub::package::Package;
8+
use pubgrub::range::Range;
89
use pubgrub::solver::{resolve, OfflineDependencyProvider};
9-
use pubgrub::version::{NumberVersion, SemanticVersion, Version};
10+
use pubgrub::version::{NumberVersion, SemanticVersion};
11+
use pubgrub::version_set::VersionSet;
1012
use serde::de::Deserialize;
11-
use std::hash::Hash;
1213

13-
fn bench<'a, P: Package + Deserialize<'a>, V: Version + Hash + Deserialize<'a>>(
14+
fn bench<'a, P: Package + Deserialize<'a>, VS: VersionSet + Deserialize<'a>>(
1415
b: &mut Bencher,
1516
case: &'a str,
16-
) {
17-
let dependency_provider: OfflineDependencyProvider<P, V> = ron::de::from_str(&case).unwrap();
17+
) where
18+
<VS as VersionSet>::V: Deserialize<'a>,
19+
{
20+
let dependency_provider: OfflineDependencyProvider<P, VS> = ron::de::from_str(&case).unwrap();
1821

1922
b.iter(|| {
2023
for p in dependency_provider.packages() {
@@ -35,11 +38,11 @@ fn bench_nested(c: &mut Criterion) {
3538
let data = std::fs::read_to_string(&case).unwrap();
3639
if name.ends_with("u16_NumberVersion.ron") {
3740
group.bench_function(name, |b| {
38-
bench::<u16, NumberVersion>(b, &data);
41+
bench::<u16, Range<NumberVersion>>(b, &data);
3942
});
4043
} else if name.ends_with("str_SemanticVersion.ron") {
4144
group.bench_function(name, |b| {
42-
bench::<&str, SemanticVersion>(b, &data);
45+
bench::<&str, Range<SemanticVersion>>(b, &data);
4346
});
4447
}
4548
}

‎examples/doc_interface.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,10 +14,10 @@ type NumVS = Range<NumberVersion>;
1414
fn main() {
1515
let mut dependency_provider = OfflineDependencyProvider::<&str, NumVS>::new();
1616
dependency_provider.add_dependencies(
17-
"root", 1, [("menu", Range::any()), ("icons", Range::any())],
17+
"root", 1, [("menu", Range::full()), ("icons", Range::full())],
1818
);
19-
dependency_provider.add_dependencies("menu", 1, [("dropdown", Range::any())]);
20-
dependency_provider.add_dependencies("dropdown", 1, [("icons", Range::any())]);
19+
dependency_provider.add_dependencies("menu", 1, [("dropdown", Range::full())]);
20+
dependency_provider.add_dependencies("dropdown", 1, [("icons", Range::full())]);
2121
dependency_provider.add_dependencies("icons", 1, []);
2222

2323
// Run the algorithm.

‎examples/doc_interface_error.rs

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -20,9 +20,9 @@ fn main() {
2020
let mut dependency_provider = OfflineDependencyProvider::<&str, SemVS>::new();
2121
// Direct dependencies: menu and icons.
2222
dependency_provider.add_dependencies("root", (1, 0, 0), [
23-
("menu", Range::any()),
24-
("icons", Range::exact((1, 0, 0))),
25-
("intl", Range::exact((5, 0, 0))),
23+
("menu", Range::full()),
24+
("icons", Range::singleton((1, 0, 0))),
25+
("intl", Range::singleton((5, 0, 0))),
2626
]);
2727

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

4848
// Dependencies of the dropdown lib.
4949
dependency_provider.add_dependencies("dropdown", (1, 8, 0), [
50-
("intl", Range::exact((3, 0, 0))),
50+
("intl", Range::singleton((3, 0, 0))),
5151
]);
5252
dependency_provider.add_dependencies("dropdown", (2, 0, 0), [
53-
("icons", Range::exact((2, 0, 0))),
53+
("icons", Range::singleton((2, 0, 0))),
5454
]);
5555
dependency_provider.add_dependencies("dropdown", (2, 1, 0), [
56-
("icons", Range::exact((2, 0, 0))),
56+
("icons", Range::singleton((2, 0, 0))),
5757
]);
5858
dependency_provider.add_dependencies("dropdown", (2, 2, 0), [
59-
("icons", Range::exact((2, 0, 0))),
59+
("icons", Range::singleton((2, 0, 0))),
6060
]);
6161
dependency_provider.add_dependencies("dropdown", (2, 3, 0), [
62-
("icons", Range::exact((2, 0, 0))),
62+
("icons", Range::singleton((2, 0, 0))),
6363
]);
6464

6565
// Icons have no dependencies.

‎examples/doc_interface_semantic.rs

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,8 @@ fn main() {
1919
let mut dependency_provider = OfflineDependencyProvider::<&str, SemVS>::new();
2020
// Direct dependencies: menu and icons.
2121
dependency_provider.add_dependencies("root", (1, 0, 0), [
22-
("menu", Range::any()),
23-
("icons", Range::exact((1, 0, 0))),
22+
("menu", Range::full()),
23+
("icons", Range::singleton((1, 0, 0))),
2424
]);
2525

2626
// Dependencies of the menu lib.
@@ -46,16 +46,16 @@ fn main() {
4646
// Dependencies of the dropdown lib.
4747
dependency_provider.add_dependencies("dropdown", (1, 8, 0), []);
4848
dependency_provider.add_dependencies("dropdown", (2, 0, 0), [
49-
("icons", Range::exact((2, 0, 0))),
49+
("icons", Range::singleton((2, 0, 0))),
5050
]);
5151
dependency_provider.add_dependencies("dropdown", (2, 1, 0), [
52-
("icons", Range::exact((2, 0, 0))),
52+
("icons", Range::singleton((2, 0, 0))),
5353
]);
5454
dependency_provider.add_dependencies("dropdown", (2, 2, 0), [
55-
("icons", Range::exact((2, 0, 0))),
55+
("icons", Range::singleton((2, 0, 0))),
5656
]);
5757
dependency_provider.add_dependencies("dropdown", (2, 3, 0), [
58-
("icons", Range::exact((2, 0, 0))),
58+
("icons", Range::singleton((2, 0, 0))),
5959
]);
6060

6161
// Icons has no dependency.

‎src/internal/incompatibility.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -276,12 +276,12 @@ pub mod tests {
276276
let mut store = Arena::new();
277277
let i1 = store.alloc(Incompatibility {
278278
package_terms: SmallMap::Two([("p1", t1.clone()), ("p2", t2.negate())]),
279-
kind: Kind::UnavailableDependencies("0", Range::any())
279+
kind: Kind::UnavailableDependencies("0", Range::full())
280280
});
281281

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

287287
let mut i3 = Map::default();

‎src/internal/small_vec.rs

Lines changed: 70 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
use std::fmt;
2+
use std::hash::{Hash, Hasher};
23
use std::ops::Deref;
34

45
#[derive(Clone)]
@@ -108,6 +109,13 @@ impl<T: fmt::Debug> fmt::Debug for SmallVec<T> {
108109
}
109110
}
110111

112+
impl<T: Hash> Hash for SmallVec<T> {
113+
fn hash<H: Hasher>(&self, state: &mut H) {
114+
self.len().hash(state);
115+
Hash::hash_slice(self.as_slice(), state);
116+
}
117+
}
118+
111119
#[cfg(feature = "serde")]
112120
impl<T: serde::Serialize> serde::Serialize for SmallVec<T> {
113121
fn serialize<S: serde::Serializer>(&self, s: S) -> Result<S::Ok, S::Error> {
@@ -118,13 +126,70 @@ impl<T: serde::Serialize> serde::Serialize for SmallVec<T> {
118126
#[cfg(feature = "serde")]
119127
impl<'de, T: serde::Deserialize<'de>> serde::Deserialize<'de> for SmallVec<T> {
120128
fn deserialize<D: serde::Deserializer<'de>>(d: D) -> Result<Self, D::Error> {
121-
let items: Vec<T> = serde::Deserialize::deserialize(d)?;
129+
struct SmallVecVisitor<T> {
130+
marker: std::marker::PhantomData<T>,
131+
}
132+
133+
impl<'de, T> serde::de::Visitor<'de> for SmallVecVisitor<T>
134+
where
135+
T: serde::Deserialize<'de>,
136+
{
137+
type Value = SmallVec<T>;
138+
139+
fn expecting(&self, formatter: &mut fmt::Formatter) -> fmt::Result {
140+
formatter.write_str("a sequence")
141+
}
142+
143+
fn visit_seq<A>(self, mut seq: A) -> Result<Self::Value, A::Error>
144+
where
145+
A: serde::de::SeqAccess<'de>,
146+
{
147+
let mut values = SmallVec::empty();
148+
while let Some(value) = seq.next_element()? {
149+
values.push(value);
150+
}
151+
Ok(values)
152+
}
153+
}
154+
155+
let visitor = SmallVecVisitor {
156+
marker: Default::default(),
157+
};
158+
d.deserialize_seq(visitor)
159+
}
160+
}
161+
162+
impl<T> IntoIterator for SmallVec<T> {
163+
type Item = T;
164+
type IntoIter = SmallVecIntoIter<T>;
122165

123-
let mut v = Self::empty();
124-
for item in items {
125-
v.push(item);
166+
fn into_iter(self) -> Self::IntoIter {
167+
match self {
168+
SmallVec::Empty => SmallVecIntoIter::Empty,
169+
SmallVec::One(a) => SmallVecIntoIter::One(IntoIterator::into_iter(a)),
170+
SmallVec::Two(a) => SmallVecIntoIter::Two(IntoIterator::into_iter(a)),
171+
SmallVec::Flexible(v) => SmallVecIntoIter::Flexible(IntoIterator::into_iter(v)),
172+
}
173+
}
174+
}
175+
176+
pub enum SmallVecIntoIter<T> {
177+
Empty,
178+
One(<[T; 1] as IntoIterator>::IntoIter),
179+
Two(<[T; 2] as IntoIterator>::IntoIter),
180+
Flexible(<Vec<T> as IntoIterator>::IntoIter),
181+
}
182+
183+
impl<T> Iterator for SmallVecIntoIter<T> {
184+
type Item = T;
185+
186+
fn next(&mut self) -> Option<Self::Item> {
187+
match self {
188+
SmallVecIntoIter::Empty => None,
189+
SmallVecIntoIter::One(it) => it.next(),
190+
SmallVecIntoIter::Two(it) => it.next(),
191+
SmallVecIntoIter::Flexible(it) => it.next(),
126192
}
127-
Ok(v)
128193
}
129194
}
130195

‎src/lib.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -55,10 +55,10 @@
5555
//! let mut dependency_provider = OfflineDependencyProvider::<&str, NumVS>::new();
5656
//!
5757
//! dependency_provider.add_dependencies(
58-
//! "root", 1, [("menu", Range::any()), ("icons", Range::any())],
58+
//! "root", 1, [("menu", Range::full()), ("icons", Range::full())],
5959
//! );
60-
//! dependency_provider.add_dependencies("menu", 1, [("dropdown", Range::any())]);
61-
//! dependency_provider.add_dependencies("dropdown", 1, [("icons", Range::any())]);
60+
//! dependency_provider.add_dependencies("menu", 1, [("dropdown", Range::full())]);
61+
//! dependency_provider.add_dependencies("dropdown", 1, [("icons", Range::full())]);
6262
//! dependency_provider.add_dependencies("icons", 1, []);
6363
//!
6464
//! // Run the algorithm.

0 commit comments

Comments
 (0)