Add append existing basic block method to context impl #625
Conversation
…::is_const() which wasn't support in previous commit
|
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 #[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());
}
} |
TheDan64
left a comment
There was a problem hiding this comment.
This looks pretty good but there's build failures and some small things to fix before we can merge
Cargo.toml
Outdated
| serde = { version = "1.0", default-features = false, features = [ | ||
| "derive", | ||
| ], optional = true } | ||
| llvm-sys = "211.0.0" |
There was a problem hiding this comment.
the proper llvm version is already being named llvm-sys, so this isn't going to work
There was a problem hiding this comment.
I'll remove and fix it as soon as possible.
Is there anything else with this issue?
…enly by myself in previous commits
|
Im sure added a testcase for it in the How we can fix it? |
|
That's really weird. I suppose you could slap #[allow(dead_code)] on it and call it a day |
Description
Added a new method
append_existing_basic_blocktoContextImplthat wraps LLVM’sLLVMAppendExistingBasicBlockfunction. This allows developers to attach an existingBasicBlockto a function, which is crucial for implementing precise control flow structures such asif,loop, andmatch. The method provides a safer and more ergonomic abstraction over the unsafe raw LLVM call.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:
BasicBlocks.append_existing_basic_blockto attach a block to the function.No regressions were observed in existing tests.
Option
No breaking changes; this is an additive method that does not affect existing API behavior.