pepyakin / binaryen-rs

Binaryen bindings for Rust.
Apache License 2.0
74 stars 17 forks source link

Hide from_raw from the public API #26

Closed axic closed 5 years ago

pepyakin commented 5 years ago

Is there any rationale behind that? I think it is plausible that somebody might want to create a module from a raw module handle.

axic commented 5 years ago

I didn't notice ffi is pub. I thought it was private, hence no way to instantiate a raw module handle.

In that case you can ignore this PR, though in general I think it is better to remove the lowlevel/unsafe API.

pepyakin commented 5 years ago

though in general I think it is better to remove the lowlevel/unsafe API.

Yeah I understand what you mean. I think general Rust philosophy on this one is to provide safe crates in general but still provide low-level unsafe bindings. I think this is because high-level bindings are usually come with some sort of trade off. Sometimes it is because Rust opionated lifetime managment and it's not always maps cleanly to patterns used in C/C++. In this case you might want to fallback to some easier approach like using Rc, which is less performant but might allow you to provide a more convenient user API which might be enough for the user for 99% cases.

If some users actually don't care about API but want every bit of performance they can use low level bindings directly or (most likely) do their own wrapping that suits them. For this project they can use binaryen-sys for example.

This pattern is actually super common for wrapping foreign code.

But... some of users might also want to re-use functionality provided by this crate as well. For that, people often provide escape hatch to allow people leverage the functionality of the library. In our case this escape hatch is from_raw : )

But don't worry users can't just accidently use from_raw and then spawn nosal daemons from their nose. If they try to use it, the compiler will complain that they should use unsafe block and unsafe block is kinda special in Rust: when you use it you're on your own and compiler will not help you. Every time you use it you'd better to leave a proof (in a form of a comment at least) that this is actually safe. Usually, good libraries leave a comment what preconditions they expect from the users and users is expected fulfy these preconditions. We unfortunatelly don't provide them at the moment ; ( Some invariant I can see that this pointer shouldn't be NULL and this pointer should be obtained by some constructor of Module.