Add error enum (#44)

* initial attempts at errors

* fill in errors

* bump ver

* fix graph feat

Author: matth2k

Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "safety-net"
-version = "0.1.7"
+version = "0.2.0"
 edition = "2024"
 license = "MIT OR Apache-2.0"
 
@@ -38,6 +38,7 @@ readme = "README.md"
 repository = "https://github.com/matth2k/safety-net"
 
 [dependencies]
+thiserror = { version = "2.0.16" }
 bitvec = { version = "1.0.1" }
 petgraph = { version = "0.8.2", optional = true }
 serde = { version = "1.0.219", optional = true, features = ["derive"] }

src/error.rs

@@ -0,0 +1,41 @@
+/*!
+
+  Error types.
+
+*/
+
+use thiserror::Error;
+
+use crate::circuit::{Identifier, Net};
+
+/// Errors for the `safety-net` library.
+#[derive(Error, Debug)]
+pub enum Error {
+    /// Error for an analysis cannot run due to  cycles.
+    #[error("Cycles detected along nets {0:?}")]
+    CycleDetected(Vec<Net>),
+    /// Errors in parsing literals/identifiers.
+    #[error("Parsing error `{0}`")]
+    ParseError(String),
+    /// The labeled nets in the netlist are not unique.
+    #[error("Non-unique nets: {0:?}")]
+    NonuniqueNets(Vec<Net>),
+    /// The labeled instances in the netlist are not unique.
+    #[error("Non-unique instances: {0:?}")]
+    NonuniqueInsts(Vec<Identifier>),
+    /// The netlist has no outputs.
+    #[error("No outputs in netlist")]
+    NoOutputs,
+    /// A deletion would cause a dangling reference.
+    #[error("Attempted to create a dangling reference to nets {0:?}")]
+    DanglingReference(Vec<Net>),
+    /// Mismatch in number of arguments
+    #[error("Expected {0} arguments, got {1}")]
+    ArgumentMismatch(usize, usize),
+    /// An input needs an alias to be an output
+    #[error("Input net {0} needs an alias to be an output")]
+    InputNeedsAlias(Net),
+    /// A net that was expected but not found
+    #[error("Expected to find net {0} in netlist")]
+    NetNotFound(Net),
+}

src/graph.rs

@@ -5,6 +5,7 @@
 */
 
 use crate::circuit::{Instantiable, Net};
+use crate::error::Error;
 #[cfg(feature = "graph")]
 use crate::netlist::Connection;
 use crate::netlist::iter::DFSIterator;
@@ -21,7 +22,7 @@ where
     Self: Sized + 'a,
 {
     /// Construct the analysis to the current state of the netlist.
-    fn build(netlist: &'a Netlist<I>) -> Result<Self, String>;
+    fn build(netlist: &'a Netlist<I>) -> Result<Self, Error>;
 }
 
 /// A table that maps nets to the circuit nodes they drive
@@ -68,7 +69,7 @@ impl<'a, I> Analysis<'a, I> for FanOutTable<'a, I>
 where
     I: Instantiable,
 {
-    fn build(netlist: &'a Netlist<I>) -> Result<Self, String> {
+    fn build(netlist: &'a Netlist<I>) -> Result<Self, Error> {
         let mut net_fan_out: HashMap<Net, Vec<NetRef<I>>> = HashMap::new();
         let mut node_fan_out: HashMap<NetRef<I>, Vec<NetRef<I>>> = HashMap::new();
         let mut is_an_output: HashSet<Net> = HashSet::new();
@@ -140,15 +141,15 @@ impl<'a, I> Analysis<'a, I> for SimpleCombDepth<'a, I>
 where
     I: Instantiable,
 {
-    fn build(netlist: &'a Netlist<I>) -> Result<Self, String> {
+    fn build(netlist: &'a Netlist<I>) -> Result<Self, Error> {
         let mut comb_depth: HashMap<NetRef<I>, usize> = HashMap::new();
 
         let mut nodes = Vec::new();
         for (driven, _) in netlist.outputs() {
-            let mut dfs = DFSIterator::new(netlist, driven.unwrap());
+            let mut dfs = DFSIterator::new(netlist, driven.clone().unwrap());
             while let Some(n) = dfs.next() {
                 if dfs.check_cycles() {
-                    return Err("Cycle detected in the netlist".to_string());
+                    return Err(Error::CycleDetected(vec![driven.as_net().clone()]));
                 }
                 nodes.push(n);
             }
@@ -252,7 +253,7 @@ impl<'a, I> Analysis<'a, I> for MultiDiGraph<'a, I>
 where
     I: Instantiable,
 {
-    fn build(netlist: &'a Netlist<I>) -> Result<Self, String> {
+    fn build(netlist: &'a Netlist<I>) -> Result<Self, Error> {
         // If we verify, we can hash by name
         netlist.verify()?;
         let mut mapping = HashMap::new();

src/lib.rs

@@ -14,6 +14,7 @@ Take a look at some [examples](https://github.com/matth2k/safety-net/tree/main/e
 
 pub mod attribute;
 pub mod circuit;
+pub mod error;
 pub mod graph;
 pub mod logic;
 pub mod netlist;

src/logic.rs

@@ -6,6 +6,8 @@
 
 use std::{fmt, str::FromStr};
 
+use crate::error::Error;
+
 #[derive(Debug, Clone, PartialEq, Eq, Copy)]
 /// An enum to represent four-state logic
 pub enum Logic {
@@ -114,15 +116,15 @@ impl From<bool> for Logic {
 }
 
 impl FromStr for Logic {
-    type Err = String;
+    type Err = Error;
 
     fn from_str(s: &str) -> Result<Self, Self::Err> {
         match s {
             "1'b1" | "1'h1" => Ok(Logic::True),
             "1'b0" | "1'h0" => Ok(Logic::False),
             "1'bx" | "1'hx" => Ok(Logic::X),
             "1'bz" | "1'hz" => Ok(Logic::Z),
-            _ => Err(format!("Invalid logic value: {s}")),
+            _ => Err(Error::ParseError(s.to_string())),
         }
     }
 }

src/netlist.rs

@@ -7,6 +7,7 @@
 use crate::{
     attribute::{Attribute, AttributeKey, AttributeValue, Parameter},
     circuit::{Identifier, Instantiable, Net, Object},
+    error::Error,
     graph::{Analysis, FanOutTable},
     logic::Logic,
 };
@@ -614,7 +615,7 @@ where
     ///
     /// Panics if cell is a multi-output circuit node.
     /// Panics if the reference to the netlist is lost.
-    pub fn expose_as_output(self) -> Result<Self, String> {
+    pub fn expose_as_output(self) -> Result<Self, Error> {
         let netlist = self
             .netref
             .borrow()
@@ -647,17 +648,18 @@ where
     ///
     /// # Panics
     /// Panics if the reference to the netlist is lost.
-    pub fn expose_net(&self, net: &Net) -> Result<(), String> {
+    pub fn expose_net(&self, net: &Net) -> Result<(), Error> {
         let netlist = self
             .netref
             .borrow()
             .owner
             .upgrade()
             .expect("NetRef is unlinked from netlist");
-        let net_index = self.netref.borrow().find_net(net).ok_or(format!(
-            "Net {} not found in circuit node",
-            net.get_identifier()
-        ))?;
+        let net_index = self
+            .netref
+            .borrow()
+            .find_net(net)
+            .ok_or(Error::NetNotFound(net.clone()))?;
         let dr = DrivenNet::new(net_index, self.clone());
         netlist.expose_net(dr)?;
         Ok(())
@@ -782,7 +784,7 @@ where
     /// # Panics
     ///
     /// Panics if the reference to the netlist is lost.
-    pub fn delete_uses(self) -> Result<Object<I>, String> {
+    pub fn delete_uses(self) -> Result<Object<I>, Error> {
         let netlist = self
             .netref
             .borrow()
@@ -798,7 +800,7 @@ where
     ///
     /// Panics if either `self` or `other` is a multi-output circuit node.
     /// Panics if the weak reference to the netlist is lost.
-    pub fn replace_uses_with(self, other: &Self) -> Result<Object<I>, String> {
+    pub fn replace_uses_with(self, other: &Self) -> Result<Object<I>, Error> {
         let netlist = self
             .netref
             .borrow()
@@ -1157,7 +1159,7 @@ where
         self: &Rc<Self>,
         object: Object<I>,
         operands: &[DrivenNet<I>],
-    ) -> Result<NetRef<I>, String> {
+    ) -> Result<NetRef<I>, Error> {
         let index = self.objects.borrow().len();
         let weak = Rc::downgrade(self);
         let operands = operands
@@ -1199,19 +1201,15 @@ where
         inst_type: I,
         inst_name: Identifier,
         operands: &[DrivenNet<I>],
-    ) -> Result<NetRef<I>, String> {
+    ) -> Result<NetRef<I>, Error> {
         let nets = inst_type
             .get_output_ports()
             .into_iter()
             .map(|pnet| pnet.with_name(&inst_name + pnet.get_identifier()))
             .collect::<Vec<_>>();
         let input_count = inst_type.get_input_ports().into_iter().count();
         if operands.len() != input_count {
-            return Err(format!(
-                "Expected {} operands, got {}",
-                input_count,
-                operands.len()
-            ));
+            return Err(Error::ArgumentMismatch(input_count, operands.len()));
         }
         let obj = Object::Instance(nets, inst_name, inst_type);
         self.insert_object(obj, operands)
@@ -1282,22 +1280,20 @@ where
     }
 
     /// Set an added object as a top-level output.
-    pub fn expose_net(&self, net: DrivenNet<I>) -> Result<DrivenNet<I>, String> {
+    pub fn expose_net(&self, net: DrivenNet<I>) -> Result<DrivenNet<I>, Error> {
         if net.is_an_input() {
-            return Err(
-                "Cannot expose an input net as output without a new name to bind to".to_string(),
-            );
+            return Err(Error::InputNeedsAlias(net.as_net().clone()));
         }
         let mut outputs = self.outputs.borrow_mut();
         outputs.insert(net.get_operand(), net.as_net().clone());
         Ok(net)
     }
 
     /// Unlink a circuit node from the rest of the netlist. Return the object that was being stored.
-    pub fn delete_net_uses(&self, netref: NetRef<I>) -> Result<Object<I>, String> {
+    pub fn delete_net_uses(&self, netref: NetRef<I>) -> Result<Object<I>, Error> {
         let unwrapped = netref.clone().unwrap();
         if Rc::strong_count(&unwrapped) > 3 {
-            return Err("Cannot delete. References still exist on this node".to_string());
+            return Err(Error::DanglingReference(netref.nets().collect()));
         }
         let old_index = unwrapped.borrow().get_index();
         let objects = self.objects.borrow();
@@ -1336,10 +1332,10 @@ where
 
     /// Replaces the uses of a circuit node with another circuit node. The [Object] stored at `of` is returned.
     /// Panics if `of` and  `with` are not single-output nodes.
-    pub fn replace_net_uses(&self, of: NetRef<I>, with: &NetRef<I>) -> Result<Object<I>, String> {
+    pub fn replace_net_uses(&self, of: NetRef<I>, with: &NetRef<I>) -> Result<Object<I>, Error> {
         let unwrapped = of.clone().unwrap();
         if Rc::strong_count(&unwrapped) > 3 {
-            return Err("Cannot replace. References still exist on this node".to_string());
+            return Err(Error::DanglingReference(of.nets().collect()));
         }
 
         let old_tag: DrivenNet<I> = of.clone().into();
@@ -1405,7 +1401,7 @@ where
     }
 
     /// Constructs an analysis of the netlist.
-    pub fn get_analysis<'a, A: Analysis<'a, I>>(&'a self) -> Result<A, String> {
+    pub fn get_analysis<'a, A: Analysis<'a, I>>(&'a self) -> Result<A, Error> {
         A::build(self)
     }
 
@@ -1450,7 +1446,7 @@ where
     }
 
     /// Cleans unused nodes from the netlist, returning `Ok(true)` if the netlist changed.
-    pub fn clean_once(&self) -> Result<bool, String> {
+    pub fn clean_once(&self) -> Result<bool, Error> {
         let mut dead_objs = HashSet::new();
         {
             let fan_out = self.get_analysis::<FanOutTable<I>>()?;
@@ -1477,11 +1473,10 @@ where
         let mut remap: HashMap<usize, usize> = HashMap::new();
         for (old_index, obj) in old_objects.into_iter().enumerate() {
             if dead_objs.contains(&old_index) {
+                // 1. this ref, 2. as an output
                 if Rc::strong_count(&obj) > 2 {
-                    return Err(format!(
-                        "Cannot delete object {} as a NetRef still exists, or it is an output. SC = {}",
-                        obj.borrow().get(),
-                        Rc::strong_count(&obj)
+                    return Err(Error::DanglingReference(
+                        obj.borrow().get().get_nets().to_vec(),
                     ));
                 }
                 continue;
@@ -1513,7 +1508,7 @@ where
 
     /// Greedly removes unused nodes from the netlist, until it stops changing.
     /// Returns true if the netlist was changed.
-    pub fn clean(&self) -> Result<bool, String> {
+    pub fn clean(&self) -> Result<bool, Error> {
         if !self.clean_once()? {
             Ok(false)
         } else {
@@ -1526,42 +1521,39 @@ where
     }
 
     /// Returns `true` if all the nets are uniquely named
-    fn nets_unique(&self) -> bool {
+    fn nets_unique(&self) -> Result<(), Error> {
         let mut nets = HashSet::new();
         for net in self.into_iter() {
-            if !nets.insert(net.take_identifier()) {
-                return false;
+            if !nets.insert(net.clone().take_identifier()) {
+                return Err(Error::NonuniqueNets(vec![net]));
             }
         }
-        true
+        Ok(())
     }
 
     /// Returns `true` if all the nets are uniquely named
-    fn insts_unique(&self) -> bool {
+    fn insts_unique(&self) -> Result<(), Error> {
         let mut insts = HashSet::new();
         for inst in self.objects() {
             if let Some(name) = inst.get_instance_name()
-                && !insts.insert(name)
+                && !insts.insert(name.clone())
             {
-                return false;
+                return Err(Error::NonuniqueInsts(vec![name]));
             }
         }
-        true
+        Ok(())
     }
 
     /// Verifies that a netlist is well-formed.
-    pub fn verify(&self) -> Result<(), String> {
+    pub fn verify(&self) -> Result<(), Error> {
         if self.outputs.borrow().is_empty() {
-            return Err("Netlist has no outputs".to_string());
+            return Err(Error::NoOutputs);
         }
 
-        if !self.nets_unique() {
-            return Err("Netlist contains non-unique nets (multiple drivers)".to_string());
-        }
+        self.nets_unique()?;
+
+        self.insts_unique()?;
 
-        if !self.insts_unique() {
-            return Err("Netlist contains non-unique instances".to_string());
-        }
         Ok(())
     }
 }