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

Allow bitflags v1 and v2 to coexist #894

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,8 @@ We have several packages which live in this repository. Changes are tracked sepa

### [defmt-next]

* [#894] Add optional support for bitflags v2
jonathanpallant marked this conversation as resolved.
Show resolved Hide resolved
Urhengulas marked this conversation as resolved.
Show resolved Hide resolved

* [#914] Add cargo-deny as a CI action to check crate security and licensing

### [defmt-v0.3.10] (2024-11-29)
Expand Down Expand Up @@ -843,6 +845,7 @@ Initial release
[#901]: https://github.com/knurling-rs/defmt/pull/901
[#899]: https://github.com/knurling-rs/defmt/pull/899
[#897]: https://github.com/knurling-rs/defmt/pull/897
[#894]: https://github.com/knurling-rs/defmt/pull/894
[#889]: https://github.com/knurling-rs/defmt/pull/889
[#887]: https://github.com/knurling-rs/defmt/pull/887
[#884]: https://github.com/knurling-rs/defmt/pull/884
Expand Down
2 changes: 2 additions & 0 deletions defmt/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ version = "0.3.10"
alloc = []
avoid-default-panic = []
ip_in_core = []
bitflagsv2 = ["dep:bitflagsv2"]

# Encoding feature flags. These should only be set by end-user crates, not by library crates.
#
Expand All @@ -47,6 +48,7 @@ unstable-test = [ "defmt-macros/unstable-test" ]
# defmt. Although, multiple versions of defmt might use the *same* defmt-macros.
defmt-macros = { path = "../macros", version = "=0.4.0" }
bitflags = "1"
bitflagsv2 = { package = "bitflags", version = "2", optional = true }

[dev-dependencies]
rustc_version = "0.4"
Expand Down
3 changes: 3 additions & 0 deletions defmt/src/export/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@ use crate::{Format, Formatter, Str};
pub use self::integers::*;
pub use bitflags::bitflags;

#[cfg(feature = "bitflagsv2")]
pub use bitflagsv2::bitflags as bitflagsv2;
Copy link

@vic1707 vic1707 Jan 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

may I ask for the bitflags traits (Bits & Flags) to also be re-exported.

Suggested change
pub use bitflagsv2::bitflags as bitflagsv2;
pub use bitflagsv2::{bitflags as bitflagsv2, Bits, Flags};

I need them, and other users might want them too, for a project and would prefer relying on the version installed by defmt instead of listing it manually.
Refers to #923

Copy link
Contributor

@jonathanpallant jonathanpallant Jan 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's reasonable - perhaps for v2 import the whole crate like:

pub use bitflags::{bitflags, self};

#[cfg(feature = "bitflagsv2")]
pub use bitflagsv2::{bitflags as bitflagsv2, self};

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@manushT any thoughts on this one?


pub trait UnsignedInt {}
impl UnsignedInt for u8 {}
impl UnsignedInt for u16 {}
Expand Down
40 changes: 39 additions & 1 deletion defmt/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -337,7 +337,7 @@ pub use defmt_macros::timestamp;

/// Generates a bitflags structure that can be formatted with defmt.
///
/// This macro is a wrapper around the [`bitflags!`] crate, and provides an (almost) identical
/// This macro is a wrapper around the [`bitflags!`] macro of the bitflags version 1 crate, and provides an (almost) identical
/// interface. Refer to [its documentation] for an explanation of the syntax.
///
/// [its documentation]: https://docs.rs/bitflags/1/bitflags/
Expand Down Expand Up @@ -367,6 +367,44 @@ pub use defmt_macros::timestamp;
/// ```
pub use defmt_macros::bitflags;

/// Generates a bitflags structure that can be formatted with defmt (using version 2 of the bitflags crate)
///

/// Users of the defmt crate can enable the bitflagsv2 feature by adding defmt = { features = ["bitflagsv2"] }
jonathanpallant marked this conversation as resolved.
Show resolved Hide resolved
/// to their Cargo.toml. Bitflags version 2 introduces significant improvements over version 1, including a safer
/// API with the replacement of the unsafe from_bits_unchecked method by the safe from_bits_retain method
/// and enhanced serialization support via an optional serde feature.
/// This macro is a wrapper around the [`bitflags!`][bitflagsv2] macro of the bitflags version 2 crate, and provides an (almost) identical
/// interface. Refer to [its documentation][bitflagsv2] for an explanation of the syntax.
///
/// [bitflagsv2]: https://docs.rs/bitflags/2/bitflags/macro.bitflags.html
///
/// # Limitations
///
/// This macro only supports bitflags structs represented as one of Rust's built-in unsigned integer
/// types (`u8`, `u16`, `u32`, `u64`, or `u128`). Custom types are not supported. This restriction
/// is necessary to support defmt's efficient encoding.
///
/// # Examples
///
/// The example from the bitflagsv2 crate works as-is:
///
/// ```
/// #[cfg(feature = "bitflagsv2")]
jonathanpallant marked this conversation as resolved.
Show resolved Hide resolved
/// defmt::bitflagsv2! {
/// struct Flags: u32 {
/// const A = 0b00000001;
/// const B = 0b00000010;
/// const C = 0b00000100;
/// const ABC = Self::A.bits() | Self::B.bits() | Self::C.bits()
/// }
/// }
/// defmt::info!("Flags::ABC: {}", Flags::ABC);
/// defmt::info!("Flags::empty(): {}", Flags::empty());
/// ```
#[cfg(feature = "bitflagsv2")]
pub use defmt_macros::bitflagsv2;

#[doc(hidden)] // documented as the `Format` trait instead
pub use defmt_macros::Format;

Expand Down
2 changes: 1 addition & 1 deletion firmware/qemu/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ name = "defmt-test"
harness = false

[dependencies]
defmt = { path = "../../defmt" }
defmt = { path = "../../defmt", default-features = true, features = ["bitflagsv2"]}
defmt-semihosting = { path = "../defmt-semihosting" }
defmt-test = { path = "../defmt-test" }
cortex-m = "0.7"
Expand Down
12 changes: 12 additions & 0 deletions firmware/qemu/src/bin/bitflagsv2.out
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
INFO Flags::empty(): FLAG_0
INFO Flags::empty(): Flags(0x0) (fmt::Debug)
INFO Flags::all(): FLAG_1 | FLAG_2 | FLAG_7 | FLAG_7_COPY
INFO Flags::all(): Flags(FLAG_1 | FLAG_2 | FLAG_7) (fmt::Debug)
INFO Flags::FLAG_1: FLAG_1
INFO Flags::FLAG_1: Flags(FLAG_1) (fmt::Debug)
INFO Flags::FLAG_7: FLAG_7 | FLAG_7_COPY
INFO Flags::FLAG_7: Flags(FLAG_7) (fmt::Debug)
INFO LargeFlags::ALL: MSB | ALL | NON_LITERAL
INFO LargeFlags::ALL: LargeFlags(MSB | ALL) (fmt::Debug)
INFO LargeFlags::empty(): (empty)
INFO LargeFlags::empty(): LargeFlags(0x0) (fmt::Debug)
81 changes: 81 additions & 0 deletions firmware/qemu/src/bin/bitflagsv2.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
#![no_std]
#![no_main]
#![allow(clippy::bad_bit_mask)]

use cortex_m_rt::entry;
use cortex_m_semihosting::debug;
use defmt::{bitflagsv2, Debug2Format};

use defmt_semihosting as _; // global logger

bitflagsv2! {
#[derive(Debug)]
struct Flags: u8 {
#[cfg(not(never))]
const FLAG_0 = 0b00;
const FLAG_1 = 0b01;
const FLAG_2 = 0b10;
const FLAG_7 = 1 << 7;
const FLAG_7_COPY = Self::FLAG_7.bits();
#[cfg(never)]
const CFGD_OUT = 1;
}
}

bitflagsv2! {
#[derive(Debug)]
struct LargeFlags: u128 {
const MSB = 1 << 127;
const ALL = !0;
const NON_LITERAL = compute_flag_value(0x346934553632462);
}
}

const fn compute_flag_value(x: u128) -> u128 {
x ^ 0xdeadbeef
}

#[entry]
fn main() -> ! {
defmt::info!("Flags::empty(): {}", Flags::empty());
defmt::info!(
"Flags::empty(): {} (fmt::Debug)",
Debug2Format(&Flags::empty())
);
defmt::info!("Flags::all(): {}", Flags::all());
defmt::info!("Flags::all(): {} (fmt::Debug)", Debug2Format(&Flags::all()));
defmt::info!("Flags::FLAG_1: {}", Flags::FLAG_1);
defmt::info!(
"Flags::FLAG_1: {} (fmt::Debug)",
Debug2Format(&Flags::FLAG_1)
);
defmt::info!("Flags::FLAG_7: {}", Flags::FLAG_7);
defmt::info!(
"Flags::FLAG_7: {} (fmt::Debug)",
Debug2Format(&Flags::FLAG_7)
);

defmt::info!("LargeFlags::ALL: {}", LargeFlags::ALL);
defmt::info!(
"LargeFlags::ALL: {} (fmt::Debug)",
Debug2Format(&LargeFlags::ALL)
);
defmt::info!("LargeFlags::empty(): {}", LargeFlags::empty());
defmt::info!(
"LargeFlags::empty(): {} (fmt::Debug)",
Debug2Format(&LargeFlags::empty())
);

loop {
debug::exit(debug::EXIT_SUCCESS)
}
}

// like `panic-semihosting` but doesn't print to stdout (that would corrupt the defmt stream)
#[cfg(target_os = "none")]
#[panic_handler]
fn panic(_: &core::panic::PanicInfo) -> ! {
loop {
debug::exit(debug::EXIT_FAILURE)
}
}
28 changes: 20 additions & 8 deletions macros/src/items/bitflags.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use proc_macro::TokenStream;
use proc_macro2::TokenStream as TokenStream2;
use quote::{quote, ToTokens};
use quote::{format_ident, quote, ToTokens};
use syn::parse_macro_input;

use crate::{cargo, construct};
Expand All @@ -9,7 +9,7 @@ use self::input::Input;

mod input;

pub(crate) fn expand(input: TokenStream) -> TokenStream {
pub(crate) fn expand(input: TokenStream, is_v2: bool) -> TokenStream {
let bitflags_input = TokenStream2::from(input.clone());
let input = parse_macro_input!(input as Input);

Expand All @@ -23,11 +23,13 @@ pub(crate) fn expand(input: TokenStream) -> TokenStream {
construct::crate_local_disambiguator(),
cargo::crate_name(),
);
let format_tag = construct::interned_string(&format_string, "bitflags", false, None);
let bitflags_tag = if is_v2 { "bitflagsv2" } else { "bitflags" };
let format_tag = construct::interned_string(&format_string, bitflags_tag, false, None);

let ident = input.ident();
let ty = input.ty();
let flag_statics = codegen_flag_statics(&input);
let flag_statics = codegen_flag_statics(&input, is_v2);
let bitflag_macro = format_ident!("{bitflags_tag}");
quote!(
const _: () = {
fn assert<T: defmt::export::UnsignedInt>() {}
Expand All @@ -36,7 +38,7 @@ pub(crate) fn expand(input: TokenStream) -> TokenStream {
#(#flag_statics)*
};

defmt::export::bitflags! {
defmt::export::#bitflag_macro! {
#bitflags_input
}

Expand All @@ -59,7 +61,7 @@ pub(crate) fn expand(input: TokenStream) -> TokenStream {
.into()
}

fn codegen_flag_statics(input: &Input) -> Vec<TokenStream2> {
fn codegen_flag_statics(input: &Input, is_v2: bool) -> Vec<TokenStream2> {
input
.flags()
.enumerate()
Expand All @@ -69,11 +71,21 @@ fn codegen_flag_statics(input: &Input) -> Vec<TokenStream2> {
let struct_name = input.ident();
let repr_ty = input.ty();

let _tag = if is_v2 {
"bitflagsv2_value"
} else {
"bitflags_value"
};
let sym_name = construct::mangled_symbol_name(
"bitflags_value",
_tag,
&format!("{}::{i}::{}", input.ident(), flag.ident()),
);

let bits_access = if is_v2 {
quote! { bits() }
} else {
quote! { bits }
};
quote! {
#(#cfg_attrs)*
#[cfg_attr(target_os = "macos", link_section = ".defmt,end")]
Expand All @@ -84,7 +96,7 @@ fn codegen_flag_statics(input: &Input) -> Vec<TokenStream2> {
// causes a value such as `1 << 127` to be evaluated as an `i32`, which
// overflows. So we instead coerce (but don't cast) it to the bitflags' raw
// type, and then cast that to u128.
let coerced_value: #repr_ty = #struct_name::#var_name.bits;
let coerced_value: #repr_ty = #struct_name::#var_name.#bits_access;
coerced_value as u128
};
}
Expand Down
9 changes: 8 additions & 1 deletion macros/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,14 @@ pub fn write(args: TokenStream) -> TokenStream {
#[proc_macro]
#[proc_macro_error]
pub fn bitflags(ts: TokenStream) -> TokenStream {
items::bitflags::expand(ts)
items::bitflags::expand(ts, false)
}

/* # Items */
#[proc_macro]
#[proc_macro_error]
pub fn bitflagsv2(ts: TokenStream) -> TokenStream {
items::bitflags::expand(ts, true)
}

#[proc_macro]
Expand Down