Merge pull request #209 from crypdoughdoteth/main

"improve: error msgs and handling"

book/src/modules.md

@@ -293,4 +293,4 @@ If we have a name resolution phase, we could do this:
 
 - fully qualify all things that need to be fully qualified: structs, functions, methods (which are defined as function currently, should we not do that?), consts. And that's it?
 - create a `Hashmap<usize, String>` to store all the filenames
-- add the `usize` in all `Span` 
+- add the `usize` in all `Span` 

src/backends/kimchi/builtin.rs

@@ -8,7 +8,6 @@ use kimchi::mina_poseidon::permutation::full_round;
 use super::{KimchiCellVar, KimchiVesta, VestaField};
 use crate::backends::kimchi::NUM_REGISTERS;
 use crate::backends::Backend;
-
 use crate::parser::types::GenericParameters;
 use crate::{
     circuit_writer::{CircuitWriter, GateKind, VarInfo},
@@ -37,7 +36,12 @@ pub fn poseidon(
         Some(TyKind::Array(el_typ, 2)) => {
             assert!(matches!(&**el_typ, TyKind::Field { .. }));
         }
-        _ => panic!("wrong type for input to poseidon"),
+        _ => Err(compiler.error(
+            ErrorKind::UnexpectedError(
+                "incorrect type used as input to the builtin poseidon hash function",
+            ),
+            span,
+        ))?,
     };
 
     // extract the values
@@ -46,10 +50,10 @@ pub fn poseidon(
 
     // hashing a full-constant input is not a good idea
     if input[0].is_const() && input[1].is_const() {
-        return Err(compiler.error(
+        Err(compiler.error(
             ErrorKind::UnexpectedError("cannot hash a full-constant input"),
             span,
-        ));
+        ))?
     }
 
     // IMPORTANT: time to constrain any constants

src/backends/kimchi/mod.rs

@@ -343,16 +343,22 @@ impl Backend for KimchiVesta {
                         ErrorKind::PrivateInputNotUsed,
                         private_cell_var.span,
                     );
-                    return Err(err);
+                    Err(err)?;
                 } else {
-                    panic!("there's a bug in the circuit_writer, some cellvar does not end up being a cellvar in the circuit!");
+                    Err(Error::new("contraint-finalization", ErrorKind::UnexpectedError("there's a bug in the circuit_writer, some cellvar does not end up being a cellvar in the circuit!"), Span::default()))?;
                 }
             }
         }
 
         // kimchi hack
         if self.gates.len() <= 2 {
-            panic!("the circuit is either too small or does not constrain anything (TODO: better error)");
+            Err(Error::new(
+                "contraint-finalization",
+                ErrorKind::UnexpectedError(
+                    "The circuit is either too small or does not constrain anything, too few gates create in the R1CS",
+                ),
+                Span::default(),
+            ))?;
         }
 
         // store the return value in the public input that was created for that ^

src/backends/r1cs/mod.rs

@@ -400,14 +400,17 @@ where
                     .iter()
                     .find(|private_cell_var| private_cell_var.index == index)
                 {
-                    let err = Error::new(
+                    Err(Error::new(
                         "constraint-finalization",
                         ErrorKind::PrivateInputNotUsed,
                         private_cell_var.span,
-                    );
-                    return Err(err);
+                    ))?
                 } else {
-                    panic!("there's a bug in the circuit_writer, some cellvar does not end up being a cellvar in the circuit!");
+                    Err(Error::new(
+                        "constraint-finalization",
+                        ErrorKind::UnexpectedError("there's a bug in the circuit_writer, some cellvar does not end up being a cellvar in the circuit!"),
+                        Span::default(),
+                    ))?
                 }
             }
         }

src/backends/r1cs/snarkjs.rs

@@ -1,11 +1,11 @@
 use crate::backends::BackendField;
 use constraint_writers::r1cs_writer::{ConstraintSection, HeaderData, R1CSWriter};
-use miette::{miette, Diagnostic};
+use miette::Diagnostic;
 use thiserror::Error;
 
 use std::collections::HashMap;
 use std::fs::{File, OpenOptions};
-use std::io::{self, BufWriter, Seek, SeekFrom, Write};
+use std::io::{self, BufWriter, ErrorKind, Seek, SeekFrom, Write};
 use std::vec;
 
 use super::{GeneratedWitness, LinearCombination, R1CS};
@@ -39,16 +39,20 @@ struct SnarkjsLinearCombination {
 }
 
 impl SnarkjsLinearCombination {
-    fn to_hashmap(&self) -> HashMap<usize, BigInt> {
+    fn try_hashmap(&self) -> Result<HashMap<usize, BigInt>, Error> {
         let mut terms = self.terms.clone();
 
         // add the constant term with var indexed at 0
         if terms.insert(0, self.constant.clone()).is_some() {
             // sanity check
-            panic!("The first var should be preserved for constant term");
+
+            Err(std::io::Error::new(
+                ErrorKind::Other,
+                "The first var should be preserved for constant term",
+            ))?
         }
 
-        terms
+        Ok(terms)
     }
 }
 
@@ -134,9 +138,9 @@ where
         for constraint in &restructure_constraints {
             ConstraintSection::write_constraint_usize(
                 &mut constraint_section,
-                &constraint.a.to_hashmap(),
-                &constraint.b.to_hashmap(),
-                &constraint.c.to_hashmap(),
+                &constraint.a.try_hashmap()?,
+                &constraint.b.try_hashmap()?,
+                &constraint.c.try_hashmap()?,
             )
             .map_err(|_| Error::R1CSWriterIo)?;
         }
@@ -227,7 +231,10 @@ impl WitnessWriter {
         // Write the file type (magic string) as bytes
         let file_type_bytes = file_type.as_bytes();
         if file_type_bytes.len() != 4 {
-            panic!("File type must be 4 characters long");
+            Err(std::io::Error::new(
+                ErrorKind::Other,
+                "File type must be 4 characters long",
+            ))?
         }
         writer.write_all(file_type_bytes)?;
 

src/circuit_writer/mod.rs

@@ -82,17 +82,13 @@ impl<B: Backend> CircuitWriter<B> {
         fn_env: &mut FnEnv<B::Field, B::Var>,
         var_name: String,
         var_info: VarInfo<B::Field, B::Var>,
-    ) {
+    ) -> Result<()> {
         // check for consts first
         let qualified = FullyQualified::local(var_name.clone());
         if let Some(_cst_info) = self.typed.const_info(&qualified) {
-            panic!(
-                "type checker bug: we already have a constant with the same name (`{var_name}`)!"
-            );
+            Err(Error::new("add-local-var", ErrorKind::UnexpectedError("type checker bug: we already have a constant with the same name (`{var_name}`)!"), Span::default()))?
         }
-
-        //
-        fn_env.add_local_var(var_name, var_info)
+        Ok(fn_env.add_local_var(var_name, var_info))
     }
 
     pub fn get_local_var(
@@ -172,7 +168,11 @@ impl<B: Backend> CircuitWriter<B> {
                         );
                     }
                 }
-                None => panic!("public arguments must have a pub attribute"),
+                None => Err(Error::new(
+                    "generate-circuit",
+                    ErrorKind::UnexpectedError("public arguments must have a pub attribute"),
+                    Span::default(),
+                ))?,
             }
             circuit_writer.handle_arg(arg, fn_env, CircuitWriter::add_public_inputs)?;
         }
@@ -247,7 +247,7 @@ impl<B: Backend> CircuitWriter<B> {
         // add argument variable to the ast env
         let mutable = false; // TODO: should we add a mut keyword in arguments as well?
         let var_info = VarInfo::new(var, mutable, Some(typ.kind.clone()));
-        self.add_local_var(fn_env, name.value.clone(), var_info);
+        self.add_local_var(fn_env, name.value.clone(), var_info)?;
 
         Ok(())
     }

src/circuit_writer/writer.rs

@@ -10,7 +10,7 @@ use crate::{
     circuit_writer::{CircuitWriter, DebugInfo, FnEnv, VarInfo},
     constants::Span,
     constraints::{boolean, field},
-    error::{ErrorKind, Result},
+    error::{Error, ErrorKind, Result},
     imports::FnKind,
     parser::{
         types::{ForLoopArgument, FunctionDef, Stmt, StmtKind, TyKind},
@@ -133,7 +133,7 @@ impl<B: Backend> CircuitWriter<B> {
 
                 // store the new variable
                 // TODO: do we really need to store that in the scope? That's not an actual var in the scope that's an internal var...
-                self.add_local_var(fn_env, lhs.value.clone(), var_info);
+                self.add_local_var(fn_env, lhs.value.clone(), var_info)?;
             }
 
             StmtKind::ForLoop {
@@ -178,7 +178,7 @@ impl<B: Backend> CircuitWriter<B> {
                                 false,
                                 Some(TyKind::Field { constant: true }),
                             );
-                            self.add_local_var(fn_env, var.value.clone(), var_info);
+                            self.add_local_var(fn_env, var.value.clone(), var_info)?;
 
                             self.compile_block(fn_env, body)?;
 
@@ -197,7 +197,11 @@ impl<B: Backend> CircuitWriter<B> {
 
                         let (elem_type, array_len) = match array_typ {
                             TyKind::Array(ty, array_len) => (ty, array_len),
-                            _ => panic!("expected array"),
+                            _ => Err(Error::new(
+                                "compile-stmt",
+                                ErrorKind::UnexpectedError("expected array"),
+                                stmt.span,
+                            ))?,
                         };
 
                         // compute the size of each element in the array
@@ -215,7 +219,7 @@ impl<B: Backend> CircuitWriter<B> {
                             let indexed_var = iterator_var.narrow(start, len).value(self, fn_env);
                             let var_info =
                                 VarInfo::new(indexed_var.clone(), false, Some(*elem_type.clone()));
-                            self.add_local_var(fn_env, var.value.clone(), var_info);
+                            self.add_local_var(fn_env, var.value.clone(), var_info)?;
 
                             self.compile_block(fn_env, body)?;
 
@@ -280,7 +284,7 @@ impl<B: Backend> CircuitWriter<B> {
         assert_eq!(function.sig.arguments.len(), args.len());
 
         for (name, var_info) in function.sig.arguments.iter().zip(args) {
-            self.add_local_var(fn_env, name.name.value.clone(), var_info);
+            self.add_local_var(fn_env, name.name.value.clone(), var_info)?;
         }
 
         // compile it and potentially return a return value
@@ -354,7 +358,7 @@ impl<B: Backend> CircuitWriter<B> {
                     match r {
                         ConstOrCell::Cell(c) => returned_cells.push(c.clone()),
                         ConstOrCell::Const(_) => {
-                            return Err(self.error(ErrorKind::ConstantInOutput, returned.span))
+                            Err(self.error(ErrorKind::ConstantInOutput, returned.span))?
                         }
                     }
                 }
@@ -383,7 +387,7 @@ impl<B: Backend> CircuitWriter<B> {
             } => {
                 // sanity check
                 if fn_name.value == "main" {
-                    return Err(self.error(ErrorKind::RecursiveMain, expr.span));
+                    Err(self.error(ErrorKind::RecursiveMain, expr.span))?
                 }
 
                 // retrieve the function in the env
@@ -452,9 +456,13 @@ impl<B: Backend> CircuitWriter<B> {
 
                 let (module, self_struct) = match lhs_struct {
                     TyKind::Custom { module, name } => (module, name),
-                    _ => {
-                        panic!("could not figure out struct implementing that method call")
-                    }
+                    _ => Err(Error::new(
+                        "compute-expr",
+                        ErrorKind::UnexpectedError(
+                            "could not figure out struct implementing that method call",
+                        ),
+                        lhs.span,
+                    ))?,
                 };
 
                 let qualified = FullyQualified::new(module, self_struct);
@@ -596,7 +604,11 @@ impl<B: Backend> CircuitWriter<B> {
 
                 // replace the left with the right
                 match lhs {
-                    VarOrRef::Var(_) => panic!("can't reassign this non-mutable variable"),
+                    VarOrRef::Var(_) => Err(Error::new(
+                        "compute-expr",
+                        ErrorKind::UnexpectedError("can't reassign this non-mutable variable"),
+                        expr.span,
+                    ))?,
                     VarOrRef::Ref {
                         var_name,
                         start,
@@ -717,7 +729,11 @@ impl<B: Backend> CircuitWriter<B> {
                         }
                         ty
                     }
-                    _ => panic!("expected array"),
+                    _ => Err(Error::new(
+                        "compute-expr",
+                        ErrorKind::UnexpectedError("expected array"),
+                        expr.span,
+                    ))?,
                 };
 
                 // compute the size of each element in the array

src/cli/cmd_new_and_init.rs

@@ -1,5 +1,6 @@
 use camino::Utf8PathBuf as PathBuf;
-use miette::{IntoDiagnostic, Result, WrapErr};
+use miette::{IntoDiagnostic, MietteError, Result, WrapErr};
+use std::io::{Error, ErrorKind};
 
 const MAIN_CONTENT: &str = r#"fn main(pub xx: Field, yy: Field) {
     let zz = yy + 1;
@@ -78,7 +79,7 @@ pub fn cmd_init(args: CmdInit) -> Result<()> {
 }
 
 fn mk(path: PathBuf, package_name: &str, is_lib: bool) -> Result<()> {
-    let user = get_git_user();
+    let user = get_git_user()?;
 
     let content = format!(
         r#"[package]
@@ -136,20 +137,21 @@ dependencies = []
     Ok(())
 }
 
-fn get_git_user() -> String {
+fn get_git_user() -> Result<String> {
     let output = std::process::Command::new("git")
         .arg("config")
         .arg("user.name")
         .output()
-        .expect("failed to execute git command");
+        .map_err(|e| MietteError::from(e))?;
 
     if !output.status.success() {
-        panic!("failed to get git user name");
+        let err = Error::new(ErrorKind::Other, "failed to get git username");
+        Err(MietteError::IoError(err))?
     }
 
     let output = String::from_utf8(output.stdout).expect("couldn't parse git output as utf8");
 
     let username = output.trim().to_owned();
 
-    username.replace(" ", "_").to_lowercase()
+    Ok(username.replace(" ", "_").to_lowercase())
 }

src/error.rs

@@ -36,20 +36,18 @@ impl Error {
 /// The type of error.
 #[derive(Error, Diagnostic, Debug)]
 pub enum ErrorKind {
-    /// this error is for testing. You can use it when you want to quickly see in what file and what line of code the error is.
     #[error(
         "Unexpected error: {0}. Please report this error on https://github.com/mimoo/noname/issues"
     )]
     UnexpectedError(&'static str),
-
     #[error("variable is not mutable. You must set the `mut` keyword to make it mutable")]
     AssignmentToImmutableVariable,
-
+    #[error("the {0} of assert_eq must be of type Field or BigInt. It was of type {1}")]
+    AssertTypeMismatch(&'static str, TyKind),
     #[error(
         "the dependency `{0}` does not appear to be listed in your manifest file `Noname.toml`"
     )]
     UnknownDependency(String),
-
     #[error("the function `{1}` does not exist in the module `{0}`")]
     UnknownExternalFn(String, String),