Improve error messages when a constructed Quantity truncates

[hoffbrinkle]

Given some units defined as such¹:

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.

Footnotes

1. https://godbolt.org/z/GjG38c7Yc ↩

[chiphogg]

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.

[chiphogg]

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.cc uses this to test that we support/omit the same operations as the chrono library.
• Some calls to clamp in math_test.cc --- specifically, those that include exactly one shapeshifter type as one of the three arguments --- become ambiugous if every Quantity appears to be constructible from every other.
• There are a couple of QuantityPoint tests 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.

[chiphogg]

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.

[chiphogg]

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.