-
Notifications
You must be signed in to change notification settings - Fork 236
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
expose functions for working with LLVMContextRef for interoperability with other libraries #518
Conversation
Obviously, tests are failing not due to this PR :/ |
Yes, I understand that those are safe functions. I intentionally marked
them as unsafe, because they shouldn't be used by sole inkwell users. And
those getters will be used in unsafe context either way, so it doesn't
actually matter
…On Sat, Jul 20, 2024, 19:38 Dan Kolsoi ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In src/context.rs
<#518 (comment)>:
> @@ -1321,7 +1338,23 @@ pub struct ContextRef<'ctx> {
}
impl<'ctx> ContextRef<'ctx> {
- pub(crate) unsafe fn new(context: LLVMContextRef) -> Self {
+ /// Get raw [`LLVMContextRef`].
+ ///
+ /// # Safety
+ ///
+ /// This function is exposed only for interoperability with other LLVM IR libraries.
+ /// It's not intended to be used by most users, hence marked as unsafe.
+ pub unsafe fn raw(&self) -> LLVMContextRef {
Same here; calling the raw method alone can't produce undefined behavior
since it just returns a pointer. Unsafety is in how it might be used
—
Reply to this email directly, view it on GitHub
<#518 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AR5SO7SAGCC5L5FGY3BX63LZNJ77RAVCNFSM6AAAAABLE3BZKOVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDCOJQGAYTCMBUGI>
.
You are receiving this because you were assigned.Message ID:
***@***.***>
|
We should not mark the raw methods as unsafe since there's no unsafety that can result from using them alone. See the std lib for example which has safe methods which return raw ptrs: https://doc.rust-lang.org/std/vec/struct.Vec.html#method.into_raw_parts |
Fixed |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
Description
This makes some functions in Context and ContextRef
pub
instead ofpub(crate)
.Also adds getter for raw LLVMContextRef
How This Has Been Tested
Tested with llvm18-0 in my code that uses
melior
,mlir-sys
andinkwell
together.Checklist