Skip to content

Commit ab41a3e

Browse files
prsabahramiprsabahrami
committed
nore changes
1 parent 698e88c commit ab41a3e

File tree

9 files changed

+298
-167
lines changed

9 files changed

+298
-167
lines changed

‎src/conflict.rs

+39-40
Original file line numberDiff line numberDiff line change
@@ -15,10 +15,7 @@ use crate::{
1515
internal::{
1616
arena::ArenaId,
1717
id::{ClauseId, SolvableId, SolvableOrRootId, StringId, VersionSetId},
18-
},
19-
runtime::AsyncRuntime,
20-
solver::{clause::Clause, variable_map::VariableOrigin, Solver},
21-
DependencyProvider, Interner, Requirement,
18+
}, requirement::Condition, runtime::AsyncRuntime, solver::{clause::Clause, variable_map::VariableOrigin, Solver}, DependencyProvider, Interner, Requirement
2219
};
2320

2421
/// Represents the cause of the solver being unable to find a solution
@@ -159,10 +156,9 @@ impl Conflict {
159156
ConflictEdge::Conflict(ConflictCause::Constrains(version_set_id)),
160157
);
161158
}
162-
&Clause::Conditional(
159+
Clause::Conditional(
163160
package_id,
164-
condition_variable,
165-
condition_version_set_id,
161+
condition_variables,
166162
requirement,
167163
) => {
168164
let solvable = package_id
@@ -173,7 +169,7 @@ impl Conflict {
173169
let requirement_candidates = solver
174170
.async_runtime
175171
.block_on(solver.cache.get_or_cache_sorted_candidates(
176-
requirement,
172+
*requirement,
177173
))
178174
.unwrap_or_else(|_| {
179175
unreachable!(
@@ -189,13 +185,13 @@ impl Conflict {
189185
package_node,
190186
unresolved_node,
191187
ConflictEdge::ConditionalRequires(
192-
condition_version_set_id,
193-
requirement,
188+
*requirement,
189+
condition_variables.iter().map(|(_, condition)| *condition).collect(),
194190
),
195191
);
196192
} else {
197193
tracing::trace!(
198-
"{package_id:?} conditionally requires {requirement:?} if {condition_variable:?}"
194+
"{package_id:?} conditionally requires {requirement:?} if {condition_variables:?}"
199195
);
200196

201197
for &candidate_id in requirement_candidates {
@@ -206,8 +202,8 @@ impl Conflict {
206202
package_node,
207203
candidate_node,
208204
ConflictEdge::ConditionalRequires(
209-
condition_version_set_id,
210-
requirement,
205+
*requirement,
206+
condition_variables.iter().map(|(_, condition)| *condition).collect(),
211207
),
212208
);
213209
}
@@ -292,33 +288,33 @@ impl ConflictNode {
292288
}
293289

294290
/// An edge in the graph representation of a [`Conflict`]
295-
#[derive(Clone, Copy, Hash, Eq, PartialEq, Ord, PartialOrd, Debug)]
291+
#[derive(Clone, Hash, Eq, PartialEq, Ord, PartialOrd, Debug)]
296292
pub(crate) enum ConflictEdge {
297293
/// The target node is a candidate for the dependency specified by the
298294
/// [`Requirement`]
299295
Requires(Requirement),
300296
/// The target node is involved in a conflict, caused by `ConflictCause`
301297
Conflict(ConflictCause),
302298
/// The target node is a candidate for a conditional dependency
303-
ConditionalRequires(VersionSetId, Requirement),
299+
ConditionalRequires(Requirement, Vec<Condition>),
304300
}
305301

306302
impl ConflictEdge {
307-
fn try_requires_or_conditional(self) -> Option<(Requirement, Option<VersionSetId>)> {
303+
fn try_requires_or_conditional(self) -> Option<(Requirement, Vec<Condition>)> {
308304
match self {
309-
ConflictEdge::Requires(match_spec_id) => Some((match_spec_id, None)),
310-
ConflictEdge::ConditionalRequires(version_set_id, match_spec_id) => {
311-
Some((match_spec_id, Some(version_set_id)))
305+
ConflictEdge::Requires(match_spec_id) => Some((match_spec_id, vec![])),
306+
ConflictEdge::ConditionalRequires(match_spec_id, conditions) => {
307+
Some((match_spec_id, conditions))
312308
}
313309
ConflictEdge::Conflict(_) => None,
314310
}
315311
}
316312

317-
fn requires_or_conditional(self) -> (Requirement, Option<VersionSetId>) {
313+
fn requires_or_conditional(self) -> (Requirement, Vec<Condition>) {
318314
match self {
319-
ConflictEdge::Requires(match_spec_id) => (match_spec_id, None),
320-
ConflictEdge::ConditionalRequires(version_set_id, match_spec_id) => {
321-
(match_spec_id, Some(version_set_id))
315+
ConflictEdge::Requires(match_spec_id) => (match_spec_id, vec![]),
316+
ConflictEdge::ConditionalRequires(match_spec_id, conditions) => {
317+
(match_spec_id, conditions)
322318
}
323319
ConflictEdge::Conflict(_) => panic!("expected requires edge, found conflict"),
324320
}
@@ -414,10 +410,13 @@ impl ConflictGraph {
414410
ConflictEdge::Requires(requirement) => {
415411
requirement.display(interner).to_string()
416412
}
417-
ConflictEdge::ConditionalRequires(condition_version_set_id, requirement) => {
413+
ConflictEdge::ConditionalRequires(requirement, conditions) => {
418414
format!(
419415
"if {} then {}",
420-
Requirement::from(*condition_version_set_id).display(interner),
416+
conditions.iter()
417+
.map(|c| interner.display_condition(*c).to_string())
418+
.collect::<Vec<_>>()
419+
.join(" and "),
421420
requirement.display(interner)
422421
)
423422
}
@@ -566,15 +565,15 @@ impl ConflictGraph {
566565
.graph
567566
.edges_directed(nx, Direction::Outgoing)
568567
.map(|e| match e.weight() {
569-
ConflictEdge::Requires(req) => ((req, None), e.target()),
570-
ConflictEdge::ConditionalRequires(condition, req) => {
571-
((req, Some(condition)), e.target())
568+
ConflictEdge::Requires(req) => ((req, vec![]), e.target()),
569+
ConflictEdge::ConditionalRequires(req, conditions) => {
570+
((req, conditions.clone()), e.target())
572571
}
573572
ConflictEdge::Conflict(_) => unreachable!(),
574573
})
575574
.collect::<Vec<_>>()
576575
.into_iter()
577-
.chunk_by(|((&version_set_id, condition), _)| (version_set_id, *condition));
576+
.chunk_by(|((&version_set_id, condition), _)| (version_set_id, condition.clone()));
578577

579578
for (_, mut deps) in &dependencies {
580579
if deps.all(|(_, target)| !installable.contains(&target)) {
@@ -617,15 +616,15 @@ impl ConflictGraph {
617616
.graph
618617
.edges_directed(nx, Direction::Outgoing)
619618
.map(|e| match e.weight() {
620-
ConflictEdge::Requires(version_set_id) => ((version_set_id, None), e.target()),
621-
ConflictEdge::ConditionalRequires(condition, version_set_id) => {
622-
((version_set_id, Some(condition)), e.target())
619+
ConflictEdge::Requires(version_set_id) => ((version_set_id, vec![]), e.target()),
620+
ConflictEdge::ConditionalRequires(reqs, conditions) => {
621+
((reqs, conditions.clone()), e.target())
623622
}
624623
ConflictEdge::Conflict(_) => unreachable!(),
625624
})
626625
.collect::<Vec<_>>()
627626
.into_iter()
628-
.chunk_by(|((&version_set_id, condition), _)| (version_set_id, *condition));
627+
.chunk_by(|((&version_set_id, condition), _)| (version_set_id, condition.clone()));
629628

630629
// Missing if at least one dependency is missing
631630
if dependencies
@@ -744,7 +743,7 @@ impl<'i, I: Interner> DisplayUnsat<'i, I> {
744743
top_level_indent: bool,
745744
) -> fmt::Result {
746745
pub enum DisplayOp {
747-
ConditionalRequirement((Requirement, VersionSetId), Vec<EdgeIndex>),
746+
ConditionalRequirement((Requirement, Vec<Condition>), Vec<EdgeIndex>),
748747
Requirement(Requirement, Vec<EdgeIndex>),
749748
Candidate(NodeIndex),
750749
}
@@ -758,8 +757,8 @@ impl<'i, I: Interner> DisplayUnsat<'i, I> {
758757
let indenter = Indenter::new(top_level_indent);
759758
let mut stack = top_level_edges
760759
.iter()
761-
.filter(|e| e.weight().try_requires_or_conditional().is_some())
762-
.chunk_by(|e| e.weight().requires_or_conditional())
760+
.filter(|e| e.weight().clone().try_requires_or_conditional().is_some())
761+
.chunk_by(|e| e.weight().clone().requires_or_conditional())
763762
.into_iter()
764763
.map(|(version_set_id_with_condition, group)| {
765764
let edges: Vec<_> = group.map(|e| e.id()).collect();
@@ -772,7 +771,7 @@ impl<'i, I: Interner> DisplayUnsat<'i, I> {
772771
})
773772
.map(|((version_set_id, condition), edges)| {
774773
(
775-
if let Some(condition) = condition {
774+
if !condition.is_empty() {
776775
println!("conditional requirement");
777776
DisplayOp::ConditionalRequirement((version_set_id, condition), edges)
778777
} else {
@@ -1011,7 +1010,7 @@ impl<'i, I: Interner> DisplayUnsat<'i, I> {
10111010
writeln!(f, "{indent}{version} would require",)?;
10121011
let mut requirements = graph
10131012
.edges(candidate)
1014-
.chunk_by(|e| e.weight().requires_or_conditional())
1013+
.chunk_by(|e| e.weight().clone().requires_or_conditional())
10151014
.into_iter()
10161015
.map(|(version_set_id, group)| {
10171016
let edges: Vec<_> = group.map(|e| e.id()).collect();
@@ -1025,7 +1024,7 @@ impl<'i, I: Interner> DisplayUnsat<'i, I> {
10251024
})
10261025
.map(|((version_set_id, condition), edges)| {
10271026
(
1028-
if let Some(condition) = condition {
1027+
if !condition.is_empty() {
10291028
DisplayOp::ConditionalRequirement(
10301029
(version_set_id, condition),
10311030
edges,
@@ -1054,7 +1053,7 @@ impl<'i, I: Interner> DisplayUnsat<'i, I> {
10541053
});
10551054

10561055
let req = requirement.display(self.interner).to_string();
1057-
let condition = self.interner.display_version_set(condition);
1056+
let condition = condition.iter().map(|c| self.interner.display_condition(*c).to_string()).collect::<Vec<_>>().join(" and ");
10581057

10591058
let target_nx = graph.edge_endpoints(edges[0]).unwrap().1;
10601059
let missing =

‎src/lib.rs

+4
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ pub use internal::{
2828
mapping::Mapping,
2929
};
3030
use itertools::Itertools;
31+
use requirement::Condition;
3132
pub use requirement::{ConditionalRequirement, Requirement};
3233
pub use solver::{Problem, Solver, SolverCache, UnsolvableOrCancelled};
3334

@@ -73,6 +74,9 @@ pub trait Interner {
7374
/// user-friendly way.
7475
fn display_name(&self, name: NameId) -> impl Display + '_;
7576

77+
/// Returns an object that can used to display a [`Condition`] where a condition is either a [`Extra(StringId)`] or a [`VersionSetId`]
78+
fn display_condition(&self, condition: Condition) -> impl Display + '_;
79+
7680
/// Returns an object that can be used to display the given version set in a
7781
/// user-friendly way.
7882
///

‎src/requirement.rs

+21
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,27 @@ pub enum Condition {
1111
Extra(StringId),
1212
}
1313

14+
impl From<VersionSetId> for Condition {
15+
fn from(value: VersionSetId) -> Self {
16+
Condition::VersionSetId(value)
17+
}
18+
}
19+
20+
impl From<StringId> for Condition {
21+
fn from(value: StringId) -> Self {
22+
Condition::Extra(value)
23+
}
24+
}
25+
26+
impl From<Condition> for VersionSetId {
27+
fn from(value: Condition) -> Self {
28+
match value {
29+
Condition::VersionSetId(id) => id,
30+
Condition::Extra(_) => panic!("Cannot convert Extra to VersionSetId"),
31+
}
32+
}
33+
}
34+
1435
/// Specifies a conditional requirement, where the requirement is only active when the condition is met.
1536
#[derive(Clone, Debug, Hash, Eq, PartialEq, Ord, PartialOrd)]
1637
#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]

‎src/snapshot.rs

+22-7
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,7 @@ use ahash::HashSet;
1515
use futures::FutureExt;
1616

1717
use crate::{
18-
internal::arena::ArenaId, Candidates, Dependencies, DependencyProvider, Interner, Mapping,
19-
NameId, Requirement, SolvableId, SolverCache, StringId, VersionSetId, VersionSetUnionId,
18+
internal::arena::ArenaId, requirement::Condition, Candidates, Dependencies, DependencyProvider, Interner, Mapping, NameId, Requirement, SolvableId, SolverCache, StringId, VersionSetId, VersionSetUnionId
2019
};
2120

2221
/// A single solvable in a [`DependencySnapshot`].
@@ -220,12 +219,21 @@ impl DependencySnapshot {
220219
}
221220
}
222221

223-
for &req in deps.requirements.iter() {
224-
let (condition, requirement) = req.into_condition_and_requirement();
222+
for req in deps.requirements.iter() {
223+
let (conditions, requirement) = req.clone().into_condition_and_requirement();
225224

226-
if let Some(condition) = condition {
227-
if seen.insert(Element::VersionSet(condition)) {
228-
queue.push_back(Element::VersionSet(condition));
225+
for condition in conditions {
226+
match condition {
227+
Condition::Extra(string_id) => {
228+
if seen.insert(Element::String(string_id)) {
229+
queue.push_back(Element::String(string_id));
230+
}
231+
}
232+
Condition::VersionSetId(version_set_id) => {
233+
if seen.insert(Element::VersionSet(version_set_id)) {
234+
queue.push_back(Element::VersionSet(version_set_id));
235+
}
236+
}
229237
}
230238
}
231239

@@ -437,6 +445,13 @@ impl<'s> Interner for SnapshotProvider<'s> {
437445
self.string(string_id)
438446
}
439447

448+
fn display_condition(&self, condition: Condition) -> impl Display + '_ {
449+
match condition {
450+
Condition::Extra(string_id) => format!("{}", self.display_string(string_id)),
451+
Condition::VersionSetId(version_set_id) => format!("{} {}", self.display_name(self.version_set_name(version_set_id)), self.display_version_set(version_set_id)),
452+
}
453+
}
454+
440455
fn version_set_name(&self, version_set: VersionSetId) -> NameId {
441456
self.version_set(version_set).name
442457
}

‎src/solver/clause.rs

+9-9
Original file line numberDiff line numberDiff line change
@@ -258,7 +258,7 @@ impl Clause {
258258
};
259259

260260
(
261-
Clause::Conditional(parent_id, condition_variables, requirement),
261+
Clause::Conditional(parent_id, condition_variables.clone(), requirement),
262262
Some([
263263
parent_id.negative(),
264264
requirement_literal.unwrap_or(condition_variables.first().unwrap().0.negative()),
@@ -288,10 +288,10 @@ impl Clause {
288288
where
289289
F: FnMut(C, Literal) -> ControlFlow<B, C>,
290290
{
291-
match *self {
291+
match self {
292292
Clause::InstallRoot => unreachable!(),
293293
Clause::Excluded(solvable, _) => visit(init, solvable.negative()),
294-
Clause::Learnt(learnt_id) => learnt_clauses[learnt_id]
294+
Clause::Learnt(learnt_id) => learnt_clauses[*learnt_id]
295295
.iter()
296296
.copied()
297297
.try_fold(init, visit),
@@ -307,7 +307,7 @@ impl Clause {
307307
.into_iter()
308308
.try_fold(init, visit),
309309
Clause::ForbidMultipleInstances(s1, s2, _) => {
310-
[s1.negative(), s2].into_iter().try_fold(init, visit)
310+
[s1.negative(), *s2].into_iter().try_fold(init, visit)
311311
}
312312
Clause::Lock(_, s) => [s.negative(), VariableId::root().negative()]
313313
.into_iter()
@@ -357,7 +357,7 @@ impl Clause {
357357
interner: &'i I,
358358
) -> ClauseDisplay<'i, I> {
359359
ClauseDisplay {
360-
kind: *self,
360+
kind: self.clone(),
361361
variable_map,
362362
interner,
363363
}
@@ -636,15 +636,15 @@ pub(crate) struct ClauseDisplay<'i, I: Interner> {
636636

637637
impl<'i, I: Interner> Display for ClauseDisplay<'i, I> {
638638
fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
639-
match self.kind {
639+
match &self.kind {
640640
Clause::InstallRoot => write!(f, "InstallRoot"),
641641
Clause::Excluded(variable, reason) => {
642642
write!(
643643
f,
644644
"Excluded({}({:?}), {})",
645645
variable.display(self.variable_map, self.interner),
646646
variable,
647-
self.interner.display_string(reason)
647+
self.interner.display_string(*reason)
648648
)
649649
}
650650
Clause::Learnt(learnt_id) => write!(f, "Learnt({learnt_id:?})"),
@@ -665,7 +665,7 @@ impl<'i, I: Interner> Display for ClauseDisplay<'i, I> {
665665
v1,
666666
v2.display(self.variable_map, self.interner),
667667
v2,
668-
self.interner.display_version_set(version_set_id)
668+
self.interner.display_version_set(*version_set_id)
669669
)
670670
}
671671
Clause::ForbidMultipleInstances(v1, v2, name) => {
@@ -676,7 +676,7 @@ impl<'i, I: Interner> Display for ClauseDisplay<'i, I> {
676676
v1,
677677
v2.variable().display(self.variable_map, self.interner),
678678
v2,
679-
self.interner.display_name(name)
679+
self.interner.display_name(*name)
680680
)
681681
}
682682
Clause::Lock(locked, other) => {

0 commit comments

Comments
 (0)