Enforce register value constraints #120
Conversation
src/error.rs
Outdated
| } | ||
|
|
||
| pub(crate) fn check_reset_value( | ||
| size: &Option<u32>, |
There was a problem hiding this comment.
That's how it's getting stored on RegisterProperties, so I figured I'd just take a reference. I can switch to non-ref if you think it's more ergonomic.
There was a problem hiding this comment.
I do think a reference to an Option of a primitive type is a weird and unergonomic choice, yes. Not sure how much efforts it would be to change that but I certainly would prefer less ampersands in new code.
There was a problem hiding this comment.
Sounds good, just pushed with references switched to plain Option<u32>s.
|
Maybe it makes sense to add cargo feature something like |
I feel like |
|
Resolve conflicts, please. |
Enforce register size, reset, and mask value agreement. If the provided reset value or mask extend past the register size, or the provided reset value would conflict with the mask, then fail parsing.
Done. Thank you for the reviews! |
|
bors r+ |
120: Enforce register value constraints r=burrbull a=gkelly Enforce register size, reset, and mask value agreement. If the provided reset value or mask extend past the register size, or the provided reset value would conflict with the mask, then fail parsing. --- I saw that there are some tests from CMSIS that are disabled because of constraint violations due to a check like this. There are also a couple of ATSAMD51 tests (at least) that will fail because of this that are currently enabled. Co-authored-by: Garret Kelly <[email protected]>
|
Build failed: |
|
Add and fix |
Done.
Done. Apologies, I wasn't building with |
|
bors r+ |
|
Build succeeded: |
3 participants
Enforce register size, reset, and mask value agreement. If the provided
reset value or mask extend past the register size, or the provided reset
value would conflict with the mask, then fail parsing.
I saw that there are some tests from CMSIS that are disabled because of constraint violations due to a check like this. There are also a couple of ATSAMD51 tests (at least) that will fail because of this that are currently enabled.