Fix #672: Implement TryFrom<PointerValue> for GlobalValue#676
Fix #672: Implement TryFrom<PointerValue> for GlobalValue#676meftunca wants to merge 3 commits intoTheDan64:masterfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Adds a fallible conversion API to obtain a GlobalValue from a PointerValue, addressing #672 by checking the underlying LLVM value via LLVMIsAGlobalVariable.
Changes:
- Import
std::convert::TryFrominglobal_value.rs. - Implement
TryFrom<PointerValue<'ctx>> for GlobalValue<'ctx>usingLLVMIsAGlobalVariablefor validation.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| impl<'ctx> TryFrom<PointerValue<'ctx>> for GlobalValue<'ctx> { | ||
| type Error = (); | ||
|
|
||
| fn try_from(value: PointerValue<'ctx>) -> Result<Self, Self::Error> { | ||
| let is_global = unsafe { !llvm_sys::core::LLVMIsAGlobalVariable(value.as_value_ref()).is_null() }; | ||
| if is_global { | ||
| unsafe { Ok(GlobalValue::new(value.as_value_ref())) } | ||
| } else { | ||
| Err(()) |
There was a problem hiding this comment.
TryFrom<PointerValue> currently only accepts values recognized by LLVMIsAGlobalVariable, which will return Err(()) for function pointers. This breaks a natural round-trip like GlobalValue::try_from(fn_value.as_global_value().as_pointer_value()) even though FunctionValue::as_global_value() returns a GlobalValue. Consider broadening the check to also accept functions (e.g., via LLVMIsAFunction / LLVMGetValueKind) or otherwise clarifying/renaming the type/impl so it’s explicit that this conversion is only for global variables.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
| type Error = (); | ||
|
|
||
| fn try_from(value: PointerValue<'ctx>) -> Result<Self, Self::Error> { | ||
| let is_global = unsafe { !llvm_sys::core::LLVMIsAGlobalValue(value.as_value_ref()).is_null() }; |
There was a problem hiding this comment.
Please import the LLVMIsAGlobalValue type and call it without the full path
| impl<'ctx> TryFrom<PointerValue<'ctx>> for GlobalValue<'ctx> { | ||
| type Error = (); | ||
|
|
||
| fn try_from(value: PointerValue<'ctx>) -> Result<Self, Self::Error> { |
There was a problem hiding this comment.
Perhaps it would make sense to have a GlobalValue::from_pointer_value function? That way it's more apparent in the API.
There was a problem hiding this comment.
Also, PointerValue implements Copy, so it's not really necessary to return the value on error.
Fixes #672 by adding fallible conversion from
PointerValuetoGlobalValueutilizingLLVMIsAGlobalVariable.