-
Notifications
You must be signed in to change notification settings - Fork 217
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
Fixed triggering a debug-assertion during scan #2612
Conversation
Failing tests seem to be random and unrelated - will just rerun them once GitHub calms down |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should also remove the debug assert and place a warning there instead
I disagree, making a mistake will possibly become unsound if we don't panic. That assert is verifying an unsafe precondition. What we may do is keep the debug_assert, but warn in release, but if we do everything correctly that warning will never print and just takes up code space. |
Probably a usual |
The standard library also uses debug asserts for this purpose, and misuse is UB in release - same as we would do. We can also enable debug assertions for release builds. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alright, lets keep the debug_assert then, but can we also change this to true in hil-test and examples please
We should at least test with debug-assertions enabled - then it's fine but an assert which we'll never see is ... not of much use |
We can recommend using |
e7bb3c9
to
2009cec
Compare
Thank you for your contribution!
We appreciate the time and effort you've put into this pull request.
To help us review it efficiently, please ensure you've gone through the following checklist:
Submission Checklist 📝
cargo xtask fmt-packages
command to ensure that all changed code is formatted correctly.CHANGELOG.md
in the proper section.Extra:
Pull Request Details 📖
Description
Fixes #2611
skip-changelog
because of the change inesp-hal
Testing
Change that assert from
debug_assert_eq
toassert_eq
and just run thewifi-dhcp
example