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

static av1_intra_prediction_edges: Make non-mut by replacing #[derive(BitfieldStruct)] with bools_bitfield_struct! for const fns #497

Merged
merged 3 commits into from
Sep 25, 2023

Conversation

kkysen
Copy link
Collaborator

@kkysen kkysen commented Sep 23, 2023

#[derive(c2rust_bitfields::BitfieldStruct)] doesn't generate const fns (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 fns const, but they used a lot of traits, which can't contain const fns, 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 fns or still relied on traits within them, and thus only could create const fns with nightly features.

Thus, I rolled my own macro_rules! macro, bools_bitfield_struct! that generates const fns. The macro is specific for bitfields that are always bools, 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 bools elsewhere, we can extract the macro. The c2rust-bitfields dependency is also now removed as it's unused.

@kkysen
Copy link
Collaborator Author

kkysen commented Sep 23, 2023

Mostly superceded by #501, but I'm leaving this open (and intending to merge in order) so the history doesn't get overly complicated.

…`macro_rules!` macro sepcifically for `bool`s and with all `const fn`s so `static av1_intra_prediction_edges` can be `const` initialized.
…nd map to the `av1_intra_prediction_edge` array for brevity in the initializer.
@kkysen kkysen force-pushed the kkysen/mod-ipred_prepare-statics-cleanup branch from 1ae8720 to a726357 Compare September 25, 2023 21:56
@kkysen kkysen force-pushed the kkysen/static-av1_intra_prediction_edges-const-init branch from aa70282 to dd26edf Compare September 25, 2023 21:56
@randomPoison
Copy link
Collaborator

I wouldn't normally want to add a custom macro for this, but since you're ultimately replacing it with the bitflags crate I'll approve this one as-is so that you can preserve merge order 👍

Base automatically changed from kkysen/mod-ipred_prepare-statics-cleanup to main September 25, 2023 22:55
@kkysen kkysen merged commit 1b86c65 into main Sep 25, 2023
16 checks passed
@kkysen kkysen deleted the kkysen/static-av1_intra_prediction_edges-const-init branch September 25, 2023 22:56
kkysen added a commit that referenced this pull request Sep 26, 2023
I wanted to test out using `bitflags!` for things that were manually
bitflags, as there are some other uses of this, too.

In hindsight, I think I could've also made #497 just use `bitflags!`
instead.
kkysen added a commit that referenced this pull request Sep 26, 2023
…501)

After using `bitflags!` for #500, I realized I could use it for #497
instead of rolling my own version (using `bool` bitfields). So this
supercedes #497, but I'm leaving that open so the history doesn't get
overly complicated.
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

Successfully merging this pull request may close these issues.

None yet

2 participants