Skip to content

Swaps raw pointer types with NonNull for better safety and niches #687

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
ErisianArchitect wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Swaps raw pointer types with NonNull for better safety and niches #687

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
d25e84b
Swaps raw pointer types with NonNull for better safety and to have at…
ErisianArchitect Apr 18, 2026
0ec97f8
Fixed clippy problems. Hopefully.
ErisianArchitect Apr 18, 2026
37be92f
cargo fmt
ErisianArchitect Apr 18, 2026
f6b24e0
Added alignment check to assert_niche.
ErisianArchitect Apr 18, 2026
703c2c9
Added missing assertion.
ErisianArchitect Apr 18, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 34 additions & 21 deletions src/attributes.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,33 +1,40 @@
//! `Attribute`s are optional modifiers to functions, function parameters, and return types.

use llvm_sys::core::{
LLVMGetEnumAttributeKind, LLVMGetEnumAttributeKindForName, LLVMGetEnumAttributeValue, LLVMGetLastEnumAttributeKind,
LLVMGetStringAttributeKind, LLVMGetStringAttributeValue, LLVMIsEnumAttribute, LLVMIsStringAttribute,
};
#[llvm_versions(12..)]
use llvm_sys::core::{LLVMGetTypeAttributeValue, LLVMIsTypeAttribute};
use llvm_sys::prelude::LLVMAttributeRef;
use llvm_sys::{
LLVMOpaqueAttributeRef,
core::{
LLVMGetEnumAttributeKind, LLVMGetEnumAttributeKindForName, LLVMGetEnumAttributeValue,
LLVMGetLastEnumAttributeKind, LLVMGetStringAttributeKind, LLVMGetStringAttributeValue, LLVMIsEnumAttribute,
LLVMIsStringAttribute,
},
};

use std::ffi::CStr;
use std::{ffi::CStr, ptr::NonNull};

use crate::support::assert_niche;
#[llvm_versions(12..)]
use crate::types::AnyTypeEnum;

// SubTypes: Attribute<Enum>, Attribute<String>
// REVIEW: Should Attributes have a 'ctx lifetime?
/// Functions, function parameters, and return types can have `Attribute`s to indicate
/// how they should be treated by optimizations and code generation.
#[repr(transparent)]
#[derive(Clone, Copy)]
pub struct Attribute {
pub(crate) attribute: LLVMAttributeRef,
pub(crate) attribute: NonNull<LLVMOpaqueAttributeRef>,
}
const _: () = assert_niche::<Attribute>();

impl std::fmt::Debug for Attribute {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
if self.is_string() {
return f
.debug_struct("Attribute::String")
.field("ptr", &self.attribute)
.field("ptr", &self.as_mut_ptr())
.field("kind_id", &self.get_string_kind_id())
.field("value", &self.get_string_value())
.finish();
Expand All @@ -36,7 +43,7 @@ impl std::fmt::Debug for Attribute {
if self.is_enum() {
return f
.debug_struct("Attribute::Enum")
.field("ptr", &self.attribute)
.field("ptr", &self.as_mut_ptr())
.field("kind_id", &self.get_enum_kind_id())
.field("value", &self.get_enum_value())
.finish();
Expand All @@ -45,7 +52,7 @@ impl std::fmt::Debug for Attribute {
if self.is_type() {
return f
.debug_struct("Attribute::Type")
.field("ptr", &self.attribute)
.field("ptr", &self.as_mut_ptr())
Comment thread
ErisianArchitect marked this conversation as resolved.
.field("kind_id", &self.get_enum_kind_id())
.field("value", &self.get_type_value())
.finish();
Expand Down Expand Up @@ -84,16 +91,22 @@ impl PartialEq<Self> for Attribute {
}

impl Attribute {
/// Creates a new `Attribute` from a raw pointer.
/// Creates a new [Attribute] from a raw pointer.
///
/// # Safety
///
/// `attribute` must be non-null and point to a valid value.
pub unsafe fn new(attribute: LLVMAttributeRef) -> Self {
debug_assert!(!attribute.is_null());

Attribute { attribute }
Attribute {
attribute: unsafe { NonNull::new_unchecked(attribute) },
}
}

/// Acquires the underlying raw pointer belonging to this `Attribute` type.
pub fn as_mut_ptr(&self) -> LLVMAttributeRef {
self.attribute
self.attribute.as_ptr()
}

/// Determines whether or not an `Attribute` is an enum. This method will
Expand All @@ -111,7 +124,7 @@ impl Attribute {
/// assert!(enum_attribute.is_enum());
/// ```
pub fn is_enum(self) -> bool {
unsafe { LLVMIsEnumAttribute(self.attribute) == 1 }
unsafe { LLVMIsEnumAttribute(self.as_mut_ptr()) == 1 }
}

/// Determines whether or not an `Attribute` is a string. This method will
Expand All @@ -129,7 +142,7 @@ impl Attribute {
/// assert!(string_attribute.is_string());
/// ```
pub fn is_string(self) -> bool {
unsafe { LLVMIsStringAttribute(self.attribute) == 1 }
unsafe { LLVMIsStringAttribute(self.as_mut_ptr()) == 1 }
}

/// Determines whether or not an `Attribute` is a type attribute. This method will
Expand All @@ -153,7 +166,7 @@ impl Attribute {
/// ```
#[llvm_versions(12..)]
pub fn is_type(self) -> bool {
unsafe { LLVMIsTypeAttribute(self.attribute) == 1 }
unsafe { LLVMIsTypeAttribute(self.as_mut_ptr()) == 1 }
}

// private function to make code elsewhere easier
Expand Down Expand Up @@ -196,7 +209,7 @@ impl Attribute {
pub fn get_enum_kind_id(self) -> u32 {
assert!(self.get_enum_kind_id_is_valid()); // FIXME: SubTypes

unsafe { LLVMGetEnumAttributeKind(self.attribute) }
unsafe { LLVMGetEnumAttributeKind(self.as_mut_ptr()) }
}

/// Gets the kind id associated with an enum `Attribute`.
Expand Down Expand Up @@ -233,7 +246,7 @@ impl Attribute {
pub fn get_enum_kind_id(self) -> u32 {
assert!(self.get_enum_kind_id_is_valid()); // FIXME: SubTypes

unsafe { LLVMGetEnumAttributeKind(self.attribute) }
unsafe { LLVMGetEnumAttributeKind(self.as_mut_ptr()) }
}

#[cfg(feature = "llvm11-0")]
Expand Down Expand Up @@ -274,7 +287,7 @@ impl Attribute {
pub fn get_enum_value(self) -> u64 {
assert!(self.is_enum()); // FIXME: SubTypes

unsafe { LLVMGetEnumAttributeValue(self.attribute) }
unsafe { LLVMGetEnumAttributeValue(self.as_mut_ptr()) }
}

/// Gets the string kind id associated with a string attribute.
Expand All @@ -294,7 +307,7 @@ impl Attribute {
assert!(self.is_string()); // FIXME: SubTypes

let mut length = 0;
let cstr_ptr = unsafe { LLVMGetStringAttributeKind(self.attribute, &mut length) };
let cstr_ptr = unsafe { LLVMGetStringAttributeKind(self.as_mut_ptr(), &mut length) };

unsafe { CStr::from_ptr(cstr_ptr) }
}
Expand All @@ -315,7 +328,7 @@ impl Attribute {
assert!(self.is_string()); // FIXME: SubTypes

let mut length = 0;
let cstr_ptr = unsafe { LLVMGetStringAttributeValue(self.attribute, &mut length) };
let cstr_ptr = unsafe { LLVMGetStringAttributeValue(self.as_mut_ptr(), &mut length) };

unsafe { CStr::from_ptr(cstr_ptr) }
}
Expand Down Expand Up @@ -345,7 +358,7 @@ impl Attribute {
pub fn get_type_value(&self) -> AnyTypeEnum<'_> {
assert!(self.is_type()); // FIXME: SubTypes

unsafe { AnyTypeEnum::new(LLVMGetTypeAttributeValue(self.attribute)) }
unsafe { AnyTypeEnum::new(LLVMGetTypeAttributeValue(self.as_mut_ptr())) }
}

// private function to make code elsewhere easier
Expand Down
54 changes: 29 additions & 25 deletions src/basic_block.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
//! A `BasicBlock` is a container of instructions.

use llvm_sys::LLVMBasicBlock;
use llvm_sys::core::{
LLVMBasicBlockAsValue, LLVMBlockAddress, LLVMDeleteBasicBlock, LLVMGetBasicBlockName, LLVMGetBasicBlockParent,
LLVMGetBasicBlockTerminator, LLVMGetFirstInstruction, LLVMGetFirstUse, LLVMGetLastInstruction,
Expand All @@ -10,12 +11,13 @@ use llvm_sys::core::{
use llvm_sys::prelude::{LLVMBasicBlockRef, LLVMValueRef};

use crate::context::ContextRef;
use crate::support::to_c_str;
use crate::support::{assert_niche, to_c_str};
use crate::values::{AsValueRef, BasicValueUse, FunctionValue, InstructionValue, PointerValue};

use std::ffi::CStr;
use std::fmt;
use std::marker::PhantomData;
use std::ptr::NonNull;

/// A `BasicBlock` is a container of instructions.
///
Expand All @@ -24,11 +26,13 @@ use std::marker::PhantomData;
/// A well formed `BasicBlock` is a list of non terminating instructions followed by a single terminating
/// instruction. `BasicBlock`s are allowed to be malformed prior to running validation because it may be useful
/// when constructing or modifying a program.
#[repr(transparent)]
#[derive(PartialEq, Eq, Clone, Copy, Hash)]
pub struct BasicBlock<'ctx> {
pub(crate) basic_block: LLVMBasicBlockRef,
pub(crate) basic_block: NonNull<LLVMBasicBlock>,
_marker: PhantomData<&'ctx ()>,
}
const _: () = assert_niche::<BasicBlock>();

impl<'ctx> BasicBlock<'ctx> {
/// Create a basic block from an [LLVMBasicBlockRef].
Expand All @@ -46,15 +50,15 @@ impl<'ctx> BasicBlock<'ctx> {
assert!(!LLVMIsABasicBlock(basic_block as LLVMValueRef).is_null());

Some(BasicBlock {
basic_block,
basic_block: NonNull::new_unchecked(basic_block),
_marker: PhantomData,
})
}
}

/// Acquires the underlying raw pointer belonging to this `BasicBlock` type.
pub fn as_mut_ptr(&self) -> LLVMBasicBlockRef {
self.basic_block
self.basic_block.as_ptr()
}

/// Obtains the `FunctionValue` that this `BasicBlock` belongs to, if any.
Expand All @@ -80,7 +84,7 @@ impl<'ctx> BasicBlock<'ctx> {
/// assert!(basic_block.get_parent().is_none());
/// ```
pub fn get_parent(self) -> Option<FunctionValue<'ctx>> {
unsafe { FunctionValue::new(LLVMGetBasicBlockParent(self.basic_block)) }
unsafe { FunctionValue::new(LLVMGetBasicBlockParent(self.as_mut_ptr())) }
}

/// Gets the `BasicBlock` preceding the current one, in its own scope, if any.
Expand Down Expand Up @@ -112,7 +116,7 @@ impl<'ctx> BasicBlock<'ctx> {
pub fn get_previous_basic_block(self) -> Option<BasicBlock<'ctx>> {
self.get_parent()?;

unsafe { BasicBlock::new(LLVMGetPreviousBasicBlock(self.basic_block)) }
unsafe { BasicBlock::new(LLVMGetPreviousBasicBlock(self.as_mut_ptr())) }
}

/// Gets the `BasicBlock` succeeding the current one, in its own scope, if any.
Expand Down Expand Up @@ -145,7 +149,7 @@ impl<'ctx> BasicBlock<'ctx> {
pub fn get_next_basic_block(self) -> Option<BasicBlock<'ctx>> {
self.get_parent()?;

unsafe { BasicBlock::new(LLVMGetNextBasicBlock(self.basic_block)) }
unsafe { BasicBlock::new(LLVMGetNextBasicBlock(self.as_mut_ptr())) }
}

/// Prepends one `BasicBlock` before another.
Expand Down Expand Up @@ -178,7 +182,7 @@ impl<'ctx> BasicBlock<'ctx> {
return Err(());
}

unsafe { LLVMMoveBasicBlockBefore(self.basic_block, basic_block.basic_block) }
unsafe { LLVMMoveBasicBlockBefore(self.as_mut_ptr(), basic_block.as_mut_ptr()) }

Ok(())
}
Expand Down Expand Up @@ -213,7 +217,7 @@ impl<'ctx> BasicBlock<'ctx> {
return Err(());
}

unsafe { LLVMMoveBasicBlockAfter(self.basic_block, basic_block.basic_block) }
unsafe { LLVMMoveBasicBlockAfter(self.as_mut_ptr(), basic_block.as_mut_ptr()) }

Ok(())
}
Expand Down Expand Up @@ -241,7 +245,7 @@ impl<'ctx> BasicBlock<'ctx> {
/// assert_eq!(basic_block.get_first_instruction().unwrap().get_opcode(), InstructionOpcode::Return);
/// ```
pub fn get_first_instruction(self) -> Option<InstructionValue<'ctx>> {
let value = unsafe { LLVMGetFirstInstruction(self.basic_block) };
let value = unsafe { LLVMGetFirstInstruction(self.as_mut_ptr()) };

if value.is_null() {
return None;
Expand Down Expand Up @@ -273,7 +277,7 @@ impl<'ctx> BasicBlock<'ctx> {
/// assert_eq!(basic_block.get_last_instruction().unwrap().get_opcode(), InstructionOpcode::Return);
/// ```
pub fn get_last_instruction(self) -> Option<InstructionValue<'ctx>> {
let value = unsafe { LLVMGetLastInstruction(self.basic_block) };
let value = unsafe { LLVMGetLastInstruction(self.as_mut_ptr()) };

if value.is_null() {
return None;
Expand Down Expand Up @@ -347,7 +351,7 @@ impl<'ctx> BasicBlock<'ctx> {
// TODOC: Every BB must have a terminating instruction or else it is invalid
// REVIEW: Unclear how this differs from get_last_instruction
pub fn get_terminator(self) -> Option<InstructionValue<'ctx>> {
let value = unsafe { LLVMGetBasicBlockTerminator(self.basic_block) };
let value = unsafe { LLVMGetBasicBlockTerminator(self.as_mut_ptr()) };

if value.is_null() {
return None;
Expand Down Expand Up @@ -393,7 +397,7 @@ impl<'ctx> BasicBlock<'ctx> {
return Err(());
}

unsafe { LLVMRemoveBasicBlockFromParent(self.basic_block) }
unsafe { LLVMRemoveBasicBlockFromParent(self.as_mut_ptr()) }

Ok(())
}
Expand Down Expand Up @@ -426,7 +430,7 @@ impl<'ctx> BasicBlock<'ctx> {
return Err(());
}

LLVMDeleteBasicBlock(self.basic_block);
LLVMDeleteBasicBlock(self.as_mut_ptr());

Ok(())
}
Expand All @@ -450,7 +454,7 @@ impl<'ctx> BasicBlock<'ctx> {
/// assert_eq!(context, basic_block.get_context());
/// ```
pub fn get_context(self) -> ContextRef<'ctx> {
unsafe { ContextRef::new(LLVMGetTypeContext(LLVMTypeOf(LLVMBasicBlockAsValue(self.basic_block)))) }
unsafe { ContextRef::new(LLVMGetTypeContext(LLVMTypeOf(LLVMBasicBlockAsValue(self.as_mut_ptr())))) }
}

/// Gets the name of a `BasicBlock`.
Expand All @@ -471,7 +475,7 @@ impl<'ctx> BasicBlock<'ctx> {
/// assert_eq!(bb.get_name().to_str(), Ok("entry"));
/// ```
pub fn get_name(&self) -> &CStr {
let ptr = unsafe { LLVMGetBasicBlockName(self.basic_block) };
let ptr = unsafe { LLVMGetBasicBlockName(self.as_mut_ptr()) };

unsafe { CStr::from_ptr(ptr) }
}
Expand All @@ -482,7 +486,7 @@ impl<'ctx> BasicBlock<'ctx> {

unsafe {
LLVMSetValueName2(
LLVMBasicBlockAsValue(self.basic_block),
LLVMBasicBlockAsValue(self.as_mut_ptr()),
c_string.as_ptr(),
c_string.to_bytes().len(),
)
Expand Down Expand Up @@ -513,8 +517,8 @@ impl<'ctx> BasicBlock<'ctx> {
/// assert_eq!(branch_inst.get_operand(0).unwrap().unwrap_block(), bb2);
/// ```
pub fn replace_all_uses_with(self, other: &BasicBlock<'ctx>) {
let value = unsafe { LLVMBasicBlockAsValue(self.basic_block) };
let other = unsafe { LLVMBasicBlockAsValue(other.basic_block) };
let value = unsafe { LLVMBasicBlockAsValue(self.as_mut_ptr()) };
let other = unsafe { LLVMBasicBlockAsValue(other.as_mut_ptr()) };

// LLVM may infinite-loop when they aren't distinct, which is UB in C++.
if value != other {
Expand Down Expand Up @@ -549,7 +553,7 @@ impl<'ctx> BasicBlock<'ctx> {
/// assert!(bb1.get_first_use().is_some());
/// ```
pub fn get_first_use(self) -> Option<BasicValueUse<'ctx>> {
let use_ = unsafe { LLVMGetFirstUse(LLVMBasicBlockAsValue(self.basic_block)) };
let use_ = unsafe { LLVMGetFirstUse(LLVMBasicBlockAsValue(self.as_mut_ptr())) };

if use_.is_null() {
return None;
Expand Down Expand Up @@ -586,7 +590,7 @@ impl<'ctx> BasicBlock<'ctx> {
// Taking the address of the entry block is illegal.
self.get_previous_basic_block()?;

let value = PointerValue::new(LLVMBlockAddress(parent.as_value_ref(), self.basic_block));
let value = PointerValue::new(LLVMBlockAddress(parent.as_value_ref(), self.as_mut_ptr()));

if value.is_null() {
return None;
Expand All @@ -599,12 +603,12 @@ impl<'ctx> BasicBlock<'ctx> {

impl fmt::Debug for BasicBlock<'_> {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
let llvm_value = unsafe { CStr::from_ptr(LLVMPrintValueToString(self.basic_block as LLVMValueRef)) };
let llvm_type = unsafe { CStr::from_ptr(LLVMPrintTypeToString(LLVMTypeOf(self.basic_block as LLVMValueRef))) };
let is_const = unsafe { LLVMIsConstant(self.basic_block as LLVMValueRef) == 1 };
let llvm_value = unsafe { CStr::from_ptr(LLVMPrintValueToString(self.as_mut_ptr() as LLVMValueRef)) };
let llvm_type = unsafe { CStr::from_ptr(LLVMPrintTypeToString(LLVMTypeOf(self.as_mut_ptr() as LLVMValueRef))) };
let is_const = unsafe { LLVMIsConstant(self.as_mut_ptr() as LLVMValueRef) == 1 };

f.debug_struct("BasicBlock")
.field("address", &self.basic_block)
.field("address", &self.as_mut_ptr())
.field("is_const", &is_const)
.field("llvm_value", &llvm_value)
.field("llvm_type", &llvm_type)
Expand Down
Loading
Loading