Merge pull request #321 from MediosZ/handle_rs_errors

Improve error handling in rs_loader.

Author: viferga

source/loaders/rs_loader/rust/compiler/src/api/function.rs

@@ -33,7 +33,7 @@ extern "C" fn function_singleton_invoke(
     unsafe {
         let args = std::slice::from_raw_parts(args_p, size).to_vec();
         let nf = Box::from_raw(func_impl as *mut class::NormalFunction);
-        let res = nf.invoke(args).unwrap();
+        let res = nf.invoke(args).expect("Function return error");
 
         std::mem::forget(nf);
         res
@@ -118,8 +118,7 @@ pub fn register_function(function_registration: FunctionRegistration) {
                 loader_impl_type(function_registration.loader_impl, ret.as_ptr()),
             );
         };
-    } 
-    else {
+    } else {
         let ret = CString::new("Null").expect("Failed to convert return type to C string");
 
         unsafe {

source/loaders/rs_loader/rust/compiler/src/lib.rs

@@ -104,8 +104,15 @@ impl Source {
 
         match source {
             Source::File { ref path } => {
-                let dir = PathBuf::from(path.clone().parent().unwrap());
-                let name = PathBuf::from(path.file_name().unwrap());
+                let dir = PathBuf::from(
+                    path.clone()
+                        .parent()
+                        .expect(format!("Unable to get the parent of {:?}", path).as_str()),
+                );
+                let name = PathBuf::from(
+                    path.file_name()
+                        .expect(format!("Unable to get the filename of {:?}", path).as_str()),
+                );
 
                 SourceImpl {
                     input: SourceInput(config::Input::File(path.clone())),
@@ -129,8 +136,15 @@ impl Source {
                 }
             }
             Source::Package { ref path } => {
-                let dir = PathBuf::from(path.clone().parent().unwrap());
-                let name = PathBuf::from(path.file_name().unwrap());
+                let dir = PathBuf::from(
+                    path.clone()
+                        .parent()
+                        .expect(format!("Unable to get the parent of {:?}", path).as_str()),
+                );
+                let name = PathBuf::from(
+                    path.file_name()
+                        .expect(format!("Unable to get the filename of {:?}", path).as_str()),
+                );
 
                 SourceImpl {
                     input: SourceInput(config::Input::File(path.clone())),
@@ -252,7 +266,7 @@ impl DlopenLibrary {
                 let dll_opening_error = format!(
                     "{}\nrs_loader was unable to open the dll with the following path: `{}`", 
                     error,
-                    path_to_dll.to_str().unwrap()
+                    path_to_dll.to_str().expect("Unable to cast pathbuf to str")
                 );
 
                 return Err(dll_opening_error)
@@ -387,7 +401,11 @@ impl CompilerCallbacks {
             .borrow_mut()
             .access(|resolver| {
                 let c_store = resolver.cstore().clone();
-                c_store.crates_untracked().last().cloned().unwrap()
+                c_store
+                    .crates_untracked()
+                    .last()
+                    .cloned()
+                    .expect("Unable to get last element of crates.")
             });
         queries
             .global_ctxt()
@@ -686,10 +704,10 @@ struct DiagnosticSink(sync::Arc<sync::Mutex<Vec<u8>>>);
 
 impl std::io::Write for DiagnosticSink {
     fn write(&mut self, buf: &[u8]) -> std::io::Result<usize> {
-        self.0.lock().unwrap().write(buf)
+        self.0.lock().expect("Unable to acquire lock").write(buf)
     }
     fn flush(&mut self) -> std::io::Result<()> {
-        self.0.lock().unwrap().flush()
+        self.0.lock().expect("Unable to acquire lock").flush()
     }
 }
 
@@ -857,12 +875,23 @@ pub fn compile(source: SourceImpl) -> Result<CompilerState, CompilerError> {
         Ok(()) => Ok(()),
         Err(err) => {
             // Read buffered diagnostics
-            let diagnostics =
-                String::from_utf8(diagnostics_buffer.lock().unwrap().clone()).unwrap();
+            let diagnostics = String::from_utf8(
+                diagnostics_buffer
+                    .lock()
+                    .expect("Unable to acquire lock")
+                    .clone(),
+            )
+            .expect("Unable to get string from utf8");
             eprintln!("{}", diagnostics);
 
             // Read buffered errors
-            let errors = String::from_utf8(errors_buffer.lock().unwrap().clone()).unwrap();
+            let errors = String::from_utf8(
+                errors_buffer
+                    .lock()
+                    .expect("Unable to acquire lock")
+                    .clone(),
+            )
+            .expect("Unable to get string from utf8");
             eprintln!("{}", errors);
 
             return Err(CompilerError {
@@ -877,7 +906,7 @@ pub fn compile(source: SourceImpl) -> Result<CompilerState, CompilerError> {
         return Err(e);
     }
 
-    let mut patched_callback = generate_wrapper(callbacks).unwrap();
+    let mut patched_callback = generate_wrapper(callbacks).expect("Unable to generate wrapper");
 
     // generate binary
     match rustc_driver::catch_fatal_errors(|| {
@@ -892,12 +921,23 @@ pub fn compile(source: SourceImpl) -> Result<CompilerState, CompilerError> {
         }),
         Err(err) => {
             // Read buffered diagnostics
-            let diagnostics =
-                String::from_utf8(diagnostics_buffer.lock().unwrap().clone()).unwrap();
+            let diagnostics = String::from_utf8(
+                diagnostics_buffer
+                    .lock()
+                    .expect("Unable to acquire lock")
+                    .clone(),
+            )
+            .expect("Unable to get string from utf8");
             eprintln!("{}", diagnostics);
 
             // Read buffered errors
-            let errors = String::from_utf8(errors_buffer.lock().unwrap().clone()).unwrap();
+            let errors = String::from_utf8(
+                errors_buffer
+                    .lock()
+                    .expect("Unable to acquire lock")
+                    .clone(),
+            )
+            .expect("Unable to get string from utf8");
             eprintln!("{}", errors);
 
             Err(CompilerError {

source/loaders/rs_loader/rust/compiler/src/registrator.rs

@@ -11,7 +11,8 @@ fn function_create(func: &Function, dlopen_library: &DlopenLibrary) -> FunctionC
 
     let register_func_name = format!("metacall_register_fn_{}", name);
     let register_func: unsafe fn() -> *mut class::NormalFunction =
-        unsafe { dlopen_library.instance.symbol(&register_func_name[..]) }.unwrap();
+        unsafe { dlopen_library.instance.symbol(&register_func_name[..]) }
+            .expect(format!("Unable to find register function {}", name).as_str());
     let function_impl = unsafe { register_func() } as OpaqueType;
     FunctionCreate {
         name,
@@ -25,7 +26,8 @@ fn class_create(class: &Class, dlopen_library: &DlopenLibrary) -> ClassCreate {
     let name = class.name.clone();
     let register_func_name = format!("metacall_register_class_{}", name);
     let register_func: unsafe fn() -> *mut class::Class =
-        unsafe { dlopen_library.instance.symbol(&register_func_name[..]) }.unwrap();
+        unsafe { dlopen_library.instance.symbol(&register_func_name[..]) }
+            .expect(format!("Unable to find register function {}", name).as_str());
     let class_impl = unsafe { register_func() } as OpaqueType;
     ClassCreate {
         name,

source/loaders/rs_loader/rust/compiler/src/wrapper/class.rs

@@ -3,10 +3,10 @@ use std::cell::Ref;
 use std::cell::RefCell;
 use std::cell::RefMut;
 use std::collections::HashMap;
+use std::convert::TryInto;
 use std::ffi::CStr;
 use std::fmt;
 use std::sync::Arc;
-use std::convert::TryInto;
 type Result<T, E = i32> = core::result::Result<T, E>;
 use std::os::raw::{c_char, c_double, c_float, c_int, c_long, c_short, c_void};
 extern "C" {
@@ -62,11 +62,18 @@ impl Class {
     }
 
     pub fn init(&self, fields: Vec<MetacallValue>) -> Instance {
-        self.constructor.as_ref().unwrap().invoke(fields).unwrap()
+        self.constructor
+            .as_ref()
+            .expect("Unable to get ref of constructor.")
+            .invoke(fields)
+            .expect("Invoke return error.")
     }
 
     pub fn call(&self, attr: &str, args: Vec<MetacallValue>) -> Result<MetacallValue> {
-        let attr = self.class_methods.get(attr).unwrap();
+        let attr = self
+            .class_methods
+            .get(attr)
+            .expect(format!("Unable to get {} from {}", attr, self.name).as_str());
 
         attr.clone().invoke(args)
     }
@@ -216,11 +223,19 @@ impl Instance {
 
     /// Lookup an attribute on the instance via the registered `Class`
     pub fn get_attr(&self, name: &str, class: &Class) -> Result<MetacallValue> {
-        let attr = class.attributes.get(name).unwrap().clone();
+        let attr = class
+            .attributes
+            .get(name)
+            .expect(format!("Unable to find attribute {} from {}", name, self.name()).as_str())
+            .clone();
         attr.invoke(self)
     }
     pub fn set_attr(&mut self, name: &str, value: MetacallValue, class: &Class) {
-        let attr = class.attr_setters.get(name).unwrap().clone();
+        let attr = class
+            .attr_setters
+            .get(name)
+            .expect(format!("Unable to find attribute {} from {}", name, self.name()).as_str())
+            .clone();
         attr.invoke(value, self)
     }
 
@@ -238,7 +253,9 @@ impl Instance {
         args: Vec<MetacallValue>,
         class: &Class,
     ) -> Result<MetacallValue> {
-        let method = class.get_method(name).unwrap();
+        let method = class
+            .get_method(name)
+            .expect(format!("Unable to find method {} from {}", name, self.name()).as_str());
         method.invoke(self, args)
     }
 }
@@ -343,7 +360,9 @@ impl AttributeGetter {
     {
         Self(Arc::new(move |receiver| {
             let borrowed_receiver = receiver.borrow();
-            let receiver = Ok(borrowed_receiver.downcast_ref::<T>().unwrap());
+            let receiver = Ok(borrowed_receiver
+                .downcast_ref::<T>()
+                .expect("Unable to downcast"));
             receiver.map(&f).and_then(|v| v.to_meta_result())
         }))
     }
@@ -364,7 +383,9 @@ impl AttributeSetter {
     {
         Self(Arc::new(move |value, receiver| {
             let mut borrowed_receiver = receiver.borrow_mut();
-            let receiver = borrowed_receiver.downcast_mut::<T>().unwrap();
+            let receiver = borrowed_receiver
+                .downcast_mut::<T>()
+                .expect("Unable to downcast");
             f(FromMeta::from_meta(value).unwrap(), receiver)
         }))
     }
@@ -388,7 +409,9 @@ impl InstanceMethod {
         Self(Arc::new(
             move |receiver: &Instance, args: Vec<MetacallValue>| {
                 let borrowed_receiver = receiver.borrow();
-                let receiver = Ok(borrowed_receiver.downcast_ref::<T>().unwrap());
+                let receiver = Ok(borrowed_receiver
+                    .downcast_ref::<T>()
+                    .expect("Unable to downcast"));
 
                 let args = Args::from_meta_list(&args);
 
@@ -605,11 +628,21 @@ impl TryFrom<i32> for PrimitiveMetacallProtocolTypes {
 
     fn try_from(v: i32) -> Result<Self, Self::Error> {
         match v {
-            x if x == PrimitiveMetacallProtocolTypes::Short as i32 => Ok(PrimitiveMetacallProtocolTypes::Short),
-            x if x == PrimitiveMetacallProtocolTypes::Int as i32 => Ok(PrimitiveMetacallProtocolTypes::Int),
-            x if x == PrimitiveMetacallProtocolTypes::Long as i32 => Ok(PrimitiveMetacallProtocolTypes::Long),
-            x if x == PrimitiveMetacallProtocolTypes::Float as i32 => Ok(PrimitiveMetacallProtocolTypes::Float),
-            x if x == PrimitiveMetacallProtocolTypes::Double as i32 => Ok(PrimitiveMetacallProtocolTypes::Double),
+            x if x == PrimitiveMetacallProtocolTypes::Short as i32 => {
+                Ok(PrimitiveMetacallProtocolTypes::Short)
+            }
+            x if x == PrimitiveMetacallProtocolTypes::Int as i32 => {
+                Ok(PrimitiveMetacallProtocolTypes::Int)
+            }
+            x if x == PrimitiveMetacallProtocolTypes::Long as i32 => {
+                Ok(PrimitiveMetacallProtocolTypes::Long)
+            }
+            x if x == PrimitiveMetacallProtocolTypes::Float as i32 => {
+                Ok(PrimitiveMetacallProtocolTypes::Float)
+            }
+            x if x == PrimitiveMetacallProtocolTypes::Double as i32 => {
+                Ok(PrimitiveMetacallProtocolTypes::Double)
+            }
             _ => Err(()),
         }
     }
@@ -621,11 +654,17 @@ macro_rules! convert_to {
             let id = value_type_id($val);
 
             match id.try_into() {
-                Ok(PrimitiveMetacallProtocolTypes::Short) => Ok(metacall_value_to_short($val) as $t),
+                Ok(PrimitiveMetacallProtocolTypes::Short) => {
+                    Ok(metacall_value_to_short($val) as $t)
+                }
                 Ok(PrimitiveMetacallProtocolTypes::Int) => Ok(metacall_value_to_int($val) as $t),
                 Ok(PrimitiveMetacallProtocolTypes::Long) => Ok(metacall_value_to_long($val) as $t),
-                Ok(PrimitiveMetacallProtocolTypes::Float) => Ok(metacall_value_to_float($val) as $t),
-                Ok(PrimitiveMetacallProtocolTypes::Double) => Ok(metacall_value_to_double($val) as $t),
+                Ok(PrimitiveMetacallProtocolTypes::Float) => {
+                    Ok(metacall_value_to_float($val) as $t)
+                }
+                Ok(PrimitiveMetacallProtocolTypes::Double) => {
+                    Ok(metacall_value_to_double($val) as $t)
+                }
                 Err(_) => {
                     println!("receive id: {}, should be [2-6]", id);
                     panic!("received mismatch type");
@@ -670,7 +709,10 @@ impl FromMeta for String {
     fn from_meta(val: MetacallValue) -> Result<Self> {
         Ok(unsafe {
             let s = metacall_value_to_string(val);
-            CStr::from_ptr(s).to_str().unwrap().to_owned()
+            CStr::from_ptr(s)
+                .to_str()
+                .expect("Unable to cast Cstr to str")
+                .to_owned()
         })
     }
 }

source/loaders/rs_loader/rust/compiler/src/wrapper/mod.rs

@@ -174,7 +174,7 @@ pub fn generate_wrapper(callbacks: CompilerCallbacks) -> std::io::Result<Compile
                         .file_name()
                         .expect("not a file")
                         .to_str()
-                        .unwrap()
+                        .expect("Unable to cast OsStr to str")
                         .to_owned();
                     let _ = source_path.pop();
 

source/loaders/rs_loader/rust/src/lib.rs

@@ -1,9 +1,3 @@
-pub use std::ffi::c_void;
-pub use std::ffi::CStr;
-pub use std::os::raw::c_char;
-pub use std::os::raw::c_int;
-pub use std::path::PathBuf;
-
 mod lifecycle;
 
 pub use lifecycle::rs_loader_impl_clear;

source/loaders/rs_loader/rust/src/lifecycle/clear.rs

@@ -1,5 +1,5 @@
 use super::loader::LoadingMethod;
-use crate::{c_int, c_void};
+use std::os::raw::{c_int, c_void};
 
 use compiler::api;
 
@@ -10,18 +10,16 @@ pub extern "C" fn rs_loader_impl_clear(loader_impl: *mut c_void, handle: *mut c_
             .as_mut()
             .expect("Unable to get loader state")
     };
-    unsafe {
-        let methods = Box::from_raw(handle as *mut Vec<LoadingMethod>);
-        for loading_method in *methods {
-            match loading_method.consume_dlib() {
-                Ok(lib) => {
-                    // extend the lifetime of library
-                    loader_lifecycle_state.destroy_list.push(lib);
-                }
-                Err(err) => {
-                    eprintln!("{}", err);
-                    return 1 as c_int;
-                }
+    let methods = unsafe { Box::from_raw(handle as *mut Vec<LoadingMethod>) };
+    for loading_method in *methods {
+        match loading_method.consume_dlib() {
+            Ok(lib) => {
+                // extend the lifetime of library
+                loader_lifecycle_state.destroy_list.push(lib);
+            }
+            Err(err) => {
+                eprintln!("{}", err);
+                return 1 as c_int;
             }
         }
     }