cleanup and comments

compiler/types/src/subs.rs

@@ -58,27 +58,24 @@ pub struct Subs {
     pub record_fields: Vec<RecordField<()>>,
 }
 
+/// A slice into the Vec<T> of subs
+///
+/// The starting position is a u32 which should be plenty
+/// We limit slices to u16::MAX = 65535 elements
 pub struct SubsSlice<T> {
     start: u32,
     length: u16,
     _marker: std::marker::PhantomData<T>,
 }
 
+/// An index into the Vec<T> of subs
 pub struct SubsIndex<T> {
     start: u32,
     _marker: std::marker::PhantomData<T>,
 }
 
-impl<T> std::fmt::Debug for SubsIndex<T> {
-    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        write!(
-            f,
-            "SubsIndex<{}>({})",
-            std::any::type_name::<T>(),
-            self.start
-        )
-    }
-}
+// make `subs[some_index]` work. The types/trait resolution make sure we get the
+// element from the right vector
 
 impl std::ops::Index<SubsIndex<Variable>> for Subs {
     type Output = Variable;
@@ -122,6 +119,19 @@ impl std::ops::IndexMut<SubsIndex<RecordField<()>>> for Subs {
     }
 }
 
+// custom debug
+
+impl<T> std::fmt::Debug for SubsIndex<T> {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        write!(
+            f,
+            "SubsIndex<{}>({})",
+            std::any::type_name::<T>(),
+            self.start
+        )
+    }
+}
+
 impl<T> std::fmt::Debug for SubsSlice<T> {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         write!(
@@ -132,6 +142,8 @@ impl<T> std::fmt::Debug for SubsSlice<T> {
     }
 }
 
+// derive of copy and clone does not play well with PhantomData
+
 impl<T> Copy for SubsIndex<T> {}
 
 impl<T> Clone for SubsIndex<T> {
@@ -220,41 +232,24 @@ fn u32_to_index<T>(i: u32) -> SubsIndex<T> {
 
 pub trait GetSubsSlice<T> {
     fn get_subs_slice(&self, subs_slice: SubsSlice<T>) -> &[T];
-
-    fn get_subs_slice_mut(&mut self, subs_slice: SubsSlice<T>) -> &mut [T];
 }
 
 impl GetSubsSlice<Variable> for Subs {
     fn get_subs_slice(&self, subs_slice: SubsSlice<Variable>) -> &[Variable] {
         subs_slice.get_slice(&self.variables)
     }
-
-    fn get_subs_slice_mut(&mut self, subs_slice: SubsSlice<Variable>) -> &mut [Variable] {
-        subs_slice.get_slice_mut(&mut self.variables)
-    }
 }
 
 impl GetSubsSlice<RecordField<()>> for Subs {
     fn get_subs_slice(&self, subs_slice: SubsSlice<RecordField<()>>) -> &[RecordField<()>] {
         subs_slice.get_slice(&self.record_fields)
     }
-
-    fn get_subs_slice_mut(
-        &mut self,
-        subs_slice: SubsSlice<RecordField<()>>,
-    ) -> &mut [RecordField<()>] {
-        subs_slice.get_slice_mut(&mut self.record_fields)
-    }
 }
 
 impl GetSubsSlice<Lowercase> for Subs {
     fn get_subs_slice(&self, subs_slice: SubsSlice<Lowercase>) -> &[Lowercase] {
         subs_slice.get_slice(&self.field_names)
     }
-
-    fn get_subs_slice_mut(&mut self, subs_slice: SubsSlice<Lowercase>) -> &mut [Lowercase] {
-        subs_slice.get_slice_mut(&mut self.field_names)
-    }
 }
 
 impl fmt::Debug for Subs {
@@ -932,13 +927,20 @@ impl RecordFields {
         }
     }
 
+    /// Get a sorted iterator over the fields of this record type
+    ///
+    /// Implementation: When the record has an `ext` variable that is the empty record, then
+    /// we read the (assumed sorted) fields directly from Subs. Otherwise we have to chase the
+    /// ext var, then sort the fields.
+    ///
+    /// Hopefully the inline will get rid of the Box in practice
     #[inline(always)]
     pub fn sorted_iterator<'a>(&'_ self, subs: &'a Subs, ext: Variable) -> SortedIterator<'a> {
-        self.sorted_iterator_help(subs, ext).0
+        self.sorted_iterator_and_ext(subs, ext).0
     }
 
     #[inline(always)]
-    pub fn sorted_iterator_help<'a>(
+    pub fn sorted_iterator_and_ext<'a>(
         &'_ self,
         subs: &'a Subs,
         ext: Variable,
@@ -1011,12 +1013,6 @@ fn is_empty_record(subs: &Subs, mut var: Variable) -> bool {
     }
 }
 
-pub struct SeparateRecordFields {
-    pub only_in_1: Vec<(Lowercase, RecordField<Variable>)>,
-    pub only_in_2: Vec<(Lowercase, RecordField<Variable>)>,
-    pub in_both: Vec<(Lowercase, RecordField<Variable>, RecordField<Variable>)>,
-}
-
 fn occurs(
     subs: &Subs,
     seen: &ImSet<Variable>,

compiler/unify/src/unify.rs

@@ -479,8 +479,8 @@ fn separate_record_fields(
     Variable,
     Variable,
 ) {
-    let (it1, new_ext1) = fields1.sorted_iterator_help(subs, ext1);
-    let (it2, new_ext2) = fields2.sorted_iterator_help(subs, ext2);
+    let (it1, new_ext1) = fields1.sorted_iterator_and_ext(subs, ext1);
+    let (it2, new_ext2) = fields2.sorted_iterator_and_ext(subs, ext2);
 
     let it1 = it1.collect::<Vec<_>>();
     let it2 = it2.collect::<Vec<_>>();