Add missing docs for the vm module (#1021)

This PR is a step towards #309.
* Add some tests and documents to clarify `HeaderMetadata.bit_offset`
(related discussion:
https://mmtk.zulipchat.com/#narrow/stream/315620-Porting/topic/ScalaNative.2FMMTK/near/398587245).
* Modify the macro `define_vm_metadata_spec!` to allow adding docs for
the generated types.
* Add missing docs for public items in the `vm` module.

Please feel free to make edits to the PR if there is any issue.

Author: qinsoon

src/util/metadata/header_metadata.rs

@@ -22,7 +22,20 @@ const BITS_IN_U64: usize = 1 << LOG_BITS_IN_U64;
 /// For performance reasons, objects of this struct should be constants.
 #[derive(Clone, Copy, PartialEq, Eq, Hash)]
 pub struct HeaderMetadataSpec {
+    /// `bit_offset` is the index of the starting bit from which the data should be read or written.
+    /// It is counted from the right (least significant bit) of the byte.
+    /// Positive values refer to the bit positions within the current byte, starting with 0 for the
+    /// least significant bit (rightmost) up to 7 for the most significant bit (leftmost).
+    /// Negative values are used to refer to bit positions in the previous bytes, where -1 indicates
+    /// the most significant bit (leftmost) of the byte immediately before the current one.
     pub bit_offset: isize,
+    /// `num_of_bits` specifies the number of consecutive bits to be read or written starting from the `bit_offset`.
+    /// This value is used to define the size of the data field in bits. For instance, if `num_of_bits` is set to 1,
+    /// only a single bit is considered, whereas a value of 8 would indicate a full byte.
+    /// This field must be a positive integer and typically should not exceed the size of the data type that
+    /// will hold the extracted value (for example, 8 bits for a `u8`, 16 bits for a `u16`, etc.).
+    /// The `num_of_bits` together with the `bit_offset` enables the extraction of bit fields of arbitrary
+    /// length and position, facilitating bit-level data manipulation.
     pub num_of_bits: usize,
 }
 
@@ -500,6 +513,59 @@ mod tests {
         spec.assert_spec::<u8>();
     }
 
+    #[test]
+    fn test_negative_bit_offset() {
+        let spec = HeaderMetadataSpec {
+            bit_offset: -1,
+            num_of_bits: 1,
+        };
+        spec.assert_spec::<u8>();
+        assert_eq!(spec.get_shift_and_mask_for_bits(), (7, 0b1000_0000));
+        assert_eq!(spec.byte_offset(), -1);
+        assert_eq!(spec.get_bits_from_u8(0b1000_0000), 1);
+        assert_eq!(spec.get_bits_from_u8(0b0111_1111), 0);
+
+        let spec = HeaderMetadataSpec {
+            bit_offset: -2,
+            num_of_bits: 1,
+        };
+        spec.assert_spec::<u8>();
+        assert_eq!(spec.get_shift_and_mask_for_bits(), (6, 0b0100_0000));
+        assert_eq!(spec.byte_offset(), -1);
+        assert_eq!(spec.get_bits_from_u8(0b0100_0000), 1);
+        assert_eq!(spec.get_bits_from_u8(0b1011_1111), 0);
+
+        let spec = HeaderMetadataSpec {
+            bit_offset: -7,
+            num_of_bits: 1,
+        };
+        spec.assert_spec::<u8>();
+        assert_eq!(spec.get_shift_and_mask_for_bits(), (1, 0b0000_0010));
+        assert_eq!(spec.byte_offset(), -1);
+        assert_eq!(spec.get_bits_from_u8(0b0000_0010), 1);
+        assert_eq!(spec.get_bits_from_u8(0b1111_1101), 0);
+
+        let spec = HeaderMetadataSpec {
+            bit_offset: -8,
+            num_of_bits: 1,
+        };
+        spec.assert_spec::<u8>();
+        assert_eq!(spec.get_shift_and_mask_for_bits(), (0, 0b0000_0001));
+        assert_eq!(spec.byte_offset(), -1);
+        assert_eq!(spec.get_bits_from_u8(0b0000_0001), 1);
+        assert_eq!(spec.get_bits_from_u8(0b1111_1110), 0);
+
+        let spec = HeaderMetadataSpec {
+            bit_offset: -9,
+            num_of_bits: 1,
+        };
+        spec.assert_spec::<u8>();
+        assert_eq!(spec.get_shift_and_mask_for_bits(), (7, 0b1000_0000));
+        assert_eq!(spec.byte_offset(), -2);
+        assert_eq!(spec.get_bits_from_u8(0b1000_0000), 1);
+        assert_eq!(spec.get_bits_from_u8(0b0111_1111), 0);
+    }
+
     #[test]
     fn test_get_bits_from_u8() {
         // 1 bit
@@ -508,6 +574,7 @@ mod tests {
             num_of_bits: 1,
         };
         assert_eq!(spec.get_shift_and_mask_for_bits(), (0, 0b1));
+        assert_eq!(spec.byte_offset(), 0);
         assert_eq!(spec.get_bits_from_u8(0b0000_0001), 1);
         assert_eq!(spec.get_bits_from_u8(0b1111_1110), 0);
 

src/vm/collection.rs

@@ -5,7 +5,9 @@ use crate::{scheduler::*, Mutator};
 
 /// Thread context for the spawned GC thread.  It is used by spawn_gc_thread.
 pub enum GCThreadContext<VM: VMBinding> {
+    /// The GC thread to spawn is a controller thread. There is only one controller thread.
     Controller(Box<GCController<VM>>),
+    /// The GC thread to spawn is a worker thread. There can be multiple worker threads.
     Worker(Box<GCWorker<VM>>),
 }
 

src/vm/edge_shape.rs

@@ -126,7 +126,9 @@ fn a_simple_edge_should_have_the_same_size_as_a_pointer() {
 
 /// A abstract memory slice represents a piece of **heap** memory.
 pub trait MemorySlice: Send + Debug + PartialEq + Eq + Clone + Hash {
+    /// The associate type to define how to access edges from a memory slice.
     type Edge: Edge;
+    /// The associate type to define how to iterate edges in a memory slice.
     type EdgeIterator: Iterator<Item = Self::Edge>;
     /// Iterate object edges within the slice. If there are non-reference values in the slice, the iterator should skip them.
     fn iter_edges(&self) -> Self::EdgeIterator;

src/vm/mod.rs

@@ -19,6 +19,7 @@ use crate::util::constants::*;
 
 mod active_plan;
 mod collection;
+/// Allows MMTk to access edges in a VM-defined way.
 pub mod edge_shape;
 pub(crate) mod object_model;
 mod reference_glue;
@@ -44,10 +45,15 @@ pub trait VMBinding
 where
     Self: Sized + 'static + Send + Sync + Default,
 {
+    /// The binding's implementation of [`crate::vm::ObjectModel`].
     type VMObjectModel: ObjectModel<Self>;
+    /// The binding's implementation of [`crate::vm::Scanning`].
     type VMScanning: Scanning<Self>;
+    /// The binding's implementation of [`crate::vm::Collection`].
     type VMCollection: Collection<Self>;
+    /// The binding's implementation of [`crate::vm::ActivePlan`].
     type VMActivePlan: ActivePlan<Self>;
+    /// The binding's implementation of [`crate::vm::ReferenceGlue`].
     type VMReferenceGlue: ReferenceGlue<Self>;
 
     /// The type of edges in this VM.

src/vm/object_model.rs

@@ -488,17 +488,36 @@ pub mod specs {
     // This macro is invoked in define_vm_metadata_global_spec or define_vm_metadata_local_spec.
     // Use those two to define a new VM metadata spec.
     macro_rules! define_vm_metadata_spec {
-        ($spec_name: ident, $is_global: expr, $log_num_bits: expr, $side_min_obj_size: expr) => {
+        ($(#[$outer:meta])*$spec_name: ident, $is_global: expr, $log_num_bits: expr, $side_min_obj_size: expr) => {
+            $(#[$outer])*
             pub struct $spec_name(MetadataSpec);
             impl $spec_name {
+                /// The number of bits (in log2) that are needed for the spec.
                 pub const LOG_NUM_BITS: usize = $log_num_bits;
+
+                /// Whether this spec is global or local. For side metadata, the binding needs to make sure
+                /// global specs are laid out after another global spec, and local specs are laid
+                /// out after another local spec. Otherwise, there will be an assertion failure.
                 pub const IS_GLOBAL: bool = $is_global;
+
+                /// Declare that the VM uses in-header metadata for this metadata type.
+                /// For the specification of the `bit_offset` argument, please refer to
+                /// the document of `[crate::util::metadata::header_metadata::HeaderMetadataSpec.bit_offset]`.
+                /// The binding needs to make sure that the bits used for a spec in the header do not conflict with
+                /// the bits of another spec (unless it is specified that some bits may be reused).
                 pub const fn in_header(bit_offset: isize) -> Self {
                     Self(MetadataSpec::InHeader(HeaderMetadataSpec {
                         bit_offset,
                         num_of_bits: 1 << Self::LOG_NUM_BITS,
                     }))
                 }
+
+                /// Declare that the VM uses side metadata for this metadata type,
+                /// and the side metadata is the first of its kind (global or local).
+                /// The first global or local side metadata should be declared with `side_first()`,
+                /// and the rest side metadata should be declared with `side_after()` after a defined
+                /// side metadata of the same kind (global or local). Logically, all the declarations
+                /// create two list of side metadata, one for global, and one for local.
                 pub const fn side_first() -> Self {
                     if Self::IS_GLOBAL {
                         Self(MetadataSpec::OnSide(SideMetadataSpec {
@@ -518,6 +537,13 @@ pub mod specs {
                         }))
                     }
                 }
+
+                /// Declare that the VM uses side metadata for this metadata type,
+                /// and the side metadata should be laid out after the given side metadata spec.
+                /// The first global or local side metadata should be declared with `side_first()`,
+                /// and the rest side metadata should be declared with `side_after()` after a defined
+                /// side metadata of the same kind (global or local). Logically, all the declarations
+                /// create two list of side metadata, one for global, and one for local.
                 pub const fn side_after(spec: &MetadataSpec) -> Self {
                     assert!(spec.is_on_side());
                     let side_spec = spec.extract_side_spec();
@@ -530,9 +556,13 @@ pub mod specs {
                         log_bytes_in_region: $side_min_obj_size as usize,
                     }))
                 }
+
+                /// Return the inner `[crate::util::metadata::MetadataSpec]` for the metadata type.
                 pub const fn as_spec(&self) -> &MetadataSpec {
                     &self.0
                 }
+
+                /// Return the number of bits for the metadata type.
                 pub const fn num_bits(&self) -> usize {
                     1 << $log_num_bits
                 }
@@ -547,20 +577,57 @@ pub mod specs {
     }
 
     // Log bit: 1 bit per object, global
-    define_vm_metadata_spec!(VMGlobalLogBitSpec, true, 0, LOG_MIN_OBJECT_SIZE);
+    define_vm_metadata_spec!(
+        #[doc = "1-bit global metadata to log an object."]
+        VMGlobalLogBitSpec,
+        true,
+        0,
+        LOG_MIN_OBJECT_SIZE
+    );
     // Forwarding pointer: word size per object, local
     define_vm_metadata_spec!(
+        #[doc = "1-word local metadata for spaces that may copy objects."]
+        #[doc = "This metadata has to be stored in the header."]
+        #[doc = "This metadata can be defined at a position within the object payload."]
+        #[doc = "As a forwarding pointer is only stored in dead objects which is not"]
+        #[doc = "accessible by the language, it is okay that store a forwarding pointer overwrites object payload"]
         VMLocalForwardingPointerSpec,
         false,
         LOG_BITS_IN_WORD,
         LOG_MIN_OBJECT_SIZE
     );
     // Forwarding bits: 2 bits per object, local
-    define_vm_metadata_spec!(VMLocalForwardingBitsSpec, false, 1, LOG_MIN_OBJECT_SIZE);
+    define_vm_metadata_spec!(
+        #[doc = "2-bit local metadata for spaces that store a forwarding state for objects."]
+        #[doc = "If this spec is defined in the header, it can be defined with a position of the lowest 2 bits in the forwarding pointer."]
+        VMLocalForwardingBitsSpec,
+        false,
+        1,
+        LOG_MIN_OBJECT_SIZE
+    );
     // Mark bit: 1 bit per object, local
-    define_vm_metadata_spec!(VMLocalMarkBitSpec, false, 0, LOG_MIN_OBJECT_SIZE);
+    define_vm_metadata_spec!(
+        #[doc = "1-bit local metadata for spaces that need to mark an object."]
+        VMLocalMarkBitSpec,
+        false,
+        0,
+        LOG_MIN_OBJECT_SIZE
+    );
     // Pinning bit: 1 bit per object, local
-    define_vm_metadata_spec!(VMLocalPinningBitSpec, false, 0, LOG_MIN_OBJECT_SIZE);
+    define_vm_metadata_spec!(
+        #[doc = "1-bit local metadata for spaces that support pinning."]
+        VMLocalPinningBitSpec,
+        false,
+        0,
+        LOG_MIN_OBJECT_SIZE
+    );
     // Mark&nursery bits for LOS: 2 bit per page, local
-    define_vm_metadata_spec!(VMLocalLOSMarkNurserySpec, false, 1, LOG_BYTES_IN_PAGE);
+    define_vm_metadata_spec!(
+        #[doc = "2-bits local metadata for the large object space. The two bits serve as"]
+        #[doc = "the mark bit and the nursery bit."]
+        VMLocalLOSMarkNurserySpec,
+        false,
+        1,
+        LOG_BYTES_IN_PAGE
+    );
 }

src/vm/scanning.rs

@@ -242,6 +242,14 @@ pub trait Scanning<VM: VMBinding> {
     /// Return whether the VM supports return barriers. This is unused at the moment.
     fn supports_return_barrier() -> bool;
 
+    /// Prepare for another round of root scanning in the same GC. Some GC algorithms
+    /// need multiple transitive closures, and each transitive closure starts from
+    /// root scanning. We expect the binding to provide the same root set for every
+    /// round of root scanning in the same GC. Bindings can use this call to get
+    /// ready for another round of root scanning to make sure that the same root
+    /// set will be returned in the upcoming calls of root scanning methods,
+    /// such as [`crate::vm::Scanning::scan_roots_in_mutator_thread`] and
+    /// [`crate::vm::Scanning::scan_vm_specific_roots`].
     fn prepare_for_roots_re_scanning();
 
     /// Process weak references.