FunctionValue.as_any_value_enum() return PointerValue variant.

[PaulJuliusMartinez]

Bug description: The AnyValue implementation for FunctionValue returns an AnyValueEnum::PointerValue variant instead of a AnyValueEnum::FunctionValue variant.

Code to reproduce:

# src/main.rs
use inkwell::context::Context;
use inkwell::values::{AnyValue, AnyValueEnum};

fn main() {
    let context = Context::create();
    let module = context.create_module("fn_or_ptr");
    let fn_type = context.f32_type().fn_type(&[], false);
    let function_value = module.add_function("fn_or_ptr", fn_type, None);
    dbg!(function_value);
    dbg!(function_value.as_any_value_enum());
    dbg!(AnyValueEnum::FunctionValue(function_value));
}

# Cargo.toml
[dependencies]
inkwell = { git = "https://github.com/TheDan64/inkwell", branch = "llvm7-0" }

This produces the following output:

[src/main.rs:8] function_value = FunctionValue {
    name: "fn_or_ptr",
    address: 0x00007fe656c17128,
    is_const: true,
    is_null: false,
    llvm_value: "\ndeclare float @fn_or_ptr()\n",
    llvm_type: "float ()*"
}
[src/main.rs:9] function_value.as_any_value_enum() = PointerValue(
    PointerValue {
        ptr_value: Value {
            name: "fn_or_ptr",
            address: 0x00007fe656c17128,
            is_const: true,
            is_null: false,
            is_undef: false,
            llvm_value: "\ndeclare float @fn_or_ptr()\n",
            llvm_type: "float ()*"
        }
    }
)
[src/main.rs:10] AnyValueEnum::FunctionValue(function_value) = FunctionValue(
    FunctionValue {
        name: "fn_or_ptr",
        address: 0x00007fdf15c17128,
        is_const: true,
        is_null: false,
        llvm_value: "\ndeclare float @fn_or_ptr()\n",
        llvm_type: "float ()*"
    }
)

Expected output: function_value.as_any_enum_value() should return a AnyValueEnum::FunctionValue variant.

LLVM version: 7.0.1 Rust version: rustc 1.33.0 (2aa4c46cf 2019-02-28)

Also, unrelated, why aren't methods get_name, print_to_string, is_null, etc., part of the AnyValue trait? This forces casting AnyValueEnums into the correct Value type (using as_<type>_value() to call any of those methods. I imagine that in many cases it's possible to know the type of the value, but would just require explicit code duplication or manual matching.

[TheDan64]

I'm not sure that there's anything we can do here as the enum variant is determined by LLVM not inkwell. I think it happens because functions sort of decay into pointers (or maybe inherit from pointers?).

re: your second question. I've only just started to add methods to the traits - but it takes time and writing tests to verify that the method works for all variants and doesn't cause any memory issues.

[cdisselkoen]

Can you clarify this statement?

I've only just started to add methods to the traits - but it takes time and writing tests to verify that the method works for all variants and doesn't cause any memory issues.

For the LLVM functions listed here, the docs say pretty clearly that

Functions in this section work on all LLVMValueRef instances, regardless of their sub-type. They correspond to functions available on llvm::Value.

From this, it seems to me that those methods should be safe to apply to anything with the AnyValue trait (or more specifically, the AsValueRef trait). But I fully believe I could be naively missing something - is there a reason to believe that applying these methods to any AsValueRef object would cause problems?

[TheDan64]

I suppose I just meant I haven't had time to add tests to verify they work properly or as expected as a wrapper. Also, I think BasicValues are far more useful/common than AnyValue so I tend to test additions to them first.

[thangnguyen-69]

I believe there is a workaround for this issue. It is true that the when a function is created in LLVM, its Type is casted into PointerType, but the ValueType stay the same. Hence we can use that ValueType to infer the function type, like what i do in this code.

    pub(crate) unsafe fn new(value: LLVMValueRef) -> Self {
            match LLVMGetTypeKind(LLVMTypeOf(value)) {
                LLVMTypeKind::LLVMFloatTypeKind
                | LLVMTypeKind::LLVMFP128TypeKind
                | LLVMTypeKind::LLVMDoubleTypeKind
                | LLVMTypeKind::LLVMHalfTypeKind
                | LLVMTypeKind::LLVMX86_FP80TypeKind
                | LLVMTypeKind::LLVMPPC_FP128TypeKind => AnyValueEnum::FloatValue(FloatValue::new(value)),
                LLVMTypeKind::LLVMIntegerTypeKind => AnyValueEnum::IntValue(IntValue::new(value)),
                LLVMTypeKind::LLVMStructTypeKind => AnyValueEnum::StructValue(StructValue::new(value)),
                LLVMTypeKind::LLVMPointerTypeKind => {
                    match LLVMGetValueKind(value){
                        LLVMValueKind::LLVMFunctionValueKind => AnyValueEnum::FunctionValue(FunctionValue::new(value).unwrap()),
                        _ =>  AnyValueEnum::PointerValue(PointerValue::new(value)),
                    }
                }
                LLVMTypeKind::LLVMArrayTypeKind => AnyValueEnum::ArrayValue(ArrayValue::new(value)),
                LLVMTypeKind::LLVMVectorTypeKind => AnyValueEnum::VectorValue(VectorValue::new(value)),
                LLVMTypeKind::LLVMFunctionTypeKind => AnyValueEnum::FunctionValue(FunctionValue::new(value).unwrap()),
                LLVMTypeKind::LLVMVoidTypeKind => {
                    if LLVMIsAInstruction(value).is_null() {
                        panic!("Void value isn't an instruction.");
                    }
                    AnyValueEnum::InstructionValue(InstructionValue::new(value))
                },
                LLVMTypeKind::LLVMMetadataTypeKind => panic!("Metadata values are not supported as AnyValue's."),
                _ => panic!("The given type is not supported."),
            }
        }

[thangnguyen-69]

@TheDan64 I want to know if this fix in your codebase is legitimate?

[TheDan64]

That might work, yea

[TheDan64]

@hardcoresummer Do you plan to put up a PR for this change?

[thangnguyen-69]

Yes, i think i will make a pull request now.

On Fri, 7 Oct 2022 at 10:01 Dan Kolsoi @.***> wrote:

@hardcoresummer https://github.com/hardcoresummer Do you plan to put up a PR for this change?

— Reply to this email directly, view it on GitHub https://github.com/TheDan64/inkwell/issues/71#issuecomment-1271049783, or unsubscribe https://github.com/notifications/unsubscribe-auth/ARTVCJXM3ICXWLQTVFN2GJTWB6G75ANCNFSM4HCCQSRQ . You are receiving this because you were mentioned.Message ID: @.***>