Skip to content

Commit

Permalink
Address review comments
Browse files Browse the repository at this point in the history
Signed-off-by: Julien Jerphanion <git@jjerphan.xyz>

Co-authored-by: Bas Zalmstra <bas@prefix.dev>
  • Loading branch information
jjerphan and baszalmstra committed Jul 18, 2024
1 parent a08a3ad commit a37a65a
Show file tree
Hide file tree
Showing 3 changed files with 6 additions and 31 deletions.
6 changes: 0 additions & 6 deletions src/internal/id.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,12 +60,6 @@ impl ArenaId for VersionSetId {
#[cfg_attr(feature = "serde", serde(transparent))]
pub struct SolvableId(pub u32);

impl SolvableId {
pub(crate) const fn as_internal(self) -> InternalSolvableId {
InternalSolvableId(self.0 + 1)
}
}

/// Internally used id for solvables that can also represent root and null.
#[repr(transparent)]
#[derive(Copy, Clone, Eq, PartialEq, Hash, Debug, Ord, PartialOrd)]
Expand Down
4 changes: 2 additions & 2 deletions src/solver/cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -174,8 +174,8 @@ impl<D: DependencyProvider> SolverCache<D> {
let package_name_id = self.provider.version_set_name(version_set_id);

tracing::trace!(
"Getting matching candidates for package: {:?}",
self.provider.display_name(package_name_id).to_string()
"Getting matching candidates for package: {}",
self.provider.display_name(package_name_id)
);

let candidates = self.get_or_cache_candidates(package_name_id).await?;
Expand Down
27 changes: 4 additions & 23 deletions src/solver/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,10 @@ impl<D: DependencyProvider, RT: AsyncRuntime> Solver<D, RT> {

tracing::trace!("Solvables found:");
for step in &steps {
tracing::trace!(" - {}", step.as_internal().display(self.provider()));
tracing::trace!(
" - {}",
InternalSolvableId::from(step.clone()).display(self.provider())

Check failure on line 204 in src/solver/mod.rs

View workflow job for this annotation

GitHub Actions / Format and Lint

using `clone` on type `SolvableId` which implements the `Copy` trait
);
}

Ok(steps)
Expand Down Expand Up @@ -987,37 +990,20 @@ impl<D: DependencyProvider, RT: AsyncRuntime> Solver<D, RT> {
/// solvable has become false, in which case it picks a new solvable to
/// watch (if available) or triggers an assignment.
fn propagate(&mut self, level: u32) -> Result<(), PropagationError> {
tracing::trace!("Propagate");

if let Some(value) = self.provider().should_cancel_with_value() {
return Err(PropagationError::Cancelled(value));
};

// Negative assertions derived from other rules (assertions are clauses that
// consist of a single literal, and therefore do not have watches)
for &(solvable_id, clause_id) in &self.negative_assertions {
tracing::trace!(
"Negative assertions derived from other rules: {} = false",
solvable_id.display(self.provider())
);

let value = false;
let decided = self
.decision_tracker
.try_add_decision(Decision::new(solvable_id, value, clause_id), level)
.map_err(|_| PropagationError::Conflict(solvable_id, value, clause_id))?;

tracing::trace!(
"Negative assertions derived from other rules: Decided: {}",
decided
);

if decided {
tracing::trace!(
"├─ Propagate assertion {} = {}",
solvable_id.display(self.provider()),
value
);
tracing::trace!(
"Negative assertions derived from other rules: Propagate assertion {} = {}",
solvable_id.display(self.provider()),
Expand Down Expand Up @@ -1067,11 +1053,6 @@ impl<D: DependencyProvider, RT: AsyncRuntime> Solver<D, RT> {
while let Some(decision) = self.decision_tracker.next_unpropagated() {
let pkg = decision.solvable_id;

tracing::trace!(
"Exploring watched solvables for {}",
pkg.display(self.provider())
);

// Propagate, iterating through the linked list of clauses that watch this
// solvable
let mut old_predecessor_clause_id: Option<ClauseId>;
Expand Down

0 comments on commit a37a65a

Please sign in to comment.