Change create_object_file to a Result to better indicate failure. Fixes rust-lang#102

Author: TheDan64

src/memory_buffer.rs

@@ -98,6 +98,7 @@ impl MemoryBuffer {
         MemoryBuffer::new(memory_buffer)
     }
 
+    /// Gets a byte slice of this `MemoryBuffer`.
     pub fn as_slice(&self) -> &[u8] {
         unsafe {
             let start = LLVMGetBufferStart(self.memory_buffer);
@@ -106,28 +107,27 @@ impl MemoryBuffer {
         }
     }
 
+    /// Gets the byte size of this `MemoryBuffer`.
     pub fn get_size(&self) -> usize {
         unsafe {
             LLVMGetBufferSize(self.memory_buffer)
         }
     }
 
-    // REVIEW: I haven't yet been able to find docs or other wrappers that confirm, but my suspicion
-    // is that the method needs to take ownership of the MemoryBuffer... otherwise I see what looks like
-    // a double free in valgrind when the MemoryBuffer drops so we are `forget`ting MemoryBuffer here
-    // for now until we can confirm this is the correct thing to do
-    pub fn create_object_file(self) -> Option<ObjectFile> {
+    /// Convert this `MemoryBuffer` into an `ObjectFile`. LLVM does not currently
+    /// provide any way to determine the cause of error if conversion fails.
+    pub fn create_object_file(self) -> Result<ObjectFile, ()> {
         let object_file = unsafe {
             LLVMCreateObjectFile(self.memory_buffer)
         };
 
         forget(self);
 
         if object_file.is_null() {
-            return None;
+            return Err(());
         }
 
-        Some(ObjectFile::new(object_file))
+        Ok(ObjectFile::new(object_file))
     }
 }
 

src/values/fn_value.rs

@@ -290,9 +290,7 @@ impl FunctionValue {
     // but not in all other versions
     #[cfg(feature = "llvm3-8")]
     pub unsafe fn get_personality_function(&self) -> Option<FunctionValue> {
-        let value = unsafe {
-            LLVMGetPersonalityFn(self.as_value_ref())
-        };
+        let value = LLVMGetPersonalityFn(self.as_value_ref());
 
         FunctionValue::new(value)
     }

tests/all/test_module.rs

@@ -123,7 +123,7 @@ fn test_write_and_load_memory_buffer() {
     let memory_buffer2 = module.write_bitcode_to_memory();
     let object_file = memory_buffer2.create_object_file();
 
-    assert!(object_file.is_none());
+    assert!(object_file.is_err());
 }
 
 #[test]

tests/all/test_values.rs

@@ -325,7 +325,7 @@ fn test_verify_fn() {
 
     #[cfg(not(any(feature = "llvm3-9", feature = "llvm4-0", feature = "llvm5-0", feature = "llvm6-0", feature = "llvm7-0", feature = "llvm8-0")))]
     assert!(!function.verify(false));
-     // REVIEW: Why does 3.9 -> 8.0 return true here? LLVM bug? Bugfix?
+    // REVIEW: Why does 3.9 -> 8.0 return true here? LLVM bug? Bugfix?
     #[cfg(any(feature = "llvm3-9", feature = "llvm4-0", feature = "llvm5-0", feature = "llvm6-0", feature = "llvm7-0", feature = "llvm8-0"))]
     assert!(function.verify(false));