Skip to content

Commit 28c63ce

Browse files
authored
feat!: allocation free errors (pubgrub-rs#168)
1 parent 75c4a35 commit 28c63ce

File tree

7 files changed

+55
-60
lines changed

7 files changed

+55
-60
lines changed

examples/caching_dependency_provider.rs

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
// SPDX-License-Identifier: MPL-2.0
22

33
use std::cell::RefCell;
4-
use std::error::Error;
54

65
use pubgrub::package::Package;
76
use pubgrub::range::Range;
@@ -37,7 +36,7 @@ impl<P: Package, VS: VersionSet, DP: DependencyProvider<P, VS>> DependencyProvid
3736
&self,
3837
package: &P,
3938
version: &VS::V,
40-
) -> Result<Dependencies<P, VS>, Box<dyn Error + Send + Sync>> {
39+
) -> Result<Dependencies<P, VS>, DP::Err> {
4140
let mut cache = self.cached_dependencies.borrow_mut();
4241
match cache.get_dependencies(package, version) {
4342
Ok(Dependencies::Unknown) => {
@@ -55,16 +54,12 @@ impl<P: Package, VS: VersionSet, DP: DependencyProvider<P, VS>> DependencyProvid
5554
error @ Err(_) => error,
5655
}
5756
}
58-
dependencies @ Ok(_) => dependencies,
59-
error @ Err(_) => error,
57+
Ok(dependencies) => Ok(dependencies),
58+
Err(_) => unreachable!(),
6059
}
6160
}
6261

63-
fn choose_version(
64-
&self,
65-
package: &P,
66-
range: &VS,
67-
) -> Result<Option<VS::V>, Box<dyn Error + Send + Sync>> {
62+
fn choose_version(&self, package: &P, range: &VS) -> Result<Option<VS::V>, DP::Err> {
6863
self.remote_dependencies.choose_version(package, range)
6964
}
7065

@@ -73,6 +68,8 @@ impl<P: Package, VS: VersionSet, DP: DependencyProvider<P, VS>> DependencyProvid
7368
fn prioritize(&self, package: &P, range: &VS) -> Self::Priority {
7469
self.remote_dependencies.prioritize(package, range)
7570
}
71+
72+
type Err = DP::Err;
7673
}
7774

7875
fn main() {

src/error.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ use crate::version_set::VersionSet;
1010

1111
/// Errors that may occur while solving dependencies.
1212
#[derive(Error, Debug)]
13-
pub enum PubGrubError<P: Package, VS: VersionSet> {
13+
pub enum PubGrubError<P: Package, VS: VersionSet, E: std::error::Error> {
1414
/// There is no solution for this set of dependencies.
1515
#[error("No solution")]
1616
NoSolution(DerivationTree<P, VS>),
@@ -27,7 +27,7 @@ pub enum PubGrubError<P: Package, VS: VersionSet> {
2727
version: VS::V,
2828
/// Error raised by the implementer of
2929
/// [DependencyProvider](crate::solver::DependencyProvider).
30-
source: Box<dyn std::error::Error + Send + Sync>,
30+
source: E,
3131
},
3232

3333
/// Error arising when the implementer of
@@ -48,12 +48,12 @@ pub enum PubGrubError<P: Package, VS: VersionSet> {
4848
/// returned an error in the method
4949
/// [choose_version](crate::solver::DependencyProvider::choose_version).
5050
#[error("Decision making failed")]
51-
ErrorChoosingPackageVersion(Box<dyn std::error::Error + Send + Sync>),
51+
ErrorChoosingPackageVersion(E),
5252

5353
/// Error arising when the implementer of [DependencyProvider](crate::solver::DependencyProvider)
5454
/// returned an error in the method [should_cancel](crate::solver::DependencyProvider::should_cancel).
5555
#[error("We should cancel")]
56-
ErrorInShouldCancel(Box<dyn std::error::Error + Send + Sync>),
56+
ErrorInShouldCancel(E),
5757

5858
/// Something unexpected happened.
5959
#[error("{0}")]

src/internal/core.rs

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
//! to write a functional PubGrub algorithm.
55
66
use std::collections::HashSet as Set;
7+
use std::error::Error;
78

89
use crate::error::PubGrubError;
910
use crate::internal::arena::Arena;
@@ -92,7 +93,7 @@ impl<P: Package, VS: VersionSet, Priority: Ord + Clone> State<P, VS, Priority> {
9293

9394
/// Unit propagation is the core mechanism of the solving algorithm.
9495
/// CF <https://github.com/dart-lang/pub/blob/master/doc/solver.md#unit-propagation>
95-
pub fn unit_propagation(&mut self, package: P) -> Result<(), PubGrubError<P, VS>> {
96+
pub fn unit_propagation<E: Error>(&mut self, package: P) -> Result<(), PubGrubError<P, VS, E>> {
9697
self.unit_propagation_buffer.clear();
9798
self.unit_propagation_buffer.push(package);
9899
while let Some(current_package) = self.unit_propagation_buffer.pop() {
@@ -154,10 +155,11 @@ impl<P: Package, VS: VersionSet, Priority: Ord + Clone> State<P, VS, Priority> {
154155

155156
/// Return the root cause and the backtracked model.
156157
/// CF <https://github.com/dart-lang/pub/blob/master/doc/solver.md#unit-propagation>
157-
fn conflict_resolution(
158+
#[allow(clippy::type_complexity)]
159+
fn conflict_resolution<E: Error>(
158160
&mut self,
159161
incompatibility: IncompId<P, VS>,
160-
) -> Result<(P, IncompId<P, VS>), PubGrubError<P, VS>> {
162+
) -> Result<(P, IncompId<P, VS>), PubGrubError<P, VS, E>> {
161163
let mut current_incompat_id = incompatibility;
162164
let mut current_incompat_changed = false;
163165
loop {

src/lib.rs

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -83,13 +83,14 @@
8383
//! # use pubgrub::type_aliases::Map;
8484
//! # use std::error::Error;
8585
//! # use std::borrow::Borrow;
86+
//! # use std::convert::Infallible;
8687
//! #
8788
//! # struct MyDependencyProvider;
8889
//! #
8990
//! type SemVS = Range<SemanticVersion>;
9091
//!
9192
//! impl DependencyProvider<String, SemVS> for MyDependencyProvider {
92-
//! fn choose_version(&self, package: &String, range: &SemVS) -> Result<Option<SemanticVersion>, Box<dyn Error + Send + Sync>> {
93+
//! fn choose_version(&self, package: &String, range: &SemVS) -> Result<Option<SemanticVersion>, Infallible> {
9394
//! unimplemented!()
9495
//! }
9596
//!
@@ -102,9 +103,11 @@
102103
//! &self,
103104
//! package: &String,
104105
//! version: &SemanticVersion,
105-
//! ) -> Result<Dependencies<String, SemVS>, Box<dyn Error + Send + Sync>> {
106+
//! ) -> Result<Dependencies<String, SemVS>, Infallible> {
106107
//! unimplemented!()
107108
//! }
109+
//!
110+
//! type Err = Infallible;
108111
//! }
109112
//! ```
110113
//!

src/solver.rs

Lines changed: 20 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -41,14 +41,15 @@
4141
//! ## API
4242
//!
4343
//! ```
44+
//! # use std::convert::Infallible;
4445
//! # use pubgrub::solver::{resolve, OfflineDependencyProvider};
4546
//! # use pubgrub::version::NumberVersion;
4647
//! # use pubgrub::error::PubGrubError;
4748
//! # use pubgrub::range::Range;
4849
//! #
4950
//! # type NumVS = Range<NumberVersion>;
5051
//! #
51-
//! # fn try_main() -> Result<(), PubGrubError<&'static str, NumVS>> {
52+
//! # fn try_main() -> Result<(), PubGrubError<&'static str, NumVS, Infallible>> {
5253
//! # let dependency_provider = OfflineDependencyProvider::<&str, NumVS>::new();
5354
//! # let package = "root";
5455
//! # let version = 1;
@@ -70,6 +71,7 @@
7071
7172
use std::cmp::Reverse;
7273
use std::collections::{BTreeMap, BTreeSet as Set};
74+
use std::convert::Infallible;
7375
use std::error::Error;
7476

7577
use crate::error::PubGrubError;
@@ -82,11 +84,12 @@ use log::{debug, info};
8284

8385
/// Main function of the library.
8486
/// Finds a set of packages satisfying dependency bounds for a given package + version pair.
85-
pub fn resolve<P: Package, VS: VersionSet>(
86-
dependency_provider: &impl DependencyProvider<P, VS>,
87+
#[allow(clippy::type_complexity)]
88+
pub fn resolve<P: Package, VS: VersionSet, DP: DependencyProvider<P, VS>>(
89+
dependency_provider: &DP,
8790
package: P,
8891
version: impl Into<VS::V>,
89-
) -> Result<SelectedDependencies<P, VS::V>, PubGrubError<P, VS>> {
92+
) -> Result<SelectedDependencies<P, VS::V>, PubGrubError<P, VS, DP::Err>> {
9093
let mut state = State::init(package.clone(), version.into());
9194
let mut added_dependencies: Map<P, Set<VS::V>> = Map::default();
9295
let mut next = package;
@@ -133,7 +136,7 @@ pub fn resolve<P: Package, VS: VersionSet>(
133136
};
134137

135138
if !term_intersection.contains(&v) {
136-
return Err(PubGrubError::ErrorChoosingPackageVersion(
139+
return Err(PubGrubError::Failure(
137140
"choose_package_version picked an incompatible version".into(),
138141
));
139142
}
@@ -240,29 +243,30 @@ pub trait DependencyProvider<P: Package, VS: VersionSet> {
240243
/// the fewest versions that match the outstanding constraint.
241244
type Priority: Ord + Clone;
242245

246+
/// The kind of error returned from these methods.
247+
///
248+
/// Returning this signals that resolution should fail with this error.
249+
type Err: Error;
250+
243251
/// Once the resolver has found the highest `Priority` package from all potential valid
244252
/// packages, it needs to know what vertion of that package to use. The most common pattern
245253
/// is to select the largest vertion that the range contains.
246-
fn choose_version(
247-
&self,
248-
package: &P,
249-
range: &VS,
250-
) -> Result<Option<VS::V>, Box<dyn Error + Send + Sync>>;
254+
fn choose_version(&self, package: &P, range: &VS) -> Result<Option<VS::V>, Self::Err>;
251255

252256
/// Retrieves the package dependencies.
253257
/// Return [Dependencies::Unknown] if its dependencies are unknown.
254258
fn get_dependencies(
255259
&self,
256260
package: &P,
257261
version: &VS::V,
258-
) -> Result<Dependencies<P, VS>, Box<dyn Error + Send + Sync>>;
262+
) -> Result<Dependencies<P, VS>, Self::Err>;
259263

260264
/// This is called fairly regularly during the resolution,
261265
/// if it returns an Err then resolution will be terminated.
262266
/// This is helpful if you want to add some form of early termination like a timeout,
263267
/// or you want to add some form of user feedback if things are taking a while.
264268
/// If not provided the resolver will run as long as needed.
265-
fn should_cancel(&self) -> Result<(), Box<dyn Error + Send + Sync>> {
269+
fn should_cancel(&self) -> Result<(), Self::Err> {
266270
Ok(())
267271
}
268272
}
@@ -340,11 +344,9 @@ impl<P: Package, VS: VersionSet> OfflineDependencyProvider<P, VS> {
340344
/// But, that may change in new versions if better heuristics are found.
341345
/// Versions are picked with the newest versions first.
342346
impl<P: Package, VS: VersionSet> DependencyProvider<P, VS> for OfflineDependencyProvider<P, VS> {
343-
fn choose_version(
344-
&self,
345-
package: &P,
346-
range: &VS,
347-
) -> Result<Option<VS::V>, Box<dyn Error + Send + Sync>> {
347+
type Err = Infallible;
348+
349+
fn choose_version(&self, package: &P, range: &VS) -> Result<Option<VS::V>, Infallible> {
348350
Ok(self
349351
.dependencies
350352
.get(package)
@@ -365,7 +367,7 @@ impl<P: Package, VS: VersionSet> DependencyProvider<P, VS> for OfflineDependency
365367
&self,
366368
package: &P,
367369
version: &VS::V,
368-
) -> Result<Dependencies<P, VS>, Box<dyn Error + Send + Sync>> {
370+
) -> Result<Dependencies<P, VS>, Infallible> {
369371
Ok(match self.dependencies(package, version) {
370372
None => Dependencies::Unknown,
371373
Some(dependencies) => Dependencies::Known(dependencies),

tests/proptest.rs

Lines changed: 12 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
// SPDX-License-Identifier: MPL-2.0
22

3-
use std::{collections::BTreeSet as Set, error::Error};
3+
use std::collections::BTreeSet as Set;
4+
use std::convert::Infallible;
45

56
use pubgrub::error::PubGrubError;
67
use pubgrub::package::Package;
@@ -30,19 +31,11 @@ struct OldestVersionsDependencyProvider<P: Package, VS: VersionSet>(
3031
impl<P: Package, VS: VersionSet> DependencyProvider<P, VS>
3132
for OldestVersionsDependencyProvider<P, VS>
3233
{
33-
fn get_dependencies(
34-
&self,
35-
p: &P,
36-
v: &VS::V,
37-
) -> Result<Dependencies<P, VS>, Box<dyn Error + Send + Sync>> {
34+
fn get_dependencies(&self, p: &P, v: &VS::V) -> Result<Dependencies<P, VS>, Infallible> {
3835
self.0.get_dependencies(p, v)
3936
}
4037

41-
fn choose_version(
42-
&self,
43-
package: &P,
44-
range: &VS,
45-
) -> Result<Option<VS::V>, Box<dyn Error + Send + Sync>> {
38+
fn choose_version(&self, package: &P, range: &VS) -> Result<Option<VS::V>, Infallible> {
4639
Ok(self
4740
.0
4841
.versions(package)
@@ -57,6 +50,8 @@ impl<P: Package, VS: VersionSet> DependencyProvider<P, VS>
5750
fn prioritize(&self, package: &P, range: &VS) -> Self::Priority {
5851
self.0.prioritize(package, range)
5952
}
53+
54+
type Err = Infallible;
6055
}
6156

6257
/// The same as DP but it has a timeout.
@@ -82,27 +77,19 @@ impl<DP> TimeoutDependencyProvider<DP> {
8277
impl<P: Package, VS: VersionSet, DP: DependencyProvider<P, VS>> DependencyProvider<P, VS>
8378
for TimeoutDependencyProvider<DP>
8479
{
85-
fn get_dependencies(
86-
&self,
87-
p: &P,
88-
v: &VS::V,
89-
) -> Result<Dependencies<P, VS>, Box<dyn Error + Send + Sync>> {
80+
fn get_dependencies(&self, p: &P, v: &VS::V) -> Result<Dependencies<P, VS>, DP::Err> {
9081
self.dp.get_dependencies(p, v)
9182
}
9283

93-
fn should_cancel(&self) -> Result<(), Box<dyn Error + Send + Sync>> {
84+
fn should_cancel(&self) -> Result<(), DP::Err> {
9485
assert!(self.start_time.elapsed().as_secs() < 60);
9586
let calls = self.call_count.get();
9687
assert!(calls < self.max_calls);
9788
self.call_count.set(calls + 1);
9889
Ok(())
9990
}
10091

101-
fn choose_version(
102-
&self,
103-
package: &P,
104-
range: &VS,
105-
) -> Result<Option<VS::V>, Box<dyn Error + Send + Sync>> {
92+
fn choose_version(&self, package: &P, range: &VS) -> Result<Option<VS::V>, DP::Err> {
10693
self.dp.choose_version(package, range)
10794
}
10895

@@ -111,13 +98,15 @@ impl<P: Package, VS: VersionSet, DP: DependencyProvider<P, VS>> DependencyProvid
11198
fn prioritize(&self, package: &P, range: &VS) -> Self::Priority {
11299
self.dp.prioritize(package, range)
113100
}
101+
102+
type Err = DP::Err;
114103
}
115104

116105
fn timeout_resolve<P: Package, VS: VersionSet, DP: DependencyProvider<P, VS>>(
117106
dependency_provider: DP,
118107
name: P,
119108
version: impl Into<VS::V>,
120-
) -> Result<SelectedDependencies<P, VS::V>, PubGrubError<P, VS>> {
109+
) -> Result<SelectedDependencies<P, VS::V>, PubGrubError<P, VS, DP::Err>> {
121110
resolve(
122111
&TimeoutDependencyProvider::new(dependency_provider, 50_000),
123112
name,

tests/sat_dependency_provider.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
// SPDX-License-Identifier: MPL-2.0
22

3+
use std::convert::Infallible;
4+
35
use pubgrub::error::PubGrubError;
46
use pubgrub::package::Package;
57
use pubgrub::solver::{Dependencies, DependencyProvider, OfflineDependencyProvider};
@@ -135,7 +137,7 @@ impl<P: Package, VS: VersionSet> SatResolve<P, VS> {
135137

136138
pub fn check_resolve(
137139
&mut self,
138-
res: &Result<SelectedDependencies<P, VS::V>, PubGrubError<P, VS>>,
140+
res: &Result<SelectedDependencies<P, VS::V>, PubGrubError<P, VS, Infallible>>,
139141
p: &P,
140142
v: &VS::V,
141143
) {

0 commit comments

Comments
 (0)