[Cider] Replace ibig with num-bigint (#2279)

A relatively minor set of documentation changes and some changes to the
arbitrary precision numbers. Deceptively frustrating to implement but
should be more compatible with future changes to `Value`.

interp/Cargo.toml

@@ -24,16 +24,16 @@ pest_consume.workspace = true
 argh.workspace = true
 owo-colors = "^3.5"
 bitvec = "1.0"
-base64 = "0.13.0"
 serde_json = "1.0"
 rustyline = "=10.0.0"
 fraction = { version = "0.11.0", features = ["with-serde-support"] }
 thiserror = "1.0.26"
-ibig = { version = "0.3.4", features = ["serde"] }
 slog = "2.7.0"
 slog-term = "2.8.0"
 slog-async = "2.7.0"
 ahash = "0.8.3"
+num-bigint = "0.4.6"
+num-traits = "0.2.19"
 
 once_cell = "1.9.0"
 petgraph = "0.6.3"

interp/src/flatten/flat_ir/base.rs

@@ -85,18 +85,21 @@ impl_index!(GlobalRefPortIdx);
 
 // Offset indices
 
-/// A local port offset for a component. These are used in the definition of
-/// assignments and can only be understood in the context of the component they
-/// are defined under. Combined with a base index from a component instance this
-/// can be resolved to a [`GlobalPortIdx`].
+/// A local port offset for a component.
+///
+/// These are used in the definition of assignments and can only be understood
+/// in the context of the component they are defined under. Combined with a base
+/// index from a component instance this can be resolved to a [`GlobalPortIdx`].
 #[derive(Debug, Eq, Copy, Clone, PartialEq, Hash, PartialOrd, Ord)]
 pub struct LocalPortOffset(u32);
 impl_index!(LocalPortOffset);
 
-/// A local ref port offset for a component. These are used in the definition of
-/// assignments and can only be understood in the context of the component they
-/// are defined under. Combined with a base index from a component instance this
-/// can be resolved to a [`GlobalRefPortIdx`].
+/// A local ref port offset for a component.
+///
+/// These are used in the definition of assignments and can only be understood
+/// in the context of the component they are defined under. Combined with a base
+/// index from a component instance this can be resolved to a
+/// [`GlobalRefPortIdx`].
 #[derive(Debug, Eq, Copy, Clone, PartialEq, Hash, PartialOrd, Ord)]
 pub struct LocalRefPortOffset(u32);
 impl_index!(LocalRefPortOffset);
@@ -380,9 +383,11 @@ impl From<CellDefinitionIdx> for CellDefinitionRef {
 pub struct AssignmentIdx(u32);
 impl_index!(AssignmentIdx);
 
-/// An enum representing the "winner" of an assignment. This tells us how the
-/// value was assigned to the port. For standard assignments, this is also used
-/// to detect conflicts where there are multiple driving assignments.
+/// An enum representing the "winner" of an assignment.
+///
+/// This tells us how the value was assigned to the port. For standard
+/// assignments, this is also used to detect conflicts where there are multiple
+/// driving assignments.
 #[derive(Debug, Clone, PartialEq, Eq)]
 pub enum AssignmentWinner {
     /// Indicates that the "winning" assignment for this port was produced by a

interp/src/flatten/flat_ir/identifier.rs

@@ -3,10 +3,12 @@ use std::hash::Hash;
 
 use crate::flatten::structures::index_trait::{impl_index, IndexRef};
 
-/// An index type corresponding to a string. Similar to [calyx_ir::Id] but
-/// cannot be turned into a string directly. Strings are stored in the
-/// interpretation context [crate::flatten::structures::context::Context] and
-/// can be looked up via [crate::flatten::structures::context::Context::lookup_name]
+/// An index type corresponding to a string.
+///
+/// Similar to [calyx_ir::Id] but cannot be turned into a string directly.
+/// Strings are stored in the interpretation context
+/// [crate::flatten::structures::context::Context] and can be looked up via
+/// [crate::flatten::structures::context::Context::lookup_name]
 #[derive(Debug, Eq, Copy, Clone, PartialEq, Hash, PartialOrd, Ord)]
 pub struct Identifier(u32);
 

interp/src/flatten/primitives/stateful/math.rs

@@ -1,22 +1,16 @@
-use ibig::ops::RemEuclid;
-
 use crate::{
     flatten::{
         flat_ir::prelude::*,
         primitives::{
-            declare_ports,
+            declare_ports, ports,
+            prim_trait::*,
             utils::{floored_division, int_sqrt, ShiftBuffer},
         },
-    },
-    values::InputNumber,
-};
-use crate::{
-    flatten::{
-        primitives::{ports, prim_trait::*},
         structures::environment::PortMap,
     },
-    values::Value,
+    values::{InputNumber, Value},
 };
+use num_traits::Euclid;
 
 pub struct StdMultPipe<const DEPTH: usize> {
     base_port: GlobalPortIdx,
@@ -199,7 +193,7 @@ impl<const DEPTH: usize, const SIGNED: bool> Primitive
                                     (left
                                         .val()
                                         .as_unsigned()
-                                        .rem_euclid(right.val().as_unsigned()))
+                                        .rem_euclid(&right.val().as_unsigned()))
                                     .into()
                                 } else {
                                     (left.val().as_signed()
@@ -530,7 +524,7 @@ impl<const DEPTH: usize, const SIGNED: bool> Primitive
                                     (left
                                         .val()
                                         .as_unsigned()
-                                        .rem_euclid(right.val().as_unsigned()))
+                                        .rem_euclid(&right.val().as_unsigned()))
                                     .into()
                                 } else {
                                     (left.val().as_signed()

interp/src/flatten/primitives/utils.rs

@@ -1,14 +1,13 @@
+use num_bigint::{BigInt, BigUint, Sign};
 use std::collections::VecDeque;
 
-use ibig::{ibig, UBig};
-use ibig::{ubig, IBig};
-
-pub(crate) fn floored_division(left: &IBig, right: &IBig) -> IBig {
+/// There's probably a better way to do this but I'm not fixing it right now
+pub(crate) fn floored_division(left: &BigInt, right: &BigInt) -> BigInt {
     let div = left / right;
 
-    if left.signum() != ibig!(-1) && right.signum() != ibig!(-1) {
+    if left.sign() != Sign::Minus && right.sign() != Sign::Minus {
         div
-    } else if (div.signum() == (-1).into() || div.signum() == 0.into())
+    } else if (div.sign() == Sign::Minus || div.sign() == Sign::NoSign)
         && (left != &(&div * right))
     {
         div - 1_i32
@@ -18,14 +17,17 @@ pub(crate) fn floored_division(left: &IBig, right: &IBig) -> IBig {
 }
 
 /// Implementation of integer square root via a basic binary search algorithm
-/// based on wikipedia psuedocode
-pub(crate) fn int_sqrt(i: &UBig) -> UBig {
-    let mut lower: UBig = ubig!(0);
-    let mut upper: UBig = i + ubig!(1);
-    let mut temp: UBig;
+/// based on wikipedia pseudocode.
+///
+/// TODO griffin: See if this can be replaced with a `sqrt` function in the
+/// `num-bigint` crate
+pub(crate) fn int_sqrt(i: &BigUint) -> BigUint {
+    let mut lower: BigUint = BigUint::from(0_u32);
+    let mut upper: BigUint = i + BigUint::from(1_u32);
+    let mut temp: BigUint;
 
-    while lower != (&upper - ubig!(1)) {
-        temp = (&lower + &upper) / ubig!(2);
+    while lower != (&upper - BigUint::from(1_u32)) {
+        temp = (&lower + &upper) / BigUint::from(2_u32);
         if &(&temp * &temp) <= i {
             lower = temp
         } else {

interp/src/flatten/setup.rs

@@ -14,6 +14,7 @@ fn do_setup(
     file: &Option<PathBuf>,
     lib_path: &Path,
     skip_verification: bool,
+    gen_metadata: bool,
 ) -> InterpreterResult<(Context, InterpreterResult<NewSourceMap>)> {
     // Construct IR
     let ws = frontend::Workspace::construct(file, lib_path)?;
@@ -23,42 +24,53 @@ fn do_setup(
     if !skip_verification {
         pm.execute_plan(&mut ctx, &["validate".to_string()], &[], &[], false)?;
     }
-    pm.execute_plan(
-        &mut ctx,
-        &["metadata-table-generation".to_string()],
-        &[],
-        &[],
-        false,
-    )?;
 
-    let mapping = ctx
-        .metadata
-        .as_ref()
-        .map(|metadata| {
-            crate::debugger::source::new_parser::parse_metadata(metadata)
-        })
-        .unwrap_or_else(|| {
-            Err(crate::errors::InterpreterError::MissingMetaData.into())
-        });
+    let mapping = if gen_metadata {
+        pm.execute_plan(
+            &mut ctx,
+            &["metadata-table-generation".to_string()],
+            &[],
+            &[],
+            false,
+        )?;
+        ctx.metadata
+            .as_ref()
+            .map(|metadata| {
+                crate::debugger::source::new_parser::parse_metadata(metadata)
+            })
+            .unwrap_or_else(|| {
+                Err(crate::errors::InterpreterError::MissingMetaData.into())
+            })
+    } else {
+        Err(crate::errors::InterpreterError::MissingMetaData.into())
+    };
 
     // general setup
     Ok((crate::flatten::flat_ir::translate(&ctx), mapping))
 }
 
+/// This function sets up the simulation context for the given program. This is
+/// meant to be used in contexts where calyx metadata is not required. For other
+/// cases, use [setup_simulation_with_metadata]
 pub fn setup_simulation(
     file: &Option<PathBuf>,
     lib_path: &Path,
     skip_verification: bool,
 ) -> InterpreterResult<Context> {
-    let (ctx, _) = do_setup(file, lib_path, skip_verification)?;
+    let (ctx, _) = do_setup(file, lib_path, skip_verification, false)?;
     Ok(ctx)
 }
 
+/// Constructs the simulation context for the given program. Additionally
+/// attempts to construct the metadata table for the program.
+///
+/// For cases where the metadata is not required, use [setup_simulation], which
+/// has less of a performance impact.
 pub fn setup_simulation_with_metadata(
     file: &Option<PathBuf>,
     lib_path: &Path,
     skip_verification: bool,
 ) -> InterpreterResult<(Context, NewSourceMap)> {
-    let (ctx, mapping) = do_setup(file, lib_path, skip_verification)?;
+    let (ctx, mapping) = do_setup(file, lib_path, skip_verification, true)?;
     Ok((ctx, mapping?))
 }

interp/src/flatten/structures/environment/env.rs

@@ -1188,8 +1188,10 @@ impl<C: AsRef<Context> + Clone> Environment<C> {
 }
 
 /// A wrapper struct for the environment that provides the functions used to
-/// simulate the actual program. This is just to keep the simulation logic under
-/// a different namespace than the environment to avoid confusion
+/// simulate the actual program.
+///
+/// This is just to keep the simulation logic under a different namespace than
+/// the environment to avoid confusion
 pub struct Simulator<C: AsRef<Context> + Clone> {
     env: Environment<C>,
 }

interp/src/flatten/structures/index_trait.rs

@@ -242,7 +242,8 @@ where
         (size, Some(size))
     }
 }
-
+/// An iterator over a range of indices but without
+///
 /// Because I really played myself by making the [IndexRangeIterator] have a
 /// lifetime attached to it. This one doesn't do that. As with it's sibling, the
 /// range is half open, meaning that the start is inclusive, but the end is

interp/src/serialization/formatting.rs

@@ -96,9 +96,11 @@ impl From<&MemoryDimensions> for Shape {
     }
 }
 
-/// A wrapper enum used during serialization. It represents either an unsigned integer,
-/// or a signed integer and is serialized as the underlying integer. This also allows
-/// mixed serialization of signed and unsigned values
+/// A wrapper enum used during serialization.
+///
+/// It represents either an unsigned integer, or a signed integer and is
+/// serialized as the underlying integer. This also allows mixed serialization
+/// of signed and unsigned values
 #[derive(Serialize, Clone)]
 #[serde(untagged)]
 pub enum Entry {