Make downcasting safer

[nyurik]

Per discussion with @Sh3Rm4n at https://github.com/rust-embedded/embedded-hal/commit/d5f9747a060ca4109d215f512aa80a89ccb90df0#r133323441

[Dirbaio]

the cast should never overflow as long as the user gives a valid fraction (witih num <= denom). IMO we shouldn't take the performance hit.

[nyurik]

@Dirbaio is there any mechanism to catch user errors like assert! or debug_assert! that rust embedded tends to use? As the saying goes, if there is a way to make a mistake, someone will make it, so it might be good to notify user of an error in some way?

Comparing assembly, on x64 it does generate two additional assembly instructions: https://rust.godbolt.org/z/oPs59nEnY - but I wouldn't know how significant it is. If you think this is fine, should i add an #[allow(...)] to it? BTW, it might make sense to add a bunch of [lints.clippy] to Cargo.toml (even though it will produce a warning on older Rust compilers, it shouldn't affect anything, and it will auto-work on the newer ones)

[Sh3Rm4n]

If you think this is fine, should i add an #[allow(...)] to it

I'd propose to use #[allow(...)] on it and adding a comment, why it is no problem to cast here (pointing out the API contract, that num is less than or equal to denom

[Dirbaio]

debug_assert! sounds good to me, the user has the option of disabling them for optimized builds. We could add debug_assert!(num <= denom) to check the fraction supplied by the user is OK, and debug_assert!(val <= u16::MAX) before the cast to check it's not out of bounds (though that's impossible if the 1st assert passed)

[nyurik]

Updated the code, now uses debug_assert. There are a few things that keep bothering me (somewhat unrelated):

• Would it make sense to use thiserror crate?
• The code uses Error everywhere - would it make sense to rename it to HalError? See clippy lint
• Would it make sense to add a justfile in the root - so that developers can run things like just test without thinking of all the various cargo flags? This is very similar to a Makefile, but far simpler and frequently used, especially in the Rust projects

[Dirbaio]

these are indeed unrelated, can you open separate issues?

Would it make sense to use thiserror crate?

we have an error trait, the HAL prvides the actual type, so I don't think there's much we gain from it. Also std::error::Error is not available in core.