refactor fayalite::cli to expose subcommands

crates/fayalite/examples/blinky.rs

@@ -1,8 +1,9 @@
 use clap::Parser;
 use fayalite::{
+    cli,
     clock::{Clock, ClockDomain},
     hdl_module,
-    int::{DynUInt, DynUIntType, IntTypeTrait, UInt},
+    int::{DynUInt, DynUIntType, IntCmp, IntTypeTrait, UInt},
     reset::{SyncReset, ToReset},
 };
 
@@ -24,8 +25,8 @@ fn blinky(clock_frequency: u64) {
     #[hdl]
     let output_reg: UInt<1> = m.reg_builder().clock_domain(cd).reset_default();
     #[hdl]
-    if counter == int_ty.literal(max_value) {
-        m.connect(counter, int_ty.literal(0));
+    if counter.cmp_eq(max_value) {
+        m.connect_any(counter, 0u8);
         m.connect(output_reg, !output_reg);
     } else {
         m.connect_any(counter, counter + 1_hdl_u1);
@@ -41,10 +42,10 @@ struct Cli {
     #[arg(long, default_value = "1000000", value_parser = clap::value_parser!(u64).range(2..))]
     clock_frequency: u64,
     #[command(subcommand)]
-    cli: fayalite::cli::Cli,
+    cli: cli::Cli,
 }
 
-fn main() {
+fn main() -> cli::Result {
     let cli = Cli::parse();
     cli.cli.run(blinky(cli.clock_frequency))
 }

crates/fayalite/src/cli.rs

@@ -4,20 +4,97 @@ use crate::{
     intern::Interned,
     module::Module,
 };
-use clap::{Parser, Subcommand, ValueHint};
-use std::{path::PathBuf, process::exit};
+use clap::{Args, Parser, Subcommand, ValueHint};
+use std::{fmt, path::PathBuf};
+
+enum CliErrorImpl {
+    IoError(std::io::Error),
+}
+
+pub type Result<T = (), E = CliError> = std::result::Result<T, E>;
+
+pub struct CliError(CliErrorImpl);
+
+impl fmt::Debug for CliError {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        match &self.0 {
+            CliErrorImpl::IoError(e) => e.fmt(f),
+        }
+    }
+}
+
+impl fmt::Display for CliError {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        match &self.0 {
+            CliErrorImpl::IoError(e) => e.fmt(f),
+        }
+    }
+}
+
+impl std::error::Error for CliError {
+    fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
+        match &self.0 {
+            CliErrorImpl::IoError(e) => e.source(),
+        }
+    }
+}
+
+impl From<std::io::Error> for CliError {
+    fn from(value: std::io::Error) -> Self {
+        CliError(CliErrorImpl::IoError(value))
+    }
+}
+
+pub trait RunPhase<Arg> {
+    fn run(self, arg: Arg) -> Result;
+}
+
+#[derive(Args)]
+#[non_exhaustive]
+pub struct FirrtlArgs {
+    /// the directory to put the generated FIRRTL and associated files in
+    #[arg(short, long, value_hint = ValueHint::DirPath)]
+    pub output: PathBuf,
+    /// the stem of the generated .fir file, e.g. to get foo.fir, pass --file-stem=foo
+    #[arg(long)]
+    pub file_stem: Option<String>,
+}
+
+impl FirrtlArgs {
+    fn run_impl(self, top_module: Module<DynBundle>) -> Result {
+        firrtl::export(
+            firrtl::FileBackend {
+                dir_path: self.output,
+                top_fir_file_stem: self.file_stem,
+            },
+            &top_module,
+        )?;
+        Ok(())
+    }
+}
+
+impl<T: BundleValue> RunPhase<Module<T>> for FirrtlArgs
+where
+    T::Type: BundleType<Value = T>,
+{
+    fn run(self, top_module: Module<T>) -> Result {
+        self.run_impl(top_module.canonical())
+    }
+}
+
+impl<T: BundleValue> RunPhase<Interned<Module<T>>> for FirrtlArgs
+where
+    T::Type: BundleType<Value = T>,
+{
+    fn run(self, top_module: Interned<Module<T>>) -> Result {
+        self.run(*top_module)
+    }
+}
 
 #[derive(Subcommand)]
 enum CliCommand {
     /// Generate FIRRTL
-    Firrtl {
-        /// the directory to put the generated FIRRTL and associated files in
-        #[arg(short, long, value_hint = ValueHint::DirPath)]
-        output: PathBuf,
-        /// the stem of the generated .fir file, e.g. to get foo.fir, pass --file-stem=foo
-        #[arg(long)]
-        file_stem: Option<String>,
-    },
+    Firrtl(FirrtlArgs),
 }
 
 /// a simple CLI
@@ -25,26 +102,29 @@ enum CliCommand {
 /// Use like:
 ///
 /// ```no_run
-/// # use fayalite::{hdl_module, cli::Cli};
+/// # use fayalite::hdl_module;
 /// # #[hdl_module]
 /// # fn my_module() {}
-/// fn main() {
-///     Cli::parse().run(my_module());
+/// use fayalite::cli;
+///
+/// fn main() -> cli::Result {
+///     cli::Cli::parse().run(my_module())
 /// }
 /// ```
 ///
 /// You can also use it with a larger [`clap`]-based CLI like so:
 ///
 /// ```no_run
-/// # use fayalite::{hdl_module};
+/// # use fayalite::hdl_module;
 /// # #[hdl_module]
 /// # fn my_module() {}
 /// use clap::{Subcommand, Parser};
+/// use fayalite::cli;
 ///
 /// #[derive(Subcommand)]
 /// pub enum Cmd {
 ///     #[command(flatten)]
-///     Fayalite(fayalite::cli::Cli),
+///     Fayalite(cli::Cli),
 ///     MySpecialCommand {
 ///         #[arg(long)]
 ///         foo: bool,
@@ -54,14 +134,15 @@ enum CliCommand {
 /// #[derive(Parser)]
 /// pub struct Cli {
 ///     #[command(subcommand)]
-///     cmd: Cmd, // or just use fayalite::cli::Cli directly
+///     cmd: Cmd, // or just use cli::Cli directly if you don't need more subcommands
 /// }
 ///
-/// fn main() {
+/// fn main() -> cli::Result {
 ///     match Cli::parse().cmd {
-///         Cmd::Fayalite(v) => v.run(my_module()),
+///         Cmd::Fayalite(v) => v.run(my_module())?,
 ///         Cmd::MySpecialCommand { foo } => println!("special: foo={foo}"),
 ///     }
+///     Ok(())
 /// }
 /// ```
 #[derive(Parser)]
@@ -86,33 +167,27 @@ impl clap::Subcommand for Cli {
     }
 }
 
+impl<T> RunPhase<T> for Cli
+where
+    FirrtlArgs: RunPhase<T>,
+{
+    fn run(self, arg: T) -> Result {
+        match self.subcommand {
+            CliCommand::Firrtl(firrtl_args) => firrtl_args.run(arg),
+        }
+    }
+}
+
 impl Cli {
     /// forwards to [`clap::Parser::parse()`] so you don't have to import [`clap::Parser`]
     pub fn parse() -> Self {
         clap::Parser::parse()
     }
-    fn run_impl(self, top_module: Module<DynBundle>) -> ! {
-        match self.subcommand {
-            CliCommand::Firrtl { output, file_stem } => {
-                let result = firrtl::export(
-                    firrtl::FileBackend {
-                        dir_path: output,
-                        top_fir_file_stem: file_stem,
-                    },
-                    &top_module,
-                );
-                if let Err(e) = result {
-                    eprintln!("Error: {:?}", eyre::Report::new(e));
-                    exit(1);
-                }
-            }
-        }
-        exit(0)
-    }
-    pub fn run<T: BundleValue>(self, top_module: Interned<Module<T>>) -> !
+    /// forwards to [`RunPhase::run()`] so you don't have to import [`RunPhase`]
+    pub fn run<T>(self, top_module: T) -> Result
     where
-        T::Type: BundleType<Value = T>,
+        Self: RunPhase<T>,
     {
-        self.run_impl(top_module.canonical())
+        RunPhase::run(self, top_module)
     }
 }

crates/fayalite/src/expr/ops.rs

@@ -784,16 +784,16 @@ macro_rules! cmp_op {
                     CanonicalType = DynIntType<<RhsType as IntTypeTrait>::Signed>,
                     CanonicalValue = DynInt<<RhsType as IntTypeTrait>::Signed>,
                 >,
-                Rhs: Value<Type = RhsType>,
+                Rhs: ToExpr<Type = RhsType>,
                 Lhs: Value<Type = LhsType>,
-            > IntCmp<Expr<Rhs>> for Expr<Lhs>
+            > IntCmp<Rhs> for Expr<Lhs>
         {
             type Output = Expr<UInt<1>>;
 
-            $(fn $fn(self, rhs: Expr<Rhs>) -> Self::Output {
+            $(fn $fn(self, rhs: Rhs) -> Self::Output {
                 $CmpOp::<LhsType, RhsType, UIntType<1>>::new_unchecked(
                     self.canonical(),
-                    rhs.canonical(),
+                    rhs.to_expr().canonical(),
                 ).to_expr()
             })*
         }
@@ -808,32 +808,11 @@ macro_rules! cmp_op {
                 CanonicalType = DynIntType<<RhsType as IntTypeTrait>::Signed>,
                 CanonicalValue = DynInt<<RhsType as IntTypeTrait>::Signed>,
             >,
-                Rhs: Value<Type = RhsType>,
-                Lhs: Value<Type = LhsType>,
-            > IntCmp<Rhs> for Expr<Lhs>
-        {
+            Rhs: ToExpr<Type = RhsType>,
+        > IntCmp<Rhs> for IntValue<LhsType> {
             type Output = Expr<UInt<1>>;
 
             $(fn $fn(self, rhs: Rhs) -> Self::Output {
-                self.$fn(rhs.to_expr())
-            })*
-        }
-
-        impl<
-            LhsType: IntTypeTrait<
-                CanonicalType = DynIntType<<LhsType as IntTypeTrait>::Signed>,
-                CanonicalValue = DynInt<<LhsType as IntTypeTrait>::Signed>,
-            >,
-            RhsType: IntTypeTrait<
-                Signed = LhsType::Signed,
-                CanonicalType = DynIntType<<RhsType as IntTypeTrait>::Signed>,
-                CanonicalValue = DynInt<<RhsType as IntTypeTrait>::Signed>,
-            >,
-            Rhs: Value<Type = RhsType>,
-        > IntCmp<Expr<Rhs>> for IntValue<LhsType> {
-            type Output = Expr<UInt<1>>;
-
-            $(fn $fn(self, rhs: Expr<Rhs>) -> Self::Output {
                 self.to_expr().$fn(rhs)
             })*
         }

crates/fayalite/src/int.rs

@@ -860,6 +860,47 @@ pub trait IntCmp<Rhs> {
     fn cmp_ge(self, rhs: Rhs) -> Self::Output;
 }
 
+macro_rules! forward_prim_int_cmp {
+    ($prim_ty:ident) => {
+        impl<Rhs> IntCmp<Rhs> for $prim_ty
+        where
+            IntValue<<$prim_ty as ToExpr>::Type>: IntCmp<Rhs>,
+        {
+            type Output = <IntValue<<$prim_ty as ToExpr>::Type> as IntCmp<Rhs>>::Output;
+            fn cmp_eq(self, rhs: Rhs) -> Self::Output {
+                IntValue::<<$prim_ty as ToExpr>::Type>::from(self).cmp_eq(rhs)
+            }
+            fn cmp_ne(self, rhs: Rhs) -> Self::Output {
+                IntValue::<<$prim_ty as ToExpr>::Type>::from(self).cmp_ne(rhs)
+            }
+            fn cmp_lt(self, rhs: Rhs) -> Self::Output {
+                IntValue::<<$prim_ty as ToExpr>::Type>::from(self).cmp_lt(rhs)
+            }
+            fn cmp_le(self, rhs: Rhs) -> Self::Output {
+                IntValue::<<$prim_ty as ToExpr>::Type>::from(self).cmp_le(rhs)
+            }
+            fn cmp_gt(self, rhs: Rhs) -> Self::Output {
+                IntValue::<<$prim_ty as ToExpr>::Type>::from(self).cmp_gt(rhs)
+            }
+            fn cmp_ge(self, rhs: Rhs) -> Self::Output {
+                IntValue::<<$prim_ty as ToExpr>::Type>::from(self).cmp_ge(rhs)
+            }
+        }
+    };
+}
+
+forward_prim_int_cmp!(bool);
+forward_prim_int_cmp!(u8);
+forward_prim_int_cmp!(u16);
+forward_prim_int_cmp!(u32);
+forward_prim_int_cmp!(u64);
+forward_prim_int_cmp!(u128);
+forward_prim_int_cmp!(i8);
+forward_prim_int_cmp!(i16);
+forward_prim_int_cmp!(i32);
+forward_prim_int_cmp!(i64);
+forward_prim_int_cmp!(i128);
+
 mod sealed {
     pub trait Sealed {}
 }

crates/fayalite/src/module.rs

@@ -11,7 +11,7 @@ use crate::{
         ops::VariantAccess, Expr, Flow, Target, TargetBase, TargetPathArrayElement,
         TargetPathBundleField, TargetPathElement, ToExpr,
     },
-    int::{DynUInt, DynUIntType, FixedOrDynIntType, UInt},
+    int::{DynUInt, DynUIntType, FixedOrDynIntType, IntValue, UInt},
     intern::{Intern, Interned},
     memory::{Mem, MemBuilder, MemBuilderTarget, PortName},
     reg::Reg,
@@ -2259,8 +2259,10 @@ where
     ) -> RegBuilder<'a, (), (), ()> {
         self.reg_builder_with_loc(implicit_name.0, SourceLocation::caller())
     }
+    // intentionally takes Expr instead of impl ToExpr to help prevent
+    // `#[hdl] if a == b {}` instead of using `cmp_eq`
     #[track_caller]
-    pub fn if_<Ty>(&mut self, cond: impl ToExpr<Type = Ty>) -> IfScope
+    pub fn if_<Ty>(&mut self, cond: Expr<IntValue<Ty>>) -> IfScope
     where
         Ty: FixedOrDynIntType<
             1,
@@ -2273,7 +2275,7 @@ where
     }
     pub fn if_with_loc<Ty>(
         &mut self,
-        cond: impl ToExpr<Type = Ty>,
+        cond: Expr<IntValue<Ty>>,
         source_location: SourceLocation,
     ) -> IfScope
     where
@@ -2284,7 +2286,7 @@ where
             CanonicalValue = DynUInt,
         >,
     {
-        let cond = cond.to_expr().as_bool();
+        let cond = cond.as_bool();
         let outer_block = self.block_stack.top();
         let then_block = self.new_block();
         let else_block = self.new_block();