-
Notifications
You must be signed in to change notification settings - Fork 157
use u8 for field array offset instead of usize #495
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
Conversation
r? @therealprof (rust-highfive has picked a reviewer for you, use r? to override) |
cc @adamgreig |
Is the idea that field arrays can never have >255 elements because registers are only 32 (or maybe 64) bits wide, so there's no point using a large usize when a u8 will always work? |
I'd also like to see what the concrete benefits are. Switching from |
Yes, I forget this when implemented. As field arrays are not really arrays, I don't think someone will use big usize accessing them. |
It's only used in bit shifting arithmetic, not as a slice index; this PR changes the generated code from something like: pub unsafe fn pin(&self, n: usize) -> PIN_R {
PIN_R::new(((self.bits >> n) & 0x01) != 0)
} to something like: pub unsafe fn pin(&self, n: u8) -> PIN_R {
PIN_R::new(((self.bits >> n) & 0x01) != 0)
} where in this case @burrbull, do any of the tests in the CI actually use field arrays? |
Fair enough, but in that case it's also unlikely to provide any benefit. |
We haven't actually shipped an svd2rust since field array support was added in #400, so I see this as more cleaning up the API for it before release. But, yes, I don't know if there's any benefit to swapping to u8 for this. If anything maybe it should use the same type as the registers to make it line up with self.pins etc? |
risk-v k210 should use them, but I'm not sure is test contain actual version of SVD. |
No description provided.