Merge pull request rust-lang#133 from seanyoung/memorybuffer

MemoryBuffer::create_from_memory_range should take &[u8], not &str

Author: TheDan64

src/memory_buffer.rs

@@ -61,26 +61,16 @@ impl MemoryBuffer {
         Ok(MemoryBuffer::new(memory_buffer))
     }
 
-    // REVIEW: Does this make more sense to take a byte array as input?
-    /// This will create a new `MemoryBuffer` from the given input.
-    ///
     /// This function is likely slightly cheaper than `create_from_memory_range_copy` since it intentionally
     /// leaks data to LLVM so that it doesn't have to reallocate. `create_from_memory_range_copy` may be removed
     /// in the future
-    pub fn create_from_memory_range(input: &str, name: &str) -> Self {
-        let input_c_string = CString::new(input).expect("Conversion to CString failed unexpectedly");
+    pub fn create_from_memory_range(input: &[u8], name: &str) -> Self {
         let name_c_string = CString::new(name).expect("Conversion to CString failed unexpectedly");
 
         let memory_buffer = unsafe {
-            LLVMCreateMemoryBufferWithMemoryRange(input_c_string.as_ptr(), input.len(), name_c_string.as_ptr(), false as i32)
+            LLVMCreateMemoryBufferWithMemoryRange(input.as_ptr() as *const i8, input.len(), name_c_string.as_ptr(), false as i32)
         };
 
-        // LLVM seems to want to take ownership of input_c_string, which is why we need to forget it
-        // This originally was discovered when not forgetting it caused a subsequent as_slice call
-        // to sometimes return partially garbage data
-        // REVIEW: Does this apply to name_c_string as well?
-        forget(input_c_string);
-
         MemoryBuffer::new(memory_buffer)
     }
 
@@ -89,12 +79,11 @@ impl MemoryBuffer {
     /// This function is likely slightly more expensive than `create_from_memory_range` since it does not leak
     /// data to LLVM, forcing LLVM to make a copy. This function may be removed in the future in favor of
     /// `create_from_memory_range`
-    pub fn create_from_memory_range_copy(input: &str, name: &str) -> Self {
-        let input_c_string = CString::new(input).expect("Conversion to CString failed unexpectedly");
+    pub fn create_from_memory_range_copy(input: &[u8], name: &str) -> Self {
         let name_c_string = CString::new(name).expect("Conversion to CString failed unexpectedly");
 
         let memory_buffer = unsafe {
-            LLVMCreateMemoryBufferWithMemoryRangeCopy(input_c_string.as_ptr(), input.len(), name_c_string.as_ptr())
+            LLVMCreateMemoryBufferWithMemoryRangeCopy(input.as_ptr() as *const i8, input.len(), name_c_string.as_ptr())
         };
 
         MemoryBuffer::new(memory_buffer)

tests/all/test_module.rs

@@ -11,7 +11,6 @@ use std::ffi::CString;
 use std::fs::{File, remove_file};
 use std::io::Read;
 use std::path::Path;
-use std::str::from_utf8;
 
 #[test]
 fn test_write_bitcode_to_path() {
@@ -129,20 +128,20 @@ fn test_write_and_load_memory_buffer() {
 #[test]
 fn test_garbage_ir_fails_create_module_from_ir() {
     let context = Context::create();
-    let memory_buffer = MemoryBuffer::create_from_memory_range("garbage ir data", "my_ir");
+    let memory_buffer = MemoryBuffer::create_from_memory_range(b"garbage ir data", "my_ir");
 
     assert_eq!(memory_buffer.get_size(), 15);
-    assert_eq!(from_utf8(memory_buffer.as_slice()).unwrap(), "garbage ir data");
+    assert_eq!(memory_buffer.as_slice(), b"garbage ir data");
     assert!(context.create_module_from_ir(memory_buffer).is_err());
 }
 
 #[test]
 fn test_garbage_ir_fails_create_module_from_ir_copy() {
     let context = Context::create();
-    let memory_buffer = MemoryBuffer::create_from_memory_range_copy("garbage ir data", "my_ir");
+    let memory_buffer = MemoryBuffer::create_from_memory_range_copy(b"garbage ir data", "my_ir");
 
     assert_eq!(memory_buffer.get_size(), 15);
-    assert_eq!(from_utf8(memory_buffer.as_slice()).unwrap(), "garbage ir data");
+    assert_eq!(memory_buffer.as_slice(), b"garbage ir data");
     assert!(context.create_module_from_ir(memory_buffer).is_err());
 }
 
@@ -199,7 +198,7 @@ fn test_owned_module_dropped_ee_and_context() {
 #[test]
 fn test_parse_from_buffer() {
     let context = Context::create();
-    let garbage_buffer = MemoryBuffer::create_from_memory_range("garbage ir data", "my_ir");
+    let garbage_buffer = MemoryBuffer::create_from_memory_range(b"garbage ir data", "my_ir");
     let module_result = Module::parse_bitcode_from_buffer(&garbage_buffer);
 
     assert!(module_result.is_err());