Skip to content
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.

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

InstructionValue::new is pub(crate) instead of pub #424

Open
willtunnels opened this issue May 31, 2023 · 2 comments
Open

InstructionValue::new is pub(crate) instead of pub #424

willtunnels opened this issue May 31, 2023 · 2 comments

Comments

@willtunnels
Copy link
Contributor

willtunnels commented May 31, 2023

As of #366, the policy for wrapper types is to be able to (unsafely) convert them to/from their underlying LLVM types. (Aside: I am really glad that this proposal was accepted because I have recently needed some LLVM features that are not even exposed in the LLVM C API yet e.g. GlobalValue::setDSOLocal; upstreaming those things all the way through the LLVM C API to llvm-sys to inkwell will take a long time.)

InstructionValue::new seems to have fallen through the cracks.

EDIT: It seems like there are actually quite a few types that do not have LLVMValueRef to Self conversions. So, the thing to do might be for someone to eventually go through all the core types and make sure they have these conversions in one big pass.

@TheDan64 TheDan64 added this to the 0.3.0 milestone Jun 7, 2023
@corbanvilla
Copy link
Contributor

@TheDan64 Yes, this would be very helpful. If I submit a PR to expose the new methods on inkwell::values, would you be willing to merge it?

As per the discussion on #366, I definitely see your point about adding necessary features to inkwell rather than conversion functions. I think there are some cases though where it makes sense to convert, though... For instance, I'm working on an existing inkwell-based compiler and trying to add LLVM Code Coverage instrumentation.

A lot of the LLVM code isn't even exposed in the LLVM C-API, but it's still useful to tie into... In that way, it doesn't really belong in llvm-sys either. However, being able to interact with it through an FFI, and then get back to inkwell typesafety is pretty useful.

A few example interfaces

@TheDan64
Copy link
Owner

TheDan64 commented Dec 8, 2023

Yes, of course

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants