Made the tests pass again

Author: Michael-F-Bryan

examples/kaleidoscope/main.rs

@@ -1317,7 +1317,7 @@ pub fn main() {
         if is_anonymous {
             let ee = module.create_jit_execution_engine(OptimizationLevel::None).unwrap();
 
-            let maybe_fn = unsafe { ee.get_function::<extern "C" fn() -> f64>(name.as_str()) };
+            let maybe_fn = unsafe { ee.get_function::<unsafe extern "C" fn() -> f64>(name.as_str()) };
             let compiled_fn = match maybe_fn {
                 Ok(f) => f,
                 Err(err) => {
@@ -1326,7 +1326,9 @@ pub fn main() {
                 }
             };
 
-            println!("=> {}", compiled_fn());
+            unsafe {
+                println!("=> {}", compiled_fn());
+            }
         }
     }
 }

src/execution_engine.rs

@@ -9,6 +9,7 @@ use std::rc::Rc;
 use std::ops::Deref;
 use std::ffi::{CStr, CString};
 use std::mem::{forget, uninitialized, zeroed, transmute_copy, size_of};
+use std::fmt::{self, Debug, Formatter};
 
 #[derive(Debug, PartialEq, Eq)]
 pub enum FunctionLookupError {
@@ -164,6 +165,12 @@ impl ExecutionEngine {
     /// If a target hasn't already been initialized, spurious "function not 
     /// found" errors may be encountered.
     /// 
+    /// The [`UnsafeFunctionPointer`] trait is designed so only `unsafe extern 
+    /// "C"` functions can be retrieved via the `get_function()` method. If you
+    /// get funny type errors then it's probably because you have specified the
+    /// wrong calling convention or forgotten to specify the retrieved function
+    /// is `unsafe`.
+    /// 
     /// # Examples
     /// 
     /// 
@@ -200,7 +207,6 @@ impl ExecutionEngine {
     /// }
     /// ```
     /// 
-    /// 
     /// # Safety
     /// 
     /// It is the caller's responsibility to ensure they call the function with
@@ -209,6 +215,8 @@ impl ExecutionEngine {
     /// The `Symbol` wrapper ensures a function won't accidentally outlive the
     /// execution engine it came from, but adding functions after calling this
     /// method *may* invalidate the function pointer.
+    /// 
+    /// [`UnsafeFunctionPointer`]: trait.UnsafeFunctionPointer.html
     pub unsafe fn get_function<F>(&self, fn_name: &str) -> Result<Symbol<F>, FunctionLookupError>
     where F: UnsafeFunctionPointer 
     {
@@ -293,10 +301,24 @@ impl ExecutionEngine {
     }
 }
 
+// Modules owned by the EE will be discarded by the EE so we don't
+// want owned modules to drop.
+impl Drop for ExecutionEngine {
+    fn drop(&mut self) {
+        forget(self.target_data.take().expect("TargetData should always exist until Drop"));
+
+        if Rc::strong_count(&self.execution_engine) == 1 {
+            unsafe {
+                LLVMDisposeExecutionEngine(*self.execution_engine);
+            }
+        }
+    }
+}
+
 /// A wrapper around a function pointer which ensures the symbol being pointed
 /// to doesn't accidentally outlive its execution engine.
-#[derive(Debug, Clone)]
-pub struct Symbol<F: UnsafeFunctionPointer> {
+#[derive(Clone)]
+pub struct Symbol<F> {
     pub(crate) execution_engine: Rc<LLVMExecutionEngineRef>,
     inner: F,
 }
@@ -309,8 +331,16 @@ impl<F: UnsafeFunctionPointer> Deref for Symbol<F> {
     }
 }
 
+impl<F> Debug for Symbol<F> {
+    fn fmt(&self, f: &mut Formatter) -> fmt::Result {
+        f.debug_tuple("Symbol")
+            .field(&"<unnamed>")
+            .finish()
+    }
+}
+
 /// Marker trait representing an unsafe function pointer (`unsafe extern "C" fn(A, B, ...) -> Output`).
-pub trait UnsafeFunctionPointer: private::Sealed {}
+pub trait UnsafeFunctionPointer: private::Sealed + Copy {}
 
 mod private {
     /// A sealed trait which ensures nobody outside this crate can implement
@@ -337,17 +367,8 @@ impl_unsafe_fn!(A, B, C, D, E, F);
 impl_unsafe_fn!(A, B, C, D, E, F, G);
 impl_unsafe_fn!(A, B, C, D, E, F, G, H);
 impl_unsafe_fn!(A, B, C, D, E, F, G, H, I);
+impl_unsafe_fn!(A, B, C, D, E, F, G, H, I, J);
+impl_unsafe_fn!(A, B, C, D, E, F, G, H, I, J, K);
+impl_unsafe_fn!(A, B, C, D, E, F, G, H, I, J, K, L);
+impl_unsafe_fn!(A, B, C, D, E, F, G, H, I, J, K, L, M);
 
-// Modules owned by the EE will be discarded by the EE so we don't
-// want owned modules to drop.
-impl Drop for ExecutionEngine {
-    fn drop(&mut self) {
-        forget(self.target_data.take().expect("TargetData should always exist until Drop"));
-
-        if Rc::strong_count(&self.execution_engine) == 1 {
-            unsafe {
-                LLVMDisposeExecutionEngine(*self.execution_engine);
-            }
-        }
-    }
-}

tests/test_builder.rs

@@ -129,14 +129,14 @@ fn test_null_checked_ptr_ops() {
     let execution_engine = module.create_jit_execution_engine(OptimizationLevel::None).unwrap();
 
     unsafe {
-        let check_null_index1: Symbol<extern "C" fn(*const i8) -> i8> = execution_engine.get_function("check_null_index1").unwrap();
+        let check_null_index1: Symbol<unsafe extern "C" fn(*const i8) -> i8> = execution_engine.get_function("check_null_index1").unwrap();
 
         let array = &[100i8, 42i8];
 
         assert_eq!(check_null_index1(null()), -1i8);
         assert_eq!(check_null_index1(array.as_ptr()), 42i8);
 
-        let check_null_index2: Symbol<extern "C" fn(*const i8) -> i8> = execution_engine.get_function("check_null_index2").unwrap();
+        let check_null_index2: Symbol<unsafe extern "C" fn(*const i8) -> i8> = execution_engine.get_function("check_null_index2").unwrap();
 
         assert_eq!(check_null_index2(null()), -1i8);
         assert_eq!(check_null_index2(array.as_ptr()), 42i8);
@@ -279,7 +279,7 @@ fn test_switch() {
     builder.build_return(Some(&double));
 
     unsafe {
-        let switch: Symbol<extern "C" fn(u8) -> u8> = execution_engine.get_function("switch").unwrap();
+        let switch: Symbol<unsafe extern "C" fn(u8) -> u8> = execution_engine.get_function("switch").unwrap();
 
         assert_eq!(switch(0), 1);
         assert_eq!(switch(1), 2);
@@ -349,9 +349,9 @@ fn test_bit_shifts() {
     builder.build_return(Some(&shift));
 
     unsafe {
-        let left_shift: Symbol<extern "C" fn(u8, u8) -> u8> = execution_engine.get_function("left_shift").unwrap();
-        let right_shift: Symbol<extern "C" fn(u8, u8) -> u8>  = execution_engine.get_function("right_shift").unwrap();
-        let right_shift_sign_extend: Symbol<extern "C" fn(i8, u8) -> i8> = execution_engine.get_function("right_shift_sign_extend").unwrap();
+        let left_shift: Symbol<unsafe extern "C" fn(u8, u8) -> u8> = execution_engine.get_function("left_shift").unwrap();
+        let right_shift: Symbol<unsafe extern "C" fn(u8, u8) -> u8>  = execution_engine.get_function("right_shift").unwrap();
+        let right_shift_sign_extend: Symbol<unsafe extern "C" fn(i8, u8) -> i8> = execution_engine.get_function("right_shift_sign_extend").unwrap();
 
         assert_eq!(left_shift(0, 0), 0);
         assert_eq!(left_shift(0, 4), 0);

tests/test_execution_engine.rs

@@ -5,6 +5,8 @@ use self::inkwell::context::Context;
 use self::inkwell::execution_engine::FunctionLookupError;
 use self::inkwell::targets::{InitializationConfig, Target};
 
+type Thunk = unsafe extern "C" fn();
+
 #[test]
 fn test_get_function_address() {
     let context = Context::create();
@@ -24,7 +26,7 @@ fn test_get_function_address() {
     let execution_engine = module.create_jit_execution_engine(OptimizationLevel::None).unwrap();
 
     unsafe {
-        assert_eq!(execution_engine.get_function::<fn()>("errors").unwrap_err(), 
+        assert_eq!(execution_engine.get_function::<Thunk>("errors").unwrap_err(), 
             FunctionLookupError::FunctionNotFound);
     }
 
@@ -38,10 +40,10 @@ fn test_get_function_address() {
     let execution_engine = module.create_jit_execution_engine(OptimizationLevel::None).unwrap();
 
     unsafe {
-        assert_eq!(execution_engine.get_function::<fn()>("errors").unwrap_err(), 
+        assert_eq!(execution_engine.get_function::<Thunk>("errors").unwrap_err(), 
             FunctionLookupError::FunctionNotFound);
 
-        assert!(execution_engine.get_function::<fn()>("func").is_ok());
+        assert!(execution_engine.get_function::<Thunk>("func").is_ok());
     }
 }
 

tests/test_tari_example.rs

@@ -33,7 +33,8 @@ fn test_tari_example() {
     builder.build_return(Some(&sum));
 
     unsafe {
-        let sum: Symbol<extern "C" fn(u64, u64, u64) -> u64> = execution_engine.get_function("sum").unwrap();
+        type Sum = unsafe extern "C" fn(u64, u64, u64) -> u64;
+        let sum: Symbol<Sum> = execution_engine.get_function("sum").unwrap();
 
         let x = 1u64;
         let y = 2u64;