search

TheDan64 / inkwell

It's a New Kind of Wrapper for Exposing LLVM (Safely)
https://thedan64.github.io/inkwell/
Apache License 2.0
2.38k stars 229 forks source link

Provide get_gep_source_element_type and InstructionValue -> CallSiteValue #506

Closed vaivaswatha closed 3 months ago

vaivaswatha commented 5 months ago

This PR adds two new functionality:

  1. get_gep_soruce_element_type: Takes as input a GEP instruction and returns the source element type.
  2. A try-conversion from InstructionValue to CallSite.

Related Issue

If the changes are acceptable (i.e., in the right direction), I'll create one and edit this description.

How This Has Been Tested

I've tested the change against LLVM-17, as "cargo test -F llvm17-0"

TheDan64 commented 5 months ago

@vaivaswatha there are some test failures that need to be addressed

vaivaswatha commented 5 months ago

@TheDan64 I've fixed the build failures for other versions of LLVM. But there're still tests failing with segfaults that aren't related to my change. I'm not sure what's happening.

vaivaswatha commented 5 months ago

@TheDan64 I've fixed the build failures for other versions of LLVM. But there're still tests failing with segfaults that aren't related to my change. I'm not sure what's happening.

btw, I've also added an is_const method for StructValue. The other Value types had this method already.

Would it be a good idea to also provide these methods (is_const, is_undef etc) for AnyValueEnum itself too?

TheDan64 commented 3 months ago

Hi @vaivaswatha, there are still some test failures that need to be addressed. It doesn't seem to compile even after the master branch changes have been fixed.

Would it be a good idea to also provide these methods (is_const, is_undef etc) for AnyValueEnum itself too?

If the method applies to all variants of the trait, without exception, I'd rather see it added to the AnyValue trait instead of the enum and then removed from the variant structs. That said, I'm not sure that would be correct here, maybe BasicValue?