Improve error messages when a constructed Quantity truncates
Given some units defined as such^1:
struct PerMille : decltype(Percent{} / mag<10>());
struct PerMyriad : decltype(PerMille{} / mag<10>());
struct PerCentMille : decltype(PerMyriad{} / mag<10>());
struct PartsPerMillion : decltype(PerCentMille{} / mag<10>());
struct PartsPerBillion : decltype(PartsPerMillion{} / mag<1'000>());
struct PartsPerTrillion : decltype(PartsPerBillion{} / mag<1'000>());
struct PartsPerQuadrillion : decltype(PartsPerTrillion{} / mag<1'000>());
struct PartsPerQuintillion : decltype(PartsPerQuadrillion{} / mag<1'000>());
Trying to assign from a QuantityI64<PartsPerQuintillion> to a QuantityI64<PartsPerBillion> results in an error message that says there isn't a constructor available:
note: candidate function (the implicit move assignment operator) not viable: no known conversion from 'Quantity<av::embedded::firmware::protocol::ptp::PartsPerQuintillion, [...]>' to 'Quantity<av::embedded::firmware::protocol::ptp::PartsPerBillion, [...]>' for 1st argument
Manually requesting the conversion using .as(PartsPerBillion{}) yields a more useful error:
error: static assertion failed due to requirement 'IMPLICIT_OK': Dangerous conversion for integer Rep! See: https://aurora-opensource.github.io/au/main/troubleshooting/#dangerous-conversion
Which can then be resolved with a .coerce_as. It would be useful to get the conversion error in the first instance.
One thing I was thinking about here: if we simplify this and just delegate to .as, then it will affect the results of std::is_constructible for various Quantity types. The better error messages are nice, but I'm not sure we want to give up the ability to query these traits. Something to think about.
I tried simplifying the constructors and running our tests, and uncovered some concrete use cases that depend on the status quo.
- Basically all of
chrono_policy_validation_test.ccuses this to test that we support/omit the same operations as the chrono library. - Some calls to
clampinmath_test.cc--- specifically, those that include exactly one shapeshifter type as one of the three arguments --- become ambiugous if everyQuantityappears to be constructible from every other. - There are a couple of
QuantityPointtests that explicitly check this trait.
This isn't necessarily a hard deal-breaker, but it shows some of the work we'll need to do and some of the things we'll need to consider if we want to make this happen.
Ah! I understand better now.
I was confused because I had explicitly deleted the constructor for disallowed conversions, which should give a nice, clean error message.
The real problem was that I made that deleted constructor explicit, which prevents it from being considered in conversions like this. If we take that out, we get something more like this:
Directing the user to the line should be good enough, I hope. We can tweak the accompanying comment. We can also add a section to the troubleshooting doc.
Before I put up a PR for review, I'll try to find time to double check that there's no compile time impact.
Hmm... this still isn't a complete solution, because it still has the problem with making the clamp calls ambiguous. 🤔
Not yet sure where to go next.