Merge pull request rust-lang#107 from wasmerio/debug

Remove get_entry_basic_block and make get_first_basic_block return None when llvm returns nullptr.

Author: TheDan64

examples/kaleidoscope/main.rs

@@ -21,7 +21,6 @@ use std::iter::Peekable;
 use std::str::Chars;
 use std::ops::DerefMut;
 
-use self::inkwell::basic_block::BasicBlock;
 use self::inkwell::builder::Builder;
 use self::inkwell::context::Context;
 use self::inkwell::module::Module;
@@ -861,16 +860,15 @@ impl<'a> Compiler<'a> {
         self.fn_value_opt.unwrap()
     }
 
-    /// Cretes a new stack allocation instruction in the entry block of the function.
-    fn create_entry_block_alloca(&self, name: &str, entry: Option<&BasicBlock>) -> PointerValue {
+    /// Creates a new stack allocation instruction in the entry block of the function.
+    fn create_entry_block_alloca(&self, name: &str) -> PointerValue {
         let builder = self.context.create_builder();
 
-        let owned_entry = self.fn_value().get_entry_basic_block();
-        let entry = owned_entry.as_ref().or(entry).unwrap();
+        let entry = self.fn_value().get_first_basic_block().unwrap();
 
         match entry.get_first_instruction() {
             Some(first_instr) => builder.position_before(&first_instr),
-            None => builder.position_at_end(entry)
+            None => builder.position_at_end(&entry)
         }
 
         builder.build_alloca(self.context.f64_type(), name)
@@ -899,7 +897,7 @@ impl<'a> Compiler<'a> {
                         None => self.context.f64_type().const_float(0.)
                     };
 
-                    let alloca = self.create_entry_block_alloca(var_name, None);
+                    let alloca = self.create_entry_block_alloca(var_name);
 
                     self.builder.build_store(alloca, initial_val);
 
@@ -1040,7 +1038,7 @@ impl<'a> Compiler<'a> {
             Expr::For { ref var_name, ref start, ref end, ref step, ref body } => {
                 let parent = self.fn_value();
 
-                let start_alloca = self.create_entry_block_alloca(var_name, None);
+                let start_alloca = self.create_entry_block_alloca(var_name);
                 let start = self.compile_expr(start)?;
 
                 self.builder.build_store(start_alloca, start);
@@ -1132,7 +1130,7 @@ impl<'a> Compiler<'a> {
 
         for (i, arg) in function.get_param_iter().enumerate() {
             let arg_name = proto.args[i].as_str();
-            let alloca = self.create_entry_block_alloca(arg_name, Some(&entry));
+            let alloca = self.create_entry_block_alloca(arg_name);
 
             self.builder.build_store(alloca, arg);
 

src/values/fn_value.rs

@@ -1,5 +1,5 @@
 use llvm_sys::analysis::{LLVMVerifierFailureAction, LLVMVerifyFunction, LLVMViewFunctionCFG, LLVMViewFunctionCFGOnly};
-use llvm_sys::core::{LLVMIsAFunction, LLVMIsConstant, LLVMGetLinkage, LLVMGetPreviousFunction, LLVMGetNextFunction, LLVMGetParam, LLVMCountParams, LLVMGetLastParam, LLVMCountBasicBlocks, LLVMGetFirstParam, LLVMGetNextParam, LLVMGetBasicBlocks, LLVMAppendBasicBlock, LLVMDeleteFunction, LLVMGetLastBasicBlock, LLVMGetFirstBasicBlock, LLVMGetEntryBasicBlock, LLVMGetIntrinsicID, LLVMGetFunctionCallConv, LLVMSetFunctionCallConv, LLVMGetGC, LLVMSetGC, LLVMSetLinkage, LLVMSetParamAlignment, LLVMGetParams};
+use llvm_sys::core::{LLVMIsAFunction, LLVMIsConstant, LLVMGetLinkage, LLVMGetPreviousFunction, LLVMGetNextFunction, LLVMGetParam, LLVMCountParams, LLVMGetLastParam, LLVMCountBasicBlocks, LLVMGetFirstParam, LLVMGetNextParam, LLVMGetBasicBlocks, LLVMAppendBasicBlock, LLVMDeleteFunction, LLVMGetLastBasicBlock, LLVMGetFirstBasicBlock, LLVMGetIntrinsicID, LLVMGetFunctionCallConv, LLVMSetFunctionCallConv, LLVMGetGC, LLVMSetGC, LLVMSetLinkage, LLVMSetParamAlignment, LLVMGetParams};
 #[llvm_versions(3.7..=latest)]
 use llvm_sys::core::{LLVMGetPersonalityFn, LLVMSetPersonalityFn};
 #[llvm_versions(3.9..=latest)]
@@ -123,22 +123,6 @@ impl FunctionValue {
         Some(BasicValueEnum::new(param))
     }
 
-    // REVIEW: Odd behavior, a function with no BBs returns a ptr
-    // that isn't actually a basic_block and seems to get corrupted
-    // Should check filed LLVM bugs - maybe just return None since
-    // we can catch it with "LLVMIsABasicBlock"
-    pub fn get_entry_basic_block(&self) -> Option<BasicBlock> {
-        let bb = unsafe {
-            LLVMGetEntryBasicBlock(self.as_value_ref())
-        };
-
-        BasicBlock::new(bb)
-    }
-
-    // REVIEW: Odd behavior, a function with no BBs returns a ptr
-    // that isn't actually a basic_block and seems to get corrupted
-    // Should check filed LLVM bugs - maybe just return None since
-    // we can catch it with "LLVMIsABasicBlock"
     pub fn get_first_basic_block(&self) -> Option<BasicBlock> {
         let bb = unsafe {
             LLVMGetFirstBasicBlock(self.as_value_ref())

tests/all/test_basic_block.rs

@@ -18,11 +18,7 @@ fn test_basic_block_ordering() {
 
     let function = module.add_function("testing", fn_type, None);
 
-    // REVIEW: Possibly LLVM bug - gives a basic block ptr that isn't
-    // actually a basic block instead of returning nullptr. Simplest solution
-    // may be to just return None if LLVMIsABB doesn't pass
-    // assert!(function.get_entry_basic_block().is_none());
-    // assert!(function.get_first_basic_block().is_none());
+    assert!(function.get_first_basic_block().is_none());
 
     let basic_block = context.append_basic_block(&function, "entry");
     let basic_block4 = context.insert_basic_block_after(&basic_block, "block4");
@@ -58,7 +54,7 @@ fn test_basic_block_ordering() {
 
     function.append_basic_block("block6");
 
-    let bb1 = function.get_entry_basic_block().unwrap();
+    let bb1 = function.get_first_basic_block().unwrap();
     let bb4 = basic_block5.get_previous_basic_block().unwrap();
 
     assert_eq!(bb1, basic_block3);