refactor: remove learnt_clause_start

crates/rattler_libsolv_rs/src/arena.rs

@@ -1,7 +1,7 @@
 use std::cell::{Cell, UnsafeCell};
 use std::cmp;
 use std::marker::PhantomData;
-use std::ops::Index;
+use std::ops::{Index, IndexMut};
 
 const CHUNK_SIZE: usize = 128;
 
@@ -77,11 +77,53 @@ impl<TId: ArenaId, TValue> Arena<TId, TValue> {
         TId::from_usize(id)
     }
 
+    /// Returns an iterator over the elements of the arena.
+    pub fn iter(&self) -> ArenaIter<TId, TValue> {
+        ArenaIter {
+            arena: self,
+            index: 0,
+        }
+    }
+
+    /// Returns an mutable iterator over the elements of the arena.
+    pub fn iter_mut(&mut self) -> ArenaIterMut<TId, TValue> {
+        ArenaIterMut {
+            arena: self,
+            index: 0,
+        }
+    }
+
     fn chunk_and_offset(index: usize) -> (usize, usize) {
         let offset = index % CHUNK_SIZE;
         let chunk = index / CHUNK_SIZE;
         (chunk, offset)
     }
+
+    /// Returns mutable references to the two values references by the two distinct indices.
+    ///
+    /// Panics if one of the Ids is invalid or when the two ids are the same.
+    pub fn get_two_mut(&mut self, a: TId, b: TId) -> (&mut TValue, &mut TValue) {
+        let a_index = a.to_usize();
+        let b_index = b.to_usize();
+        assert!(a_index < self.len());
+        assert!(b_index < self.len());
+        assert_ne!(a_index, b_index);
+        let (a_chunk, a_offset) = Self::chunk_and_offset(a_index);
+        let (b_chunk, b_offset) = Self::chunk_and_offset(b_index);
+        // SAFE: because we check that the indices are less than the length and that both indices do
+        // not refer to the same item.
+        unsafe {
+            let chunks = self.chunks.get();
+            (
+                (*chunks)
+                    .get_unchecked_mut(a_chunk)
+                    .get_unchecked_mut(a_offset),
+                (*chunks)
+                    .get_unchecked_mut(b_chunk)
+                    .get_unchecked_mut(b_offset),
+            )
+        }
+    }
 }
 
 impl<TId: ArenaId, TValue> Index<TId> for Arena<TId, TValue> {
@@ -98,8 +140,78 @@ impl<TId: ArenaId, TValue> Index<TId> for Arena<TId, TValue> {
     }
 }
 
+impl<TId: ArenaId, TValue> IndexMut<TId> for Arena<TId, TValue> {
+    fn index_mut(&mut self, index: TId) -> &mut Self::Output {
+        let index = index.to_usize();
+        assert!(index < self.len());
+        let (chunk, offset) = Self::chunk_and_offset(index);
+        // SAFE: because we check that the index is less than the length
+        unsafe {
+            self.chunks
+                .get_mut()
+                .get_unchecked_mut(chunk)
+                .get_unchecked_mut(offset)
+        }
+    }
+}
+
 /// A trait indicating that the type can be transformed to `usize` and back
 pub trait ArenaId {
     fn from_usize(x: usize) -> Self;
     fn to_usize(self) -> usize;
 }
+
+/// An iterator over the elements of an [`Arena`].
+pub struct ArenaIter<'a, TId: ArenaId, TValue> {
+    arena: &'a Arena<TId, TValue>,
+    index: usize,
+}
+
+impl<'a, TId: ArenaId, TValue> Iterator for ArenaIter<'a, TId, TValue> {
+    type Item = (TId, &'a TValue);
+
+    fn next(&mut self) -> Option<Self::Item> {
+        if self.index < self.arena.len.get() {
+            let (chunk, offset) = Arena::<TId, TValue>::chunk_and_offset(self.index);
+            let element = unsafe {
+                let vec = self.arena.chunks.get();
+                Some((
+                    TId::from_usize(self.index),
+                    (*vec).get_unchecked(chunk).get_unchecked(offset),
+                ))
+            };
+
+            self.index += 1;
+            element
+        } else {
+            None
+        }
+    }
+}
+
+/// An mutable iterator over the elements of an [`Arena`].
+pub struct ArenaIterMut<'a, TId: ArenaId, TValue> {
+    arena: &'a mut Arena<TId, TValue>,
+    index: usize,
+}
+
+impl<'a, TId: ArenaId, TValue> Iterator for ArenaIterMut<'a, TId, TValue> {
+    type Item = (TId, &'a mut TValue);
+
+    fn next(&mut self) -> Option<Self::Item> {
+        if self.index < self.arena.len.get() {
+            let (chunk, offset) = Arena::<TId, TValue>::chunk_and_offset(self.index);
+            let element = unsafe {
+                let vec = self.arena.chunks.get();
+                Some((
+                    TId::from_usize(self.index),
+                    (*vec).get_unchecked_mut(chunk).get_unchecked_mut(offset),
+                ))
+            };
+            self.index += 1;
+            element
+        } else {
+            None
+        }
+    }
+}

crates/rattler_libsolv_rs/src/id.rs

@@ -74,21 +74,12 @@ impl From<SolvableId> for u32 {
 pub(crate) struct ClauseId(u32);
 
 impl ClauseId {
-    pub(crate) fn new(index: usize) -> Self {
-        debug_assert_ne!(index, 0);
-        debug_assert_ne!(index, u32::MAX as usize);
-
-        Self(index as u32)
-    }
-
+    /// There is a guarentee that ClauseId(0) will always be "Clause::InstallRoot". This assumption
+    /// is verified by the solver.
     pub(crate) fn install_root() -> Self {
         Self(0)
     }
 
-    pub(crate) fn index(self) -> usize {
-        self.0 as usize
-    }
-
     pub(crate) fn is_null(self) -> bool {
         self.0 == u32::MAX
     }
@@ -98,6 +89,17 @@ impl ClauseId {
     }
 }
 
+impl ArenaId for ClauseId {
+    fn from_usize(x: usize) -> Self {
+        assert!(x < u32::MAX as usize, "clause id too big");
+        Self(x as u32)
+    }
+
+    fn to_usize(self) -> usize {
+        self.0 as usize
+    }
+}
+
 #[derive(Copy, Clone, Debug)]
 pub(crate) struct LearntClauseId(u32);
 

crates/rattler_libsolv_rs/src/problem.rs

@@ -50,11 +50,11 @@ impl Problem {
         let unresolved_node = graph.add_node(ProblemNode::UnresolvedDependency);
 
         for clause_id in &self.clauses {
-            let clause = solver.clauses[clause_id.index()].kind;
+            let clause = &solver.clauses[*clause_id].kind;
             match clause {
                 Clause::InstallRoot => (),
                 Clause::Learnt(..) => unreachable!(),
-                Clause::Requires(package_id, version_set_id) => {
+                &Clause::Requires(package_id, version_set_id) => {
                     let package_node = Self::add_node(&mut graph, &mut nodes, package_id);
 
                     let candidates = solver.get_or_cache_sorted_candidates(version_set_id);
@@ -81,19 +81,19 @@ impl Problem {
                         }
                     }
                 }
-                Clause::Lock(locked, forbidden) => {
+                &Clause::Lock(locked, forbidden) => {
                     let node2_id = Self::add_node(&mut graph, &mut nodes, forbidden);
                     let conflict = ConflictCause::Locked(locked);
                     graph.add_edge(root_node, node2_id, ProblemEdge::Conflict(conflict));
                 }
-                Clause::ForbidMultipleInstances(instance1_id, instance2_id) => {
+                &Clause::ForbidMultipleInstances(instance1_id, instance2_id) => {
                     let node1_id = Self::add_node(&mut graph, &mut nodes, instance1_id);
                     let node2_id = Self::add_node(&mut graph, &mut nodes, instance2_id);
 
                     let conflict = ConflictCause::ForbidMultipleInstances;
                     graph.add_edge(node1_id, node2_id, ProblemEdge::Conflict(conflict));
                 }
-                Clause::Constrains(package_id, dep_id, version_set_id) => {
+                &Clause::Constrains(package_id, dep_id, version_set_id) => {
                     let package_node = Self::add_node(&mut graph, &mut nodes, package_id);
                     let dep_node = Self::add_node(&mut graph, &mut nodes, dep_id);
 

crates/rattler_libsolv_rs/src/solver/clause.rs

@@ -584,11 +584,11 @@ mod test {
     #[test]
     fn test_unlink_clause_different() {
         let clause1 = clause(
-            [ClauseId::new(2), ClauseId::new(3)],
+            [ClauseId::from_usize(2), ClauseId::from_usize(3)],
             [SolvableId::from_usize(1596), SolvableId::from_usize(1211)],
         );
         let clause2 = clause(
-            [ClauseId::null(), ClauseId::new(3)],
+            [ClauseId::null(), ClauseId::from_usize(3)],
             [SolvableId::from_usize(1596), SolvableId::from_usize(1208)],
         );
         let clause3 = clause(
@@ -604,7 +604,10 @@ mod test {
                 clause1.watched_literals,
                 [SolvableId::from_usize(1596), SolvableId::from_usize(1211)]
             );
-            assert_eq!(clause1.next_watches, [ClauseId::null(), ClauseId::new(3)])
+            assert_eq!(
+                clause1.next_watches,
+                [ClauseId::null(), ClauseId::from_usize(3)]
+            )
         }
 
         // Unlink 1
@@ -615,14 +618,17 @@ mod test {
                 clause1.watched_literals,
                 [SolvableId::from_usize(1596), SolvableId::from_usize(1211)]
             );
-            assert_eq!(clause1.next_watches, [ClauseId::new(2), ClauseId::null()])
+            assert_eq!(
+                clause1.next_watches,
+                [ClauseId::from_usize(2), ClauseId::null()]
+            )
         }
     }
 
     #[test]
     fn test_unlink_clause_same() {
         let clause1 = clause(
-            [ClauseId::new(2), ClauseId::new(2)],
+            [ClauseId::from_usize(2), ClauseId::from_usize(2)],
             [SolvableId::from_usize(1596), SolvableId::from_usize(1211)],
         );
         let clause2 = clause(
@@ -638,7 +644,10 @@ mod test {
                 clause1.watched_literals,
                 [SolvableId::from_usize(1596), SolvableId::from_usize(1211)]
             );
-            assert_eq!(clause1.next_watches, [ClauseId::null(), ClauseId::new(2)])
+            assert_eq!(
+                clause1.next_watches,
+                [ClauseId::null(), ClauseId::from_usize(2)]
+            )
         }
 
         // Unlink 1
@@ -649,7 +658,10 @@ mod test {
                 clause1.watched_literals,
                 [SolvableId::from_usize(1596), SolvableId::from_usize(1211)]
             );
-            assert_eq!(clause1.next_watches, [ClauseId::new(2), ClauseId::null()])
+            assert_eq!(
+                clause1.next_watches,
+                [ClauseId::from_usize(2), ClauseId::null()]
+            )
         }
     }