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

defmt::{panic, assert, assert_eq} #136

Closed
jonas-schievink opened this issue Aug 26, 2020 · 0 comments · Fixed by #266
Closed

defmt::{panic, assert, assert_eq} #136

jonas-schievink opened this issue Aug 26, 2020 · 0 comments · Fixed by #266
Assignees
Labels
difficulty: medium Somewhat difficult to solve priority: low Low priority for the Knurling team status: needs PR Issue just needs a Pull Request implementing the changes type: enhancement Enhancement or feature request
Milestone

Comments

@jonas-schievink
Copy link
Contributor

jonas-schievink commented Aug 26, 2020

It would be great if defmt could be used as an efficient replacement for panic!, assert!, and assert_eq!.

These could work like this: If the panic should happen, a defmt::error! is logged with the formatting arguments, and then a core::panic!(); is raised. We might need to think about the panic payload, since a bare core::panic!() is the same as core::panic!("explicit panic");, which pulls in (or might pull in) core::fmt.

Also note that we can't use stringify! to turn the assert expression into text, since the text has to go into the symbol name. That means that assert!-style macros all have to be procedural macros and stringify part of their arguments by themselves.

This won't get rid of all inefficient panics. For example, indexing something out of bounds still uses core::fmt to format the panic string (and of course all dependencies that don't use defmt will also do that). But it should still help.

@jonas-schievink jonas-schievink added the type: enhancement Enhancement or feature request label Aug 26, 2020
@jonas-schievink jonas-schievink added difficulty: medium Somewhat difficult to solve priority: low Low priority for the Knurling team status: needs PR Issue just needs a Pull Request implementing the changes labels Nov 10, 2020
@japaric japaric self-assigned this Nov 17, 2020
@jonas-schievink jonas-schievink added this to the v0.1.1 milestone Nov 17, 2020
@japaric japaric mentioned this issue Nov 17, 2020
10 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
difficulty: medium Somewhat difficult to solve priority: low Low priority for the Knurling team status: needs PR Issue just needs a Pull Request implementing the changes type: enhancement Enhancement or feature request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants