inkwell icon indicating copy to clipboard operation
inkwell copied to clipboard

Add append existing basic block method to context impl

Open tahadostifam opened this issue 1 month ago • 3 comments

Description

Added a new method append_existing_basic_block to ContextImpl that wraps LLVM’s LLVMAppendExistingBasicBlock function. This allows developers to attach an existing BasicBlock to a function, which is crucial for implementing precise control flow structures such as if, loop, and match. The method provides a safer and more ergonomic abstraction over the unsafe raw LLVM call.

fn append_existing_basic_block<'ctx>(&self, basic_block: BasicBlock<'ctx>) {
    unsafe {
        LLVMAppendExistingBasicBlock(self.0, basic_block.as_mut_ptr());
    }
}

Related Issue

No open issue currently; this is a feature proposal to extend Inkwell’s API to better support compiler IR generation.

How This Has Been Tested

Tested by integrating the method in a sample compiler backend for control flow emission:

  1. Created a function and multiple BasicBlocks.
  2. Used append_existing_basic_block to attach a block to the function.
  3. Positioned the builder at the end of the attached block.
  4. Emitted instructions and verified they were correctly inserted into the block.
  5. Confirmed generated LLVM IR matched the expected control flow.

No regressions were observed in existing tests.

Option<Breaking Changes>

No breaking changes; this is an additive method that does not affect existing API behavior.

tahadostifam avatar Nov 13 '25 21:11 tahadostifam

Okay, the code is now ready for review.

There's a subtle issue: I’m not entirely sure why it triggers a complaint on LLVM 20. However, due to changes in LLVM 20 regarding LLVMAppendExistingBasicBlock, I separated the function into two versions to provide proper support across LLVM versions:

#[llvm_versions(12..20)]
fn append_existing_basic_block<'ctx>(&self, basic_block: BasicBlock<'ctx>) {
    unsafe {
        let basic_block_value = LLVMBasicBlockAsValue(basic_block.as_mut_ptr());
        LLVMAppendExistingBasicBlock(self.0, basic_block_value);
    }
}

#[llvm_versions(20..)]
fn append_existing_basic_block<'ctx>(&self, function: FunctionValue<'ctx>, basic_block: BasicBlock<'ctx>) {
    unsafe {
        LLVMAppendExistingBasicBlock(function.as_mut_ptr(), basic_block.as_mut_ptr());
    }
}

tahadostifam avatar Nov 14 '25 14:11 tahadostifam

Im sure added a testcase for it in the ./tests/all/test_basic_block.rs but its still complaining about it!

error: method `append_existing_basic_block` is never used
   --> src/context.rs:284:8
    |
 83 | impl ContextImpl {
    | ---------------- method in this implementation
...
284 |     fn append_existing_basic_block<'ctx>(&self, function: FunctionValue<'ctx>, basic_block: BasicBlock<'ctx>) {
    |        ^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = note: `-D dead-code` implied by `-D warnings`
    = help: to override `-D warnings` add `#[allow(dead_code)]`

How we can fix it?

tahadostifam avatar Nov 25 '25 22:11 tahadostifam

That's really weird. I suppose you could slap #[allow(dead_code)] on it and call it a day

TheDan64 avatar Nov 26 '25 00:11 TheDan64