Skip to content

Add mappings in generated Rust code to eliminate some boilerplates and improve safety #28

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

Merged
merged 6 commits into from
Sep 25, 2020

Conversation

emgre
Copy link
Contributor

@emgre emgre commented Sep 25, 2020

What was supposed to just be a check of the enums to bypass the undefined behaviour raised in #23 ended up in a large refactoring of the Rust codegen with more powerful mappings to eliminate as much manual labour as possible.

Because some mappings can happen in structs, the solution was to hide the C fields and provide public accessors/mutators to interact with them in a Rusty fashion. To help creating new structs from scratch, a MyStructFields (notice the extra "Fields") is generated for each MyStruct. This struct has all its Rusty members public and implements From<MyStructFields> for MyStruct meaning you can do something like:

let example: MyStruct = MyStructField {
   foo: MyEnum::bar,
   // ...
}.into();

The current mappings are:

  • Enums are mapped from c_int to MyEnum. The conversion makes sure that the enum value is valid and panics if it's not. This is better than the undefined behaviour of current Rust.
  • StructRefs are mapped from *const MyStruct to Option<&MyStruct>. None is mapped to std::ptr::null().
  • Durations are mapped from u64/f64 to std::time::Duration.
  • Strings are mapped from *const c_char to &std::ffi::CStr.

Also, I removed the empty safety comments in the generated code, so we'll need to add a #![allow(clippy::missing_safety_doc)] at the beginning of our ffi.rs files. For some reason, I cannot put this in the generated file.

@emgre emgre added the gen-rust Rust generator issue label Sep 25, 2020
@emgre emgre force-pushed the feature/rust-enum-check branch from c05663d to 27a6e72 Compare September 25, 2020 16:05
@jadamcrain
Copy link
Member

Those kind of crate-level annotations may only be put at the top of lib.rs.

@emgre
Copy link
Contributor Author

emgre commented Sep 25, 2020

They can be put on top of any module. The problem seems to be that we use include!() to include the generated file, so it's not exactly at the beginning of a module file.

@jadamcrain
Copy link
Member

jadamcrain commented Sep 25, 2020

Can we add a function level annotation to just ignore it?

#[allow(clippy::missing_safety_doc)]
fn ....

https://stackoverflow.com/questions/55402812/how-to-disable-a-clippy-lint-for-a-single-line-block

@emgre emgre force-pushed the feature/rust-enum-check branch from 910e981 to 6744853 Compare September 25, 2020 18:14
@emgre
Copy link
Contributor Author

emgre commented Sep 25, 2020

I added the allow line on every function now. I also added a #[allow(clippy::needless_lifetime)], because it gets just too complicated to handle the lifetime printing.

@emgre emgre merged commit 62a50e7 into master Sep 25, 2020
@jadamcrain jadamcrain deleted the feature/rust-enum-check branch September 28, 2020 21:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
gen-rust Rust generator issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants