scylladb / cpp-rust-driver

API-compatible rewrite of https://github.com/scylladb/cpp-driver as a wrapper for Rust driver.
GNU Lesser General Public License v2.1
16 stars 11 forks source link

build: generate bindings for basic types #188

Closed muzarski closed 1 month ago

muzarski commented 1 month ago

Motivation

All of the numeric types were for some reason defined by us in types.rs. We should let bindgen generate bindings for them.

This is what this PR does. Removes basic types definitions from types.rs and tells bindgen to generate bindings for these types.

size_t

Second commit addresses the issue with size_t binding. There is a question regarding whether we should treat it as usize or not. Please, take a look at that.

Modules refactor

Moved all of the include!s throughout code to one place - lib.rs. Each include! now corresponds to its module.

Pre-review checklist

Lorak-mmk commented 1 month ago

I think the current version of the PR is fine (meaning size_t_is_usize should remain false). I see it as a safer option - there is no way for C definition and our definition to diverge.

Note: now the types.rs file doesn't actually define any types. We should probably remove the of the definitions from it.

Actually we could, for each generated module, have an entry in lib.rs:

mod generated_mod {
    include!(concat!(env!("OUT_DIR"), "/generated_mod.rs"));
}

That way, lib.rs would be the only file in which we use include! macro - the rest of the project could just use the modules as usual.

wprzytula commented 1 month ago

That way, lib.rs would be the only file in which we use include! macro - the rest of the project could just use the modules as usual.

Yes, please. I hate those include!s.

muzarski commented 1 month ago

I think the current version of the PR is fine (meaning size_t_is_usize should remain false). I see it as a safer option - there is no way for C definition and our definition to diverge. Note: now the types.rs file doesn't actually define any types. We should probably remove the of the definitions from it.

Actually we could, for each generated module, have an entry in lib.rs:

mod generated_mod {
    include!(concat!(env!("OUT_DIR"), "/generated_mod.rs"));
}

That way, lib.rs would be the only file in which we use include! macro - the rest of the project could just use the modules as usual.

Done. Adjusted the names of some of the generated .rs files as well.

muzarski commented 1 month ago

v2: Created cass_consistency_types and cass_batch_types module (instead of having common cass_misc_types module). Moved CassWriteType to cass_error_types module.

muzarski commented 1 month ago

Rebased on main. I'll apply @wprzytula 's suggestion in the next push