Cleaned up some code.

Finally marked build_global_string as unsafe as per rust-lang#32

Author: TheDan64

src/builder.rs

@@ -6,7 +6,7 @@ use llvm_sys::{LLVMTypeKind, LLVMAtomicOrdering};
 use {IntPredicate, FloatPredicate};
 use basic_block::BasicBlock;
 use values::{AggregateValue, AsValueRef, BasicValue, BasicValueEnum, PhiValue, FunctionValue, IntValue, PointerValue, VectorValue, InstructionValue, GlobalValue, IntMathValue, FloatMathValue, PointerMathValue, InstructionOpcode};
-use types::{AsTypeRef, BasicType, PointerType, IntMathType, FloatMathType, PointerMathType};
+use types::{AsTypeRef, BasicType, IntMathType, FloatMathType, PointerMathType};
 
 use std::ffi::CString;
 
@@ -101,9 +101,7 @@ impl Builder {
         let mut index_values: Vec<LLVMValueRef> = ordered_indexes.iter()
                                                                  .map(|val| val.as_value_ref())
                                                                  .collect();
-        let value = unsafe {
-            LLVMBuildGEP(self.builder, ptr.as_value_ref(), index_values.as_mut_ptr(), index_values.len() as u32, c_string.as_ptr())
-        };
+        let value = LLVMBuildGEP(self.builder, ptr.as_value_ref(), index_values.as_mut_ptr(), index_values.len() as u32, c_string.as_ptr());
 
         PointerValue::new(value)
     }
@@ -116,9 +114,7 @@ impl Builder {
         let mut index_values: Vec<LLVMValueRef> = ordered_indexes.iter()
                                                                  .map(|val| val.as_value_ref())
                                                                  .collect();
-        let value = unsafe {
-            LLVMBuildInBoundsGEP(self.builder, ptr.as_value_ref(), index_values.as_mut_ptr(), index_values.len() as u32, c_string.as_ptr())
-        };
+        let value = LLVMBuildInBoundsGEP(self.builder, ptr.as_value_ref(), index_values.as_mut_ptr(), index_values.len() as u32, c_string.as_ptr());
 
         PointerValue::new(value)
     }
@@ -1040,16 +1036,17 @@ impl Builder {
         InstructionValue::new(switch_value)
     }
 
-    pub fn build_global_string(&self, value: &str, name: &str) -> GlobalValue {
+    // The unsafety of this function should be fixable with subtypes. See GH #32
+    pub unsafe fn build_global_string(&self, value: &str, name: &str) -> GlobalValue {
         let c_string_value = CString::new(value).expect("Conversion to CString failed unexpectedly");
         let c_string_name = CString::new(name).expect("Conversion to CString failed unexpectedly");
-        let value = unsafe {
-            LLVMBuildGlobalString(self.builder, c_string_value.as_ptr(), c_string_name.as_ptr())
-        };
+        let value = LLVMBuildGlobalString(self.builder, c_string_value.as_ptr(), c_string_name.as_ptr());
 
         GlobalValue::new(value)
     }
 
+    // REVIEW: Does this similar fn have the same issue build_global_string does? If so, mark as unsafe
+    // and fix with subtypes.
     pub fn build_global_string_ptr(&self, value: &str, name: &str) -> GlobalValue {
         let c_string_value = CString::new(value).expect("Conversion to CString failed unexpectedly");
         let c_string_name = CString::new(name).expect("Conversion to CString failed unexpectedly");

src/data_layout.rs

@@ -1,6 +1,5 @@
 use std::ffi::CStr;
 use std::fmt;
-use std::ops::Deref;
 
 use support::{LLVMString, LLVMStringOrRaw};
 

src/module.rs

@@ -1,5 +1,5 @@
 use llvm_sys::analysis::{LLVMVerifyModule, LLVMVerifierFailureAction};
-use llvm_sys::bit_reader::{LLVMParseBitcode, LLVMParseBitcodeInContext, LLVMGetBitcodeModuleInContext, LLVMGetBitcodeModule};
+use llvm_sys::bit_reader::{LLVMParseBitcode, LLVMParseBitcodeInContext};
 use llvm_sys::bit_writer::{LLVMWriteBitcodeToFile, LLVMWriteBitcodeToMemoryBuffer};
 use llvm_sys::core::{LLVMAddFunction, LLVMAddGlobal, LLVMDumpModule, LLVMGetNamedFunction, LLVMGetTypeByName, LLVMSetDataLayout, LLVMSetTarget, LLVMCloneModule, LLVMDisposeModule, LLVMGetTarget, LLVMModuleCreateWithName, LLVMGetModuleContext, LLVMGetFirstFunction, LLVMGetLastFunction, LLVMSetLinkage, LLVMAddGlobalInAddressSpace, LLVMPrintModuleToString, LLVMGetNamedMetadataNumOperands, LLVMAddNamedMetadataOperand, LLVMGetNamedMetadataOperands, LLVMGetFirstGlobal, LLVMGetLastGlobal, LLVMGetNamedGlobal, LLVMPrintModuleToFile, LLVMSetModuleInlineAsm};
 use llvm_sys::execution_engine::{LLVMCreateInterpreterForModule, LLVMCreateJITCompilerForModule, LLVMCreateExecutionEngineForModule};

tests/test_builder.rs

@@ -442,7 +442,7 @@ fn test_no_builder_double_free2() {
 
     #[cfg(any(feature = "llvm3-6", feature = "llvm3-7", feature = "llvm3-8"))]
     assert_eq!(*module.print_to_string(), *CString::new("; ModuleID = \'my_mod\'\n\ndefine void @my_fn() {\nentry:\n  unreachable\n}\n").unwrap());
-    #[cfg(any(feature = "llvm3-9", feature = "llvm4-0", feature = "llvm5-0", feature = "llvm6-0"))]
+    #[cfg(not(any(feature = "llvm3-6", feature = "llvm3-7", feature = "llvm3-8")))]
     assert_eq!(*module.print_to_string(), *CString::new("; ModuleID = \'my_mod\'\nsource_filename = \"my_mod\"\n\ndefine void @my_fn() {\nentry:\n  unreachable\n}\n").unwrap());
 
     // 2nd Context drops fine