nagisa / rust_libloading

Bindings around the platform's dynamic library loading primitives with greatly improved memory safety.
https://docs.rs/libloading
ISC License
1.22k stars 100 forks source link

Add try_as_raw_ptr/as_raw_ptr implementations for Symbol to convert to raw pointer #154

Closed AlexanderSchuetz97 closed 1 month ago

AlexanderSchuetz97 commented 2 months ago

see https://github.com/nagisa/rust_libloading/issues/153

nagisa commented 2 months ago

Sorry this took a while to get back to. I think I would still rather have plain inherent methods for the time being over trying to anticipate what the implementations of TryInto/From might look like in the future. An especially difficult part here is the type Error = (). I would go for Infallible for these platforms, but even then it isn't particularly ergonomic from the perspective of the users who don't care about "those other platforms".

Let me know if you don't have time or desire to modify the PR, I'll try to find time for it myself in that case.

AlexanderSchuetz97 commented 2 months ago

Ill do it until the end of the week.

I will go for making it a method then that returns Option. As of now it would always yield Some. I would call it try_as_raw_ptr() on Symbol and as_raw_ptr that returns the rawptr directly on win::Symbol and unix::Symbol. is that okay for you? the future wasi::Symbol would just not have the as_raw_ptr fn in the future and would return None for the crate::Symbol call. This way you dont need an error type. You could still later add try_into_raw_ptr with a proper Error enum should that be necesarry. The option fn i would add now would then be for the case where the library user cares not for the type of failure. That should be forward compatible for ever.

Please tell me if you agree or what I should change for my suggestion so I can begin.

AlexanderSchuetz97 commented 1 month ago

I have updated the PR please review @nagisa

AlexanderSchuetz97 commented 1 month ago

Thank you!