This adds or modifies some of the early choices that were done when
writing the pl011 crate.  Now that the implementation is safe and
readable, it's possible to look at the code as a whole, see what
looks ugly and fix it.

To see the effect on the pl011 code, jump at patches 2 and 4.  But
overall the focus is on "ugly" constant declarations of two kinds.

First, bitmasks.  These are currently written like this:

    const IRQMASK: [u32; 6] = [                
      Interrupt::E.0 | Interrupt::MS.0 | Interrupt::RT.0 | Interrupt::TX.0 | 
Interrupt::RX.0,
      ...
    }

Here the issue is threefold: 1) the repeated "Interrupt::"; 2) the
ugly ".0" and 3) the type of IRQMASK is not "Interrupt" to avoid further
".0"s elsewhere.

All this can be fixed by abstracting this kind of bitmasks, and the go-to
solution for Rust is called "bitflags".  However, it does not solve the
constant declaration part so I'm rolling my own here, while keeping it
similar to bitflags so it's not completely foreign to Rust developers.
It's a bit simpler and it has the extra expression parsing functionality,
so that the above becomes:

    const IRQMASK: [Interrupt; 6] = [                
      bits!(Interrupt: E | MS | RT | TX | RX)
      ...
    }

(this specific case can also be written simply as "Interrupt::all()",
because all bits are set, but still).


Second, while the "bilge" crate is very nice it was written with the
expectation that const traits would be stable Real Soon.  They didn't
and we're left with stuff like

    pub const BREAK: Self = Self { value: 1 << 10 };

So this series replaces "bilge" with an alternative implementation of
bitfields called (less cryptically) "bitfield-struct".  Now, there is a
trade-off here.  It is described more in detail in patch 4, but roughly
speaking:

++ bitfield-struct supports "const" well

+ bitfield-struct is much smaller than bilge, so that it is possible
  to remove a bunch of subprojects

- bitfield-struct requires manual size annotations for anything that is
  not a primitive type (bool, iNN or uNN); this is especially annoying
  for enums

It's important that the disadvantages affect only the definition of
the type.  Code that *uses* the type is the same or better, for example
the above const declaration becomes:

    pub const BREAK: Self = Self::new().with_errors(Errors::BREAK);

This "with_*()" convention is in fact the same that it's used for
VMState declarations, such as in ".with_version_id(N)", so there is some
consistency too.

Again, I do like "bilge" and I think it's usage of arbitrary-width
types like "u4" is very nice and readable.  If it ever grows "const"
capabilities it's certainly possible to come back to it, but right
now I feel that the trade-off leans towards the switch.

Paolo


Paolo Bonzini (6):
  rust: add "bits", a custom bitflags implementation
  rust: pl011: use the bits macro
  rust: qemu-api-macros: add from_bits and into_bits to
    #[derive(TryInto)]
  rust: subprojects: add bitfield-struct
  rust: pl011: switch from bilge to bitfield-struct
  rust: remove bilge crate

 rust/Cargo.lock                               |  75 +--
 rust/Cargo.toml                               |   2 +
 rust/bits/Cargo.toml                          |  19 +
 rust/bits/meson.build                         |  12 +
 rust/bits/src/lib.rs                          | 441 ++++++++++++++++++
 rust/hw/char/pl011/Cargo.toml                 |   4 +-
 rust/hw/char/pl011/meson.build                |  12 +-
 rust/hw/char/pl011/src/device.rs              |  51 +-
 rust/hw/char/pl011/src/registers.rs           | 145 +++---
 rust/meson.build                              |   4 +
 rust/qemu-api-macros/src/bits.rs              | 227 +++++++++
 rust/qemu-api-macros/src/lib.rs               |  60 ++-
 rust/qemu-api/src/vmstate.rs                  |  34 +-
 subprojects/.gitignore                        |   8 +-
 subprojects/arbitrary-int-1-rs.wrap           |  10 -
 subprojects/bilge-0.2-rs.wrap                 |  10 -
 subprojects/bilge-impl-0.2-rs.wrap            |  10 -
 subprojects/bitfield-struct-0.9-rs.wrap       |   7 +
 subprojects/either-1-rs.wrap                  |  10 -
 subprojects/itertools-0.11-rs.wrap            |  10 -
 .../bitfield-struct-0.9-rs/meson.build        |  36 ++
 subprojects/proc-macro-error-1-rs.wrap        |  10 -
 subprojects/proc-macro-error-attr-1-rs.wrap   |  10 -
 23 files changed, 917 insertions(+), 290 deletions(-)
 create mode 100644 rust/bits/Cargo.toml
 create mode 100644 rust/bits/meson.build
 create mode 100644 rust/bits/src/lib.rs
 create mode 100644 rust/qemu-api-macros/src/bits.rs
 delete mode 100644 subprojects/arbitrary-int-1-rs.wrap
 delete mode 100644 subprojects/bilge-0.2-rs.wrap
 delete mode 100644 subprojects/bilge-impl-0.2-rs.wrap
 create mode 100644 subprojects/bitfield-struct-0.9-rs.wrap
 delete mode 100644 subprojects/either-1-rs.wrap
 delete mode 100644 subprojects/itertools-0.11-rs.wrap
 create mode 100644 subprojects/packagefiles/bitfield-struct-0.9-rs/meson.build
 delete mode 100644 subprojects/proc-macro-error-1-rs.wrap
 delete mode 100644 subprojects/proc-macro-error-attr-1-rs.wrap

-- 
2.49.0


Reply via email to