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

bitflags! support #307

Closed
jonas-schievink opened this issue Dec 15, 2020 · 4 comments · Fixed by #528
Closed

bitflags! support #307

jonas-schievink opened this issue Dec 15, 2020 · 4 comments · Fixed by #528
Labels
breaking change fix / feature / improvement involves a breaking change and needs to wait until next minor version difficulty: hard Fairly difficult to solve priority: low Low priority for the Knurling team status: needs design This feature needs design work to move forward type: enhancement Enhancement or feature request

Comments

@jonas-schievink
Copy link
Contributor

Currently, using #[derive(Format)] on a bitflags!-generated type results in output that looks like Bitflags { bits: 3564356 }, which isn't very readable.

It would be nice if defmt offered a convenient way to format bitflags the same way as the generated Debug implementation. Doing this efficiently probably requires adding first-class support for bitflags.

@jonas-schievink jonas-schievink added type: enhancement Enhancement or feature request priority: low Low priority for the Knurling team status: needs design This feature needs design work to move forward difficulty: hard Fairly difficult to solve breaking change fix / feature / improvement involves a breaking change and needs to wait until next minor version labels Dec 15, 2020
@jonas-schievink
Copy link
Contributor Author

jonas-schievink commented Jan 13, 2021

I think the simplest way to do this (in terms of additional API surface) would be to add a bitflags! macro that has the same interface as the one from the bitflags crate, and in fact just forwards its arguments to that (so that any improvements to the bitflags crate are picked up automatically).

That macro would then use some internal magic to also generate an efficient Format impl.

@jonas-schievink
Copy link
Contributor Author

Ah, this is going to be much more difficult to implement efficiently since bitflags can be arbitrary constants, but a proc macro can't access the final value.

@Urhengulas Urhengulas added this to the v0.3.0 milestone Jun 18, 2021
@jonas-schievink jonas-schievink removed this from the v0.3.0 milestone Jun 29, 2021
@jonas-schievink
Copy link
Contributor Author

jonas-schievink commented Jun 29, 2021

Removing from the milestone as I don't think this will happen in 0.3

@jonas-schievink
Copy link
Contributor Author

I still really want this, since nicely-formatted bitflags are incredibly useful. I think one possible way to support this would be to create a static per bitflags constant that stores the corresponding mask, and then reading that back out (somehow) in the decoder. They can be stored in a separate section to not use up any defmt format indices. For example:

defmt::bitflags! {
    pub struct Flags: u8 {
        const LE_LIMITED_DISCOVERABLE = 1 << 0;
        const LE_GENERAL_DISCOVERABLE = 1 << 1;
    }
}

This would create 2 additional statics initialized to 1 << 0 and 1 << 1, respectively. The symbol name would contain the name of the constant, so LE_LIMITED_DISCOVERABLE and LE_GENERAL_DISCOVERABLE. It also has to contain some unique identifier so that multiple bitflags types with the same constant names are supported, but Symbol already handles this.

The decoder would then read the static's initializers out of the binary (which is somewhat tricky, and not something we currently do, but should be implementable), and essentially run bitflags' Debug impl code at runtime: for each const defined for the bitflags type, check if all the bits are set, and if so, print the constant's name.

The runtime encoding of Flags would then be maximally efficient: just its format string index followed by Flags' raw bits.

@jonas-schievink jonas-schievink mentioned this issue Jul 10, 2021
1 task
@bors bors bot closed this as completed in 5089314 Jul 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change fix / feature / improvement involves a breaking change and needs to wait until next minor version difficulty: hard Fairly difficult to solve priority: low Low priority for the Knurling team status: needs design This feature needs design work to move forward type: enhancement Enhancement or feature request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants