Skip to content
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

c2rust-bitfields-derive: Support const initialization #1027

Open
kkysen opened this issue Sep 22, 2023 · 1 comment
Open

c2rust-bitfields-derive: Support const initialization #1027

kkysen opened this issue Sep 22, 2023 · 1 comment
Assignees

Comments

@kkysen
Copy link
Contributor

kkysen commented Sep 22, 2023

In rav1d, we need const initialization of bitfield structs to initialization statics. Otherwise, the static has to be mut, which is unsafe to access (or else you have to pay a runtime cost). For example, see this code: https://github.com/memorysafety/rav1d/blob/4029b17b6762d3dcf75c8ac7a592666d483ba78d/src/ipred_prepare_tmpl_8.rs#L273

Since const fns can't have &muts, the setter fns generated can't be made const, but we can add with setters that return an updated struct, and these can be made const. And we might as well make all of the getters const fns, too.

It seems like the relevant code is around here:

let q = quote! {
#[automatically_derived]
impl #struct_ident {
#(
/// This method allows you to write to a bitfield with a value
pub fn #method_name_setters(&mut self, int: #field_types_setter_arg) {
use c2rust_bitfields::FieldType;
let field = &mut self.#field_names_setters;
let (lhs_bit, rhs_bit) = #field_bit_info_setters;
int.set_field(field, (lhs_bit, rhs_bit));
}
/// This method allows you to read from a bitfield to a value
pub fn #method_names(&self) -> #field_types_return {
use c2rust_bitfields::FieldType;
type IntType = #field_types_typedef;
let field = &self.#field_names_getters;
let (lhs_bit, rhs_bit) = #field_bit_info_getters;
<IntType as FieldType>::get_field(field, (lhs_bit, rhs_bit))
}
)*
}
};
.

@kkysen
Copy link
Contributor Author

kkysen commented Sep 23, 2023

Changing the generated code to create const fns is non-trivial since it uses a lot of traits, and traits don't allow const fns. Thus, I rolled my own version in memorysafety/rav1d#497, and it was actually really simple. I'm not sure why the implementation here needs traits at all, since the logic is pretty simple.

kkysen added a commit to memorysafety/rav1d that referenced this issue Sep 25, 2023
The few remaining cases are mostly around storing raw pointers. Since
these are often just strings, these can be fixed (but are mostly in
`tools/`). The other case is when a `fn run_static_initializers` is
generated since the type is a `BitfieldStruct` and the setter `fn`s are
not `const fn`s. I've opened
immunant/c2rust#1027 for this.
kkysen added a commit to memorysafety/rav1d that referenced this issue Sep 25, 2023
…erive(BitfieldStruct)]` with `bools_bitfield_struct!` for `const fn`s (#497)

`#[derive(c2rust_bitfields::BitfieldStruct)]` doesn't generate `const
fn`s (immunant/c2rust#1027), so it can't be
used in a `const` block to initialize a `static`. Thus, the `static` has
to made `mut` and `unsafe` and initialized at runtime.

I opened immunant/c2rust#1027 and looked into
making the generated `fn`s `const`, but they used a lot of `trait`s,
which can't contain `const fn`s, so it seemed like non-trivial work. I
also tried looking for other off-the-shelf bitfield crates, but they all
either didn't support `const fn`s or still relied on `trait`s within
them, and thus only could create `const fn`s with `nightly` features.

Thus, I rolled my own `macro_rules!` macro, `bools_bitfield_struct!`
that generates `const fn`s. The macro is specific for bitfields that are
always `bool`s, which makes things a bit simpler and have a cleaner API.
And it also doesn't have to worry about being `#[repr(C)]` at all, which
might makes things a bit simpler, too. Overall, though, it's quite a
simple macro, so it seemed much easier to do it this. Since `rav1d` only
used bitfields here, I kept the macro private in this file, but if we
ever need bitfields that are all `bool`s elsewhere, we can extract the
macro. The `c2rust-bitfields` dependency is also now removed as it's
unused.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant