Skip to content
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.

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

feat!: allocation free errors #168

Merged
merged 1 commit into from
Dec 16, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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: 6 additions & 9 deletions examples/caching_dependency_provider.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
// SPDX-License-Identifier: MPL-2.0

use std::cell::RefCell;
use std::error::Error;

use pubgrub::package::Package;
use pubgrub::range::Range;
Expand Down Expand Up @@ -37,7 +36,7 @@ impl<P: Package, VS: VersionSet, DP: DependencyProvider<P, VS>> DependencyProvid
&self,
package: &P,
version: &VS::V,
) -> Result<Dependencies<P, VS>, Box<dyn Error + Send + Sync>> {
) -> Result<Dependencies<P, VS>, DP::Err> {
let mut cache = self.cached_dependencies.borrow_mut();
match cache.get_dependencies(package, version) {
Ok(Dependencies::Unknown) => {
Expand All @@ -55,16 +54,12 @@ impl<P: Package, VS: VersionSet, DP: DependencyProvider<P, VS>> DependencyProvid
error @ Err(_) => error,
}
}
dependencies @ Ok(_) => dependencies,
error @ Err(_) => error,
Ok(dependencies) => Ok(dependencies),
Err(_) => unreachable!(),
}
}

fn choose_version(
&self,
package: &P,
range: &VS,
) -> Result<Option<VS::V>, Box<dyn Error + Send + Sync>> {
fn choose_version(&self, package: &P, range: &VS) -> Result<Option<VS::V>, DP::Err> {
self.remote_dependencies.choose_version(package, range)
}

Expand All @@ -73,6 +68,8 @@ impl<P: Package, VS: VersionSet, DP: DependencyProvider<P, VS>> DependencyProvid
fn prioritize(&self, package: &P, range: &VS) -> Self::Priority {
self.remote_dependencies.prioritize(package, range)
}

type Err = DP::Err;
}

fn main() {
Expand Down
8 changes: 4 additions & 4 deletions src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use crate::version_set::VersionSet;

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

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

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

/// Something unexpected happened.
#[error("{0}")]
Expand Down
8 changes: 5 additions & 3 deletions src/internal/core.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
//! to write a functional PubGrub algorithm.

use std::collections::HashSet as Set;
use std::error::Error;

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

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

/// Return the root cause and the backtracked model.
/// CF <https://github.com/dart-lang/pub/blob/master/doc/solver.md#unit-propagation>
fn conflict_resolution(
#[allow(clippy::type_complexity)]
fn conflict_resolution<E: Error>(
&mut self,
incompatibility: IncompId<P, VS>,
) -> Result<(P, IncompId<P, VS>), PubGrubError<P, VS>> {
) -> Result<(P, IncompId<P, VS>), PubGrubError<P, VS, E>> {
let mut current_incompat_id = incompatibility;
let mut current_incompat_changed = false;
loop {
Expand Down
7 changes: 5 additions & 2 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -83,13 +83,14 @@
//! # use pubgrub::type_aliases::Map;
//! # use std::error::Error;
//! # use std::borrow::Borrow;
//! # use std::convert::Infallible;
//! #
//! # struct MyDependencyProvider;
//! #
//! type SemVS = Range<SemanticVersion>;
//!
//! impl DependencyProvider<String, SemVS> for MyDependencyProvider {
//! fn choose_version(&self, package: &String, range: &SemVS) -> Result<Option<SemanticVersion>, Box<dyn Error + Send + Sync>> {
//! fn choose_version(&self, package: &String, range: &SemVS) -> Result<Option<SemanticVersion>, Infallible> {
//! unimplemented!()
//! }
//!
Expand All @@ -102,9 +103,11 @@
//! &self,
//! package: &String,
//! version: &SemanticVersion,
//! ) -> Result<Dependencies<String, SemVS>, Box<dyn Error + Send + Sync>> {
//! ) -> Result<Dependencies<String, SemVS>, Infallible> {
//! unimplemented!()
//! }
//!
//! type Err = Infallible;
//! }
//! ```
//!
Expand Down
38 changes: 20 additions & 18 deletions src/solver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,14 +41,15 @@
//! ## API
//!
//! ```
//! # use std::convert::Infallible;
//! # use pubgrub::solver::{resolve, OfflineDependencyProvider};
//! # use pubgrub::version::NumberVersion;
//! # use pubgrub::error::PubGrubError;
//! # use pubgrub::range::Range;
//! #
//! # type NumVS = Range<NumberVersion>;
//! #
//! # fn try_main() -> Result<(), PubGrubError<&'static str, NumVS>> {
//! # fn try_main() -> Result<(), PubGrubError<&'static str, NumVS, Infallible>> {
//! # let dependency_provider = OfflineDependencyProvider::<&str, NumVS>::new();
//! # let package = "root";
//! # let version = 1;
Expand All @@ -70,6 +71,7 @@

use std::cmp::Reverse;
use std::collections::{BTreeMap, BTreeSet as Set};
use std::convert::Infallible;
use std::error::Error;

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

/// Main function of the library.
/// Finds a set of packages satisfying dependency bounds for a given package + version pair.
pub fn resolve<P: Package, VS: VersionSet>(
dependency_provider: &impl DependencyProvider<P, VS>,
#[allow(clippy::type_complexity)]
pub fn resolve<P: Package, VS: VersionSet, DP: DependencyProvider<P, VS>>(
dependency_provider: &DP,
package: P,
version: impl Into<VS::V>,
) -> Result<SelectedDependencies<P, VS::V>, PubGrubError<P, VS>> {
) -> Result<SelectedDependencies<P, VS::V>, PubGrubError<P, VS, DP::Err>> {
let mut state = State::init(package.clone(), version.into());
let mut added_dependencies: Map<P, Set<VS::V>> = Map::default();
let mut next = package;
Expand Down Expand Up @@ -133,7 +136,7 @@ pub fn resolve<P: Package, VS: VersionSet>(
};

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

/// The kind of error returned from these methods.
///
/// Returning this signals that resolution should fail with this error.
type Err: Error;

/// Once the resolver has found the highest `Priority` package from all potential valid
/// packages, it needs to know what vertion of that package to use. The most common pattern
/// is to select the largest vertion that the range contains.
fn choose_version(
&self,
package: &P,
range: &VS,
) -> Result<Option<VS::V>, Box<dyn Error + Send + Sync>>;
fn choose_version(&self, package: &P, range: &VS) -> Result<Option<VS::V>, Self::Err>;

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

/// This is called fairly regularly during the resolution,
/// if it returns an Err then resolution will be terminated.
/// This is helpful if you want to add some form of early termination like a timeout,
/// or you want to add some form of user feedback if things are taking a while.
/// If not provided the resolver will run as long as needed.
fn should_cancel(&self) -> Result<(), Box<dyn Error + Send + Sync>> {
fn should_cancel(&self) -> Result<(), Self::Err> {
Ok(())
}
}
Expand Down Expand Up @@ -340,11 +344,9 @@ impl<P: Package, VS: VersionSet> OfflineDependencyProvider<P, VS> {
/// But, that may change in new versions if better heuristics are found.
/// Versions are picked with the newest versions first.
impl<P: Package, VS: VersionSet> DependencyProvider<P, VS> for OfflineDependencyProvider<P, VS> {
fn choose_version(
&self,
package: &P,
range: &VS,
) -> Result<Option<VS::V>, Box<dyn Error + Send + Sync>> {
type Err = Infallible;

fn choose_version(&self, package: &P, range: &VS) -> Result<Option<VS::V>, Infallible> {
Ok(self
.dependencies
.get(package)
Expand All @@ -365,7 +367,7 @@ impl<P: Package, VS: VersionSet> DependencyProvider<P, VS> for OfflineDependency
&self,
package: &P,
version: &VS::V,
) -> Result<Dependencies<P, VS>, Box<dyn Error + Send + Sync>> {
) -> Result<Dependencies<P, VS>, Infallible> {
Ok(match self.dependencies(package, version) {
None => Dependencies::Unknown,
Some(dependencies) => Dependencies::Known(dependencies),
Expand Down
35 changes: 12 additions & 23 deletions tests/proptest.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// SPDX-License-Identifier: MPL-2.0

use std::{collections::BTreeSet as Set, error::Error};
use std::collections::BTreeSet as Set;
use std::convert::Infallible;

use pubgrub::error::PubGrubError;
use pubgrub::package::Package;
Expand Down Expand Up @@ -30,19 +31,11 @@ struct OldestVersionsDependencyProvider<P: Package, VS: VersionSet>(
impl<P: Package, VS: VersionSet> DependencyProvider<P, VS>
for OldestVersionsDependencyProvider<P, VS>
{
fn get_dependencies(
&self,
p: &P,
v: &VS::V,
) -> Result<Dependencies<P, VS>, Box<dyn Error + Send + Sync>> {
fn get_dependencies(&self, p: &P, v: &VS::V) -> Result<Dependencies<P, VS>, Infallible> {
self.0.get_dependencies(p, v)
}

fn choose_version(
&self,
package: &P,
range: &VS,
) -> Result<Option<VS::V>, Box<dyn Error + Send + Sync>> {
fn choose_version(&self, package: &P, range: &VS) -> Result<Option<VS::V>, Infallible> {
Ok(self
.0
.versions(package)
Expand All @@ -57,6 +50,8 @@ impl<P: Package, VS: VersionSet> DependencyProvider<P, VS>
fn prioritize(&self, package: &P, range: &VS) -> Self::Priority {
self.0.prioritize(package, range)
}

type Err = Infallible;
}

/// The same as DP but it has a timeout.
Expand All @@ -82,27 +77,19 @@ impl<DP> TimeoutDependencyProvider<DP> {
impl<P: Package, VS: VersionSet, DP: DependencyProvider<P, VS>> DependencyProvider<P, VS>
for TimeoutDependencyProvider<DP>
{
fn get_dependencies(
&self,
p: &P,
v: &VS::V,
) -> Result<Dependencies<P, VS>, Box<dyn Error + Send + Sync>> {
fn get_dependencies(&self, p: &P, v: &VS::V) -> Result<Dependencies<P, VS>, DP::Err> {
self.dp.get_dependencies(p, v)
}

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

fn choose_version(
&self,
package: &P,
range: &VS,
) -> Result<Option<VS::V>, Box<dyn Error + Send + Sync>> {
fn choose_version(&self, package: &P, range: &VS) -> Result<Option<VS::V>, DP::Err> {
self.dp.choose_version(package, range)
}

Expand All @@ -111,13 +98,15 @@ impl<P: Package, VS: VersionSet, DP: DependencyProvider<P, VS>> DependencyProvid
fn prioritize(&self, package: &P, range: &VS) -> Self::Priority {
self.dp.prioritize(package, range)
}

type Err = DP::Err;
}

fn timeout_resolve<P: Package, VS: VersionSet, DP: DependencyProvider<P, VS>>(
dependency_provider: DP,
name: P,
version: impl Into<VS::V>,
) -> Result<SelectedDependencies<P, VS::V>, PubGrubError<P, VS>> {
) -> Result<SelectedDependencies<P, VS::V>, PubGrubError<P, VS, DP::Err>> {
resolve(
&TimeoutDependencyProvider::new(dependency_provider, 50_000),
name,
Expand Down
4 changes: 3 additions & 1 deletion tests/sat_dependency_provider.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
// SPDX-License-Identifier: MPL-2.0

use std::convert::Infallible;

use pubgrub::error::PubGrubError;
use pubgrub::package::Package;
use pubgrub::solver::{Dependencies, DependencyProvider, OfflineDependencyProvider};
Expand Down Expand Up @@ -135,7 +137,7 @@ impl<P: Package, VS: VersionSet> SatResolve<P, VS> {

pub fn check_resolve(
&mut self,
res: &Result<SelectedDependencies<P, VS::V>, PubGrubError<P, VS>>,
res: &Result<SelectedDependencies<P, VS::V>, PubGrubError<P, VS, Infallible>>,
p: &P,
v: &VS::V,
) {
Expand Down