Remove unwraps (#393)

* remove unwraps * remove unneeded expect * Update src/libfuncs/felt252.rs Co-authored-by: MrAzteca <[email protected]> --------- Co-authored-by: MrAzteca <[email protected]>
lambdaclass · Dec 19, 2023 · 9e06501 · 9e06501
1 parent 57c04b3
commit 9e06501
Show file tree

Hide file tree

Showing 23 changed files with 162 additions and 95 deletions.
diff --git a/src/cache.rs b/src/cache.rs
@@ -46,9 +46,9 @@ impl<'a, K: Clone + PartialEq + Eq + Hash> ProgramCache<'a, K> {
     ) -> Rc<RefCell<NativeExecutor<'a>>> {
         let module = self.context.compile(program).expect("should compile");
         let executor = NativeExecutor::new(module);
-        self.cache
-            .insert(key.clone(), Rc::new(RefCell::new(executor)));
-        self.cache.get_mut(&key).cloned().unwrap()
+        let executor = Rc::new(RefCell::new(executor));
+        self.cache.insert(key.clone(), executor.clone());
+        executor
     }
 }
 

diff --git a/src/compiler.rs b/src/compiler.rs
@@ -314,7 +314,7 @@ where
 
                             metadata
                                 .insert(TailRecursionMeta::new(op0.result(0)?.into(), &entry_block))
-                                .unwrap();
+                                .expect("should not have this metadata inserted yet");
                         }
                     }
 
@@ -533,7 +533,9 @@ where
                 if let std::collections::btree_map::Entry::Vacant(e) = blocks.entry(statement_idx.0)
                 {
                     e.insert(Block::new(&[]));
-                    blocks.get_mut(&statement_idx.0).unwrap()
+                    blocks
+                        .get_mut(&statement_idx.0)
+                        .expect("the block should exist")
                 } else {
                     panic!("statement index already present in block");
                 }

diff --git a/src/error/libfuncs.rs b/src/error/libfuncs.rs
@@ -49,6 +49,10 @@ pub enum ErrorImpl {
     ProgramRegistryError(#[from] Box<ProgramRegistryError>),
     #[error(transparent)]
     TryFromIntError(#[from] TryFromIntError),
+    #[error("error parsing attribute")]
+    ParseAttributeError,
+    #[error("missing metadata")]
+    MissingMetadata,
 }
 
 impl From<super::CoreTypeBuilderError> for ErrorImpl {

diff --git a/src/jit_runner.rs b/src/jit_runner.rs
@@ -334,7 +334,10 @@ pub fn execute(
 
                         params_ptrs.push(
                             arena
-                                .alloc(NonNull::new(syscall_addr as *mut ()).unwrap())
+                                .alloc(
+                                    NonNull::new(syscall_addr as *mut ())
+                                        .expect("the ptr is not null"),
+                                )
                                 .cast(),
                         );
                     }
@@ -392,7 +395,8 @@ pub fn execute(
     for (type_id, offset) in entry_point.signature.ret_types.iter().zip(offsets) {
         let ty = registry.get_type(type_id)?;
 
-        let ptr = NonNull::new(((ret_ptr.as_ptr() as usize) + offset) as *mut _).unwrap();
+        let ptr = NonNull::new(((ret_ptr.as_ptr() as usize) + offset) as *mut _)
+            .expect("the ptr is not null");
 
         match ty {
             CoreTypeConcrete::GasBuiltin(_) => {

diff --git a/src/libfuncs.rs b/src/libfuncs.rs
@@ -402,7 +402,7 @@ where
         default: (BranchTarget<'ctx, '_>, &[Value<'ctx, 'this>]),
         branches: &[(i64, BranchTarget<'ctx, '_>, &[Value<'ctx, 'this>])],
         location: Location<'ctx>,
-    ) -> Operation<'ctx> {
+    ) -> Result<Operation<'ctx>, CoreLibfuncBuilderError> {
         let default_destination = match default.0 {
             BranchTarget::Jump(x) => (x, Cow::Borrowed(default.1)),
             BranchTarget::Return(i) => {
@@ -453,7 +453,7 @@ where
             });
         }
 
-        cf::switch(
+        Ok(cf::switch(
             context,
             &case_values,
             flag,
@@ -464,8 +464,7 @@ where
                 .map(|(x, y)| (*x, y.as_ref()))
                 .collect::<Vec<_>>(),
             location,
-        )
-        .unwrap()
+        )?)
     }
 }
 

diff --git a/src/libfuncs/bool.rs b/src/libfuncs/bool.rs
@@ -3,7 +3,7 @@
 use super::{LibfuncBuilder, LibfuncHelper};
 use crate::{
     error::{
-        libfuncs::{Error, Result},
+        libfuncs::{Error, ErrorImpl, Result},
         CoreTypeBuilderError,
     },
     metadata::MetadataStorage,
@@ -107,7 +107,7 @@ where
     let enum_ty = registry.get_type(&info.param_signatures()[0].ty)?;
     let tag_bits = enum_ty
         .variants()
-        .unwrap()
+        .expect("bool is a enum and has variants")
         .len()
         .next_power_of_two()
         .trailing_zeros();
@@ -172,7 +172,7 @@ where
     let enum_ty = registry.get_type(&info.param_signatures()[0].ty)?;
     let tag_bits = enum_ty
         .variants()
-        .unwrap()
+        .expect("bool is a enum and has variants")
         .len()
         .next_power_of_two()
         .trailing_zeros();
@@ -191,7 +191,8 @@ where
 
     let op = entry.append_operation(arith::constant(
         context,
-        Attribute::parse(context, &format!("1 : {tag_ty}")).unwrap(),
+        Attribute::parse(context, &format!("1 : {tag_ty}"))
+            .ok_or(ErrorImpl::ParseAttributeError)?,
         location,
     ));
     let const_1 = op.result(0)?.into();
@@ -239,7 +240,7 @@ where
 
     let tag_bits = enum_ty
         .variants()
-        .unwrap()
+        .expect("bool is a enum and has variants")
         .len()
         .next_power_of_two()
         .trailing_zeros();

diff --git a/src/libfuncs/cast.rs b/src/libfuncs/cast.rs
@@ -165,8 +165,12 @@ where
     let src_ty = registry.get_type(&info.param_signatures()[0].ty)?;
     let dst_ty = registry.get_type(&info.branch_signatures()[0].vars[0].ty)?;
 
-    let src_width = src_ty.integer_width().unwrap();
-    let dst_width = dst_ty.integer_width().unwrap();
+    let src_width = src_ty
+        .integer_width()
+        .expect("casts always happen between numerical types");
+    let dst_width = dst_ty
+        .integer_width()
+        .expect("casts always happen between numerical types");
     assert!(src_width <= dst_width);
 
     let result = if src_width == dst_width {

diff --git a/src/libfuncs/ec.rs b/src/libfuncs/ec.rs
@@ -5,7 +5,7 @@
 use super::{LibfuncBuilder, LibfuncHelper};
 use crate::{
     error::{
-        libfuncs::{Error, Result},
+        libfuncs::{Error, ErrorImpl, Result},
         CoreTypeBuilderError,
     },
     metadata::{
@@ -202,7 +202,7 @@ where
                     }
                 ),
             )
-            .unwrap(),
+            .ok_or(ErrorImpl::ParseAttributeError)?,
             location,
         ))
         .result(0)?
@@ -326,7 +326,7 @@ where
 
     let result = metadata
         .get_mut::<RuntimeBindingsMeta>()
-        .unwrap()
+        .ok_or(ErrorImpl::MissingMetadata)?
         .libfunc_ec_point_from_x_nz(context, helper, entry, point_ptr, location)?
         .result(0)?
         .into();
@@ -451,7 +451,7 @@ where
 
     metadata
         .get_mut::<RuntimeBindingsMeta>()
-        .unwrap()
+        .ok_or(ErrorImpl::MissingMetadata)?
         .libfunc_ec_state_add(context, helper, entry, state_ptr, point_ptr, location)?;
 
     let state = entry
@@ -578,7 +578,7 @@ where
 
     metadata
         .get_mut::<RuntimeBindingsMeta>()
-        .unwrap()
+        .ok_or(ErrorImpl::MissingMetadata)?
         .libfunc_ec_state_add_mul(
             context, helper, entry, state_ptr, scalar_ptr, point_ptr, location,
         )?;
@@ -676,7 +676,7 @@ where
 
     let is_zero = metadata
         .get_mut::<RuntimeBindingsMeta>()
-        .unwrap()
+        .ok_or(ErrorImpl::MissingMetadata)?
         .libfunc_ec_state_try_finalize_nz(context, helper, entry, point_ptr, state_ptr, location)?
         .result(0)?
         .into();
@@ -731,15 +731,15 @@ where
     let x = entry
         .append_operation(arith::constant(
             context,
-            Attribute::parse(context, "3151312365169595090315724863753927489909436624354740709748557281394568342450 : i252").unwrap(),
+            Attribute::parse(context, "3151312365169595090315724863753927489909436624354740709748557281394568342450 : i252").ok_or(ErrorImpl::ParseAttributeError)?,
             location,
         ))
         .result(0)?
         .into();
     let y = entry
         .append_operation(arith::constant(
             context,
-            Attribute::parse(context, "2835232394579952276045648147338966184268723952674536708929458753792035266179 : i252").unwrap(),
+            Attribute::parse(context, "2835232394579952276045648147338966184268723952674536708929458753792035266179 : i252").ok_or(ErrorImpl::ParseAttributeError)?,
             location,
         ))
         .result(0)?
@@ -876,7 +876,7 @@ where
 
     let result = metadata
         .get_mut::<RuntimeBindingsMeta>()
-        .unwrap()
+        .ok_or(ErrorImpl::MissingMetadata)?
         .libfunc_ec_point_try_new_nz(context, helper, entry, point_ptr, location)?
         .result(0)?
         .into();

diff --git a/src/libfuncs/enum.rs b/src/libfuncs/enum.rs
@@ -5,7 +5,7 @@
 use super::{LibfuncBuilder, LibfuncHelper};
 use crate::{
     error::{
-        libfuncs::{Error, Result},
+        libfuncs::{Error, ErrorImpl, Result},
         CoreTypeBuilderError,
     },
     metadata::{enum_snapshot_variants::EnumSnapshotVariantsMeta, MetadataStorage},
@@ -88,7 +88,7 @@ where
         registry
             .get_type(&info.branch_signatures()[0].vars[0].ty)?
             .variants()
-            .unwrap(),
+            .expect("enum should always have variants"),
     )?;
 
     let enum_ty = registry.build_type(
@@ -211,7 +211,7 @@ where
         registry
             .get_type(&info.param_signatures()[0].ty)?
             .variants()
-            .unwrap(),
+            .expect("enum should always have variants"),
     )?;
 
     let enum_ty = registry.build_type(
@@ -363,9 +363,9 @@ where
     // This libfunc's implementation is identical to `enum_match` aside from fetching the snapshotted enum's variants from the metadata:
     let variants = metadata
         .get::<EnumSnapshotVariantsMeta>()
-        .unwrap()
+        .ok_or(ErrorImpl::MissingMetadata)?
         .get_variants(&info.param_signatures()[0].ty)
-        .unwrap()
+        .expect("enum should always have variants")
         .clone();
     let (layout, (tag_ty, tag_layout), variant_tys) = crate::types::r#enum::get_type_for_variants(
         context, helper, registry, metadata, &variants,

diff --git a/src/libfuncs/felt252.rs b/src/libfuncs/felt252.rs
@@ -3,7 +3,7 @@
 use super::{LibfuncBuilder, LibfuncHelper};
 use crate::{
     error::{
-        libfuncs::{Error, Result},
+        libfuncs::{Error, ErrorImpl, Result},
         CoreTypeBuilderError,
     },
     metadata::{prime_modulo::PrimeModuloMeta, MetadataStorage},
@@ -98,18 +98,24 @@ where
         context,
         &format!(
             "{} : {i256}",
-            metadata.get::<PrimeModuloMeta<Felt252>>().unwrap().prime()
+            metadata
+                .get::<PrimeModuloMeta<Felt252>>()
+                .ok_or(ErrorImpl::MissingMetadata)?
+                .prime()
         ),
     )
-    .unwrap();
+    .ok_or(ErrorImpl::ParseAttributeError)?;
     let attr_prime_i512 = Attribute::parse(
         context,
         &format!(
             "{} : {i512}",
-            metadata.get::<PrimeModuloMeta<Felt252>>().unwrap().prime()
+            metadata
+                .get::<PrimeModuloMeta<Felt252>>()
+                .ok_or(ErrorImpl::MissingMetadata)?
+                .prime()
         ),
     )
-    .unwrap();
+    .ok_or(ErrorImpl::ParseAttributeError)?;
 
     let attr_cmp_uge = IntegerAttribute::new(
         CmpiPredicate::Uge as i64,
@@ -131,15 +137,19 @@ where
         Felt252BinaryOperationConcrete::WithConst(operation) => {
             let value = match operation.c.sign() {
                 Sign::Minus => {
-                    let prime = metadata.get::<PrimeModuloMeta<Felt252>>().unwrap().prime();
-                    (&operation.c + prime.to_bigint().unwrap())
+                    let prime = metadata
+                        .get::<PrimeModuloMeta<Felt252>>()
+                        .ok_or(ErrorImpl::MissingMetadata)?
+                        .prime();
+                    (&operation.c + prime.to_bigint().expect("always is Some"))
                         .to_biguint()
-                        .unwrap()
+                        .expect("always positive")
                 }
-                _ => operation.c.to_biguint().unwrap(),
+                _ => operation.c.to_biguint().expect("sign already checked"),
             };
 
-            let attr_c = Attribute::parse(context, &format!("{value} : {felt252_ty}")).unwrap();
+            let attr_c = Attribute::parse(context, &format!("{value} : {felt252_ty}"))
+                .ok_or(ErrorImpl::MissingMetadata)?;
 
             // TODO: Ensure that the constant is on the right side of the operation.
             mlir_asm! { context, entry, location =>
@@ -409,10 +419,15 @@ where
 {
     let value = match info.c.sign() {
         Sign::Minus => {
-            let prime = metadata.get::<PrimeModuloMeta<Felt252>>().unwrap().prime();
-            (&info.c + prime.to_bigint().unwrap()).to_biguint().unwrap()
+            let prime = metadata
+                .get::<PrimeModuloMeta<Felt252>>()
+                .ok_or(ErrorImpl::MissingMetadata)?
+                .prime();
+            (&info.c + prime.to_bigint().expect("always is Some"))
+                .to_biguint()
+                .expect("always is positive")
         }
-        _ => info.c.to_biguint().unwrap(),
+        _ => info.c.to_biguint().expect("sign already checked"),
     };
 
     let felt252_ty = registry.build_type(
@@ -423,7 +438,8 @@ where
         &info.branch_signatures()[0].vars[0].ty,
     )?;
 
-    let attr_c = Attribute::parse(context, &format!("{value} : {felt252_ty}")).unwrap();
+    let attr_c = Attribute::parse(context, &format!("{value} : {felt252_ty}"))
+        .ok_or(ErrorImpl::ParseAttributeError)?;
 
     mlir_asm! { context, entry, location =>
         ; k0 = "arith.constant"() { "value" = attr_c } : () -> felt252_ty

diff --git a/src/libfuncs/gas.rs b/src/libfuncs/gas.rs
@@ -5,7 +5,7 @@
 use super::{LibfuncBuilder, LibfuncHelper};
 use crate::{
     error::{
-        libfuncs::{Error, Result},
+        libfuncs::{Error, ErrorImpl, Result},
         CoreTypeBuilderError,
     },
     metadata::{gas::GasCost, MetadataStorage},
@@ -110,7 +110,8 @@ where
     let gas_cost_val = entry
         .append_operation(arith::constant(
             context,
-            Attribute::parse(context, &format!("{} : {}", cost.unwrap_or(0), u128_type)).unwrap(),
+            Attribute::parse(context, &format!("{} : {}", cost.unwrap_or(0), u128_type))
+                .ok_or(ErrorImpl::ParseAttributeError)?,
             location,
         ))
         .result(0)?
@@ -174,7 +175,8 @@ where
     let gas_cost_val = entry
         .append_operation(arith::constant(
             context,
-            Attribute::parse(context, &format!("{} : {}", cost.unwrap_or(0), u128_type)).unwrap(),
+            Attribute::parse(context, &format!("{} : {}", cost.unwrap_or(0), u128_type))
+                .ok_or(ErrorImpl::ParseAttributeError)?,
             location,
         ))
         .result(0)?