mpusz / mp-units

The quantities and units library for C++
https://mpusz.github.io/mp-units/
MIT License
994 stars 79 forks source link

implement value_cast<ToQ> and value_cast<ToQP> #571

Closed burnpanck closed 2 weeks ago

burnpanck commented 1 month ago

Fixes #568

mpusz commented 1 month ago

BTW, instead of [WIP] in the title you can set PRs as "draft" until they are ready. I just did it for this PR.

burnpanck commented 1 month ago

Given that it is somewhat nontrivial to implement a correct general value_cast<ToQP>(qp) which allows adjustments in the point_origin, I believe there is value in having this in the library. The challenge is that neither FromQP nor ToQP may have enough range to reach each other's point_origin. I believe the provided implementation manages to achieve a result within std::max(FromQP::epsilon,ToQP::epsilon) of the exact conversion result, assuming qp is within the range of ToQP. Otherwise, the conversion is unspecified (any may well be undefined behaviour, e.g. if the representation is a signed integer).

burnpanck commented 1 month ago

Oh, shall I try to sqash?

burnpanck commented 4 weeks ago

I just added a bit more detail to the documentation, opting for the analogy to the value_cast<...>(q) overloads. While doing that, I re-read the quantity-point documentation, and realised that we also have a converting constructor to quantity_point which has the same power as the new value_cast<QP>. However, it doesn't share the same implementation (potentially overflow when the point-origin offset is large compared to the range of the nested quantity type). It also is conditionally implicit even for integral types, which I believe we should re-evaluate given the potential for overflow.

mpusz commented 4 weeks ago

Sure, it is great that you have time to look into this in more detail. All of this is too much for me to address in detail at once by working alone. There are still plenty of things to do in the library. I am so happy when someone comes to help me. Thank you!