Add documentation to lib/src/index.rs and lib/src/default_index/

lib/src/default_index/composite.rs

@@ -98,7 +98,13 @@ impl<T: AsCompositeIndex + ?Sized> AsCompositeIndex for &mut T {
     }
 }
 
-/// Reference wrapper that provides global access to nested index segments.
+/// `CompositeIndex` provides an index of both commit IDs and change IDs.
+///
+/// We refer to this as a composite index because it's a composite of multiple
+/// nested index segments where each parent segment is roughly twice as large
+/// its child. segment. This provides a good balance between read and write
+/// performance.
+// Reference wrapper that provides global access to nested index segments.
 #[derive(RefCastCustom)]
 #[repr(transparent)]
 pub struct CompositeIndex(DynIndexSegment);
@@ -355,6 +361,8 @@ impl CompositeIndex {
             .map(|(i, _)| IndexPosition(u32::try_from(i).unwrap()))
     }
 
+    /// Returns the subset of positions in `candidate_positions` which refer to
+    /// entries that are heads in the repository.
     pub fn heads_pos(
         &self,
         mut candidate_positions: BTreeSet<IndexPosition>,
@@ -475,7 +483,6 @@ impl Index for &CompositeIndex {
             .collect()
     }
 
-    /// Parents before children
     fn topo_order(&self, input: &mut dyn Iterator<Item = &CommitId>) -> Vec<CommitId> {
         let mut ids = input.cloned().collect_vec();
         ids.sort_by_cached_key(|id| self.commit_id_to_pos(id).unwrap());
@@ -511,12 +518,12 @@ impl<I: AsCompositeIndex> ChangeIdIndexImpl<I> {
 }
 
 impl<I: AsCompositeIndex + Send + Sync> ChangeIdIndex for ChangeIdIndexImpl<I> {
-    /// Resolves change id prefix among all ids, then filters out hidden
-    /// entries.
-    ///
-    /// If `SingleMatch` is returned, the commits including in the set are all
-    /// visible. `AmbiguousMatch` may be returned even if the prefix is unique
-    /// within the visible entries.
+    // Resolves change id prefix among all ids, then filters out hidden
+    // entries.
+    //
+    // If `SingleMatch` is returned, the commits including in the set are all
+    // visible. `AmbiguousMatch` may be returned even if the prefix is unique
+    // within the visible entries.
     fn resolve_prefix(&self, prefix: &HexPrefix) -> PrefixResolution<Vec<CommitId>> {
         let index = self.index.as_composite();
         match index.resolve_change_id_prefix(prefix) {
@@ -540,11 +547,12 @@ impl<I: AsCompositeIndex + Send + Sync> ChangeIdIndex for ChangeIdIndexImpl<I> {
         }
     }
 
-    /// Calculates the shortest prefix length of the given `change_id` among all
-    /// ids including hidden entries.
-    ///
-    /// The returned length is usually a few digits longer than the minimum
-    /// length to disambiguate within the visible entries.
+    // Calculates the shortest prefix length of the given `change_id` among all
+    // IDs, including hidden entries.
+    //
+    // The returned length is usually a few digits longer than the minimum
+    // length necessary to disambiguate within the visible entries since hidden
+    // entries are also considered when determining the prefix length.
     fn shortest_unique_prefix_len(&self, change_id: &ChangeId) -> usize {
         self.index
             .as_composite()

lib/src/default_index/mod.rs

@@ -12,6 +12,13 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
+//! An on-disk index of the commits in a repository.
+//!
+//! Implements an index of the commits in a repository that conforms to the
+//! trains in the [index module](crate::index). The index is stored on local
+//! disk and contains an entry for every commit in the repository. See
+//! [`DefaultReadonlyIndex`] and [`DefaultMutableIndex`].
+
 #![allow(missing_docs)]
 
 mod composite;
@@ -1008,12 +1015,14 @@ mod tests {
         let id_3 = CommitId::from_hex("333333");
         let id_4 = CommitId::from_hex("444444");
         let id_5 = CommitId::from_hex("555555");
+        let id_6 = CommitId::from_hex("666666");
         index.add_commit_data(id_0.clone(), new_change_id(), &[]);
         index.add_commit_data(id_1.clone(), new_change_id(), &[id_0.clone()]);
         index.add_commit_data(id_2.clone(), new_change_id(), &[id_0.clone()]);
         index.add_commit_data(id_3.clone(), new_change_id(), &[id_0.clone()]);
         index.add_commit_data(id_4.clone(), new_change_id(), &[id_1.clone()]);
         index.add_commit_data(id_5.clone(), new_change_id(), &[id_4.clone(), id_2.clone()]);
+        index.add_commit_data(id_6.clone(), new_change_id(), &[id_4.clone()]);
 
         assert_eq!(
             index.common_ancestors(&[id_0.clone()], &[id_0.clone()]),
@@ -1047,6 +1056,10 @@ mod tests {
             index.common_ancestors(&[id_5.clone()], &[id_3.clone()]),
             vec![id_0.clone()]
         );
+        assert_eq!(
+            index.common_ancestors(&[id_2.clone()], &[id_6.clone()]),
+            vec![id_0.clone()]
+        );
 
         // With multiple commits in an input set
         assert_eq!(
@@ -1065,6 +1078,12 @@ mod tests {
             index.common_ancestors(&[id_1.clone(), id_2.clone()], &[id_4]),
             vec![id_1.clone()]
         );
+        assert_eq!(
+            index.common_ancestors(&[id_5.clone(), id_6.clone()], &[id_2.clone()]),
+            &[id_2.clone()]
+        );
+        // Both 1 and 2 are returned since (5) expands to (2, 4), which expands
+        // to (1,2) and matches the (1,2) of the first input set.
         assert_eq!(
             index.common_ancestors(&[id_1.clone(), id_2.clone()], &[id_5]),
             vec![id_1.clone(), id_2.clone()]

lib/src/index.rs

@@ -12,7 +12,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-#![allow(missing_docs)]
+//! Interfaces for indexes of the commits in a repository.
 
 use std::any::Any;
 use std::fmt::Debug;
@@ -28,49 +28,75 @@ use crate::operation::Operation;
 use crate::revset::{ResolvedExpression, Revset, RevsetEvaluationError};
 use crate::store::Store;
 
-/// Error while reading index from the `IndexStore`.
+/// Returned if an error occurs while reading an index from the [`IndexStore`].
 #[derive(Debug, Error)]
 #[error(transparent)]
 pub struct IndexReadError(pub Box<dyn std::error::Error + Send + Sync>);
 
-/// Error while writing index to the `IndexStore`.
+/// Returned if an error occurs while writing an index to the [`IndexStore`].
 #[derive(Debug, Error)]
 #[error(transparent)]
 pub struct IndexWriteError(pub Box<dyn std::error::Error + Send + Sync>);
 
-/// Error to be returned if `Index::all_heads_for_gc()` is not supported by the
+/// An error returned if `Index::all_heads_for_gc()` is not supported by the
 /// index backend.
 #[derive(Debug, Error)]
 #[error("Cannot collect all heads by index of this type")]
 pub struct AllHeadsForGcUnsupported;
 
+/// Defines the interface for types that provide persistent storage for an
+/// index.
 pub trait IndexStore: Send + Sync + Debug {
+    #[allow(missing_docs)]
     fn as_any(&self) -> &dyn Any;
 
+    /// Returns a name representing the type of index that the `IndexStore` is
+    /// compatible with. For example, the `IndexStore` for the default index
+    /// returns "default".
     fn name(&self) -> &str;
 
+    /// Returns the index at the specified operation.
     fn get_index_at_op(
         &self,
         op: &Operation,
         store: &Arc<Store>,
     ) -> Result<Box<dyn ReadonlyIndex>, IndexReadError>;
 
+    /// Writes `index` to the index store and returns a read-only version of the
+    /// index.
     fn write_index(
         &self,
         index: Box<dyn MutableIndex>,
         op_id: &OperationId,
     ) -> Result<Box<dyn ReadonlyIndex>, IndexWriteError>;
 }
 
+/// Defines the interface for types that provide an index of the commits in a
+/// repository by [`CommitId`].
 pub trait Index: Send + Sync {
+    /// Returns the minimum prefix length to disambiguate `commit_id` from other
+    /// commits in the index. The length returned is the number of hexadecimal
+    /// digits in the minimum prefix.
+    ///
+    /// If the given `commit_id` doesn't exist, returns the minimum prefix
+    /// length which matches none of the commits in the index.
     fn shortest_unique_commit_id_prefix_len(&self, commit_id: &CommitId) -> usize;
 
+    /// Searches the index for commit IDs matching `prefix`. Returns a
+    /// [`PrefixResolution`] with a [`CommitId`] if the prefix matches a single
+    /// commit.
     fn resolve_commit_id_prefix(&self, prefix: &HexPrefix) -> PrefixResolution<CommitId>;
 
+    /// Returns true if `commit_id` is present in the index.
     fn has_id(&self, commit_id: &CommitId) -> bool;
 
+    /// Returns true if `ancestor_id` commit is an ancestor of the
+    /// `descendant_id` commit, or if `ancestor_id` equals `descendant_id`.
     fn is_ancestor(&self, ancestor_id: &CommitId, descendant_id: &CommitId) -> bool;
 
+    /// Returns the best common ancestor or ancestors of the commits in `set1`
+    /// and `set2`. A "best common ancestor" has no descendants that are also
+    /// common ancestors.
     fn common_ancestors(&self, set1: &[CommitId], set2: &[CommitId]) -> Vec<CommitId>;
 
     /// Heads among all indexed commits at the associated operation.
@@ -85,18 +111,25 @@ pub trait Index: Send + Sync {
         &self,
     ) -> Result<Box<dyn Iterator<Item = CommitId> + '_>, AllHeadsForGcUnsupported>;
 
+    /// Returns the subset of commit IDs in `candidates` which are not ancestors
+    /// of other commits in `candidates`. If a commit id is duplicated in the
+    /// `candidates` list it will appear at most once in the output.
     fn heads(&self, candidates: &mut dyn Iterator<Item = &CommitId>) -> Vec<CommitId>;
 
-    /// Parents before children
+    /// Orders a list of commit IDs such that parent commits appear before any
+    /// of their children. Commit IDs in the `input` are not deduplicated.
     fn topo_order(&self, input: &mut dyn Iterator<Item = &CommitId>) -> Vec<CommitId>;
 
+    /// Resolves the revset `expression` against the index and corresponding
+    /// `store`.
     fn evaluate_revset<'index>(
         &'index self,
         expression: &ResolvedExpression,
         store: &Arc<Store>,
     ) -> Result<Box<dyn Revset + 'index>, RevsetEvaluationError>;
 }
 
+#[allow(missing_docs)]
 pub trait ReadonlyIndex: Send + Sync {
     fn as_any(&self) -> &dyn Any;
 
@@ -108,6 +141,7 @@ pub trait ReadonlyIndex: Send + Sync {
     fn start_modification(&self) -> Box<dyn MutableIndex>;
 }
 
+#[allow(missing_docs)]
 pub trait MutableIndex {
     fn as_any(&self) -> &dyn Any;
 
@@ -125,6 +159,8 @@ pub trait MutableIndex {
     fn merge_in(&mut self, other: &dyn ReadonlyIndex);
 }
 
+/// Defines the interface for types that provide an index of the commits in a
+/// repository by [`ChangeId`].
 pub trait ChangeIdIndex: Send + Sync {
     /// Resolve an unambiguous change ID prefix to the commit IDs in the index.
     ///
@@ -134,7 +170,7 @@ pub trait ChangeIdIndex: Send + Sync {
     /// This function returns the shortest length of a prefix of `key` that
     /// disambiguates it from every other key in the index.
     ///
-    /// The length to be returned is a number of hexadecimal digits.
+    /// The length returned is a number of hexadecimal digits.
     ///
     /// This has some properties that we do not currently make much use of:
     ///