feat: value_cast<Unit, Representation> added

[mpusz]

No description provided.

[JohelEGP]

The trend to require explicit units indeed results in safer code that doesn't unexpectedly change meaning over time.
However, consider the inverse, where price1 is represented with std::int64_t, and you're casting to double.
You probably want the representation conversion first, and then the scaling for the unit.

[mpusz]

Maybe I worded this wrongly in FAQ, but I think it will also work with the current implementation. I have to confirm that.

[mpusz]

Confirmed!

std::cout << value_cast<si::second, double>(42 * us) << "\n";

prints:

4.2e-05 s

[JohelEGP]

Is the representation conversion happening first?
What about something more extreme like value_cast<si::nanosecond, double>(42 * Ts)?

[mpusz]

The trend to require explicit units indeed results in safer code that doesn't unexpectedly change meaning over time.

Please note that it is not to future-proof the cast but to make the conversion correct in the first place. Without it, the value was truncated, and the result was invalid.

[mpusz]

What about something more extreme like value_cast<si::nanosecond, double>(42 * Ts)?

std::cout << value_cast<si::nano<si::second>, double>(42 * Ts) << "\n";

prints:

4.2e+22 ns

[mpusz]

I just realized that we could try a similar practice to https://abseil.io/docs/cpp/guides/strings#adaptation-to-returned-types here. Although I do not know if it would not be an over-engineering? With this, we could just write value_cast<std::int64_t>(q), and the unit would be deduced by a proxy type returned from this expression. However, the idea of returning a proxy type from this expression is quite controversial.

[JohelEGP]

Yes, and it doesn't work if you don't have a concrete target type.
See https://wg21.link/P0672.

[chiphogg]

Yes, and it doesn't work if you don't have a concrete target type. See https://wg21.link/P0672.

What is the status of that proposal? Given that it's quite old now, I'm guessing it's gone dormant.

[chiphogg]

The thrust of this PR is to make the unit into a mandatory first template argument for value_cast, right?

If we land this, what is the way for users to change the rep without repeating the unit (similar to Au's rep_cast)?

[JohelEGP]

Yes, and it doesn't work if you don't have a concrete target type. See https://wg21.link/P0672.

What is the status of that proposal? Given that it's quite old now, I'm guessing it's gone dormant.

Yeah, you can't even find it at https://github.com/cplusplus/papers/issues?q=is%3Aissue+P0672.

[mpusz]

If we land this, what is the way for users to change the rep without repeating the unit (similar to Au's rep_cast)?

This is exactly what we want to prevent. Such an interface is error-prone as I proved it in https://godbolt.org/z/M8WaeK3f3 and described in the FAQ section in this PR.

Do you think that we can somehow expose such an interface but still prevent such issues? The only idea I have is to use something like: https://abseil.io/docs/cpp/guides/strings#adaptation-to-returned-types which sucks.

[chiphogg]

I agree that the example shows it's error prone, but I'm not sure how error prone.

The downside of removing all rep_cast-like interfaces (besides just adding more nuisance typing) is that it obscures intent. It becomes hard to tell whether something is a rep_cast without looking closely at other parts of the program. And it also introduces a new opportunity for error, because a user can supply the wrong unit by mistake.

It's an analogous problem to Au's old "explicit rep" syntax for forcing conversions. If q is a Quantity<U, R>, and q.in(other_unit) wouldn't compile, then you had to say q.in<R>(other_unit), repeating the rep and obscuring the intent. It's the reason we introduced q.coerce_as(other_unit).

On the other hand... experience suggests that rep_cast is not especially common.

So, does the value in preventing these bugs outweigh the extra typing and the obscuring of intent? I think either answer could be reasonable. For my part, I'm planning to keep rep_cast in Au.

[chiphogg]

All that said --- I think introducing a syntax that simultaneously specifies the target unit and rep is a long-needed and welcome addition. I'd prefer the syntax listed in the table from #477, but I think the most important thing is to simply have some way to spell that.

[chiphogg]

The table: #477 (comment) or #477 (comment).

This one: #477 (comment)

i.e., the fuller, more comprehensive one.

[mpusz]

Thanks for your feedback.

So, does the value in preventing these bugs outweigh the extra typing and the obscuring of intent? I think either answer could be reasonable.

Yeah, this is why I created this PR rather than doing the change directly on the mainline.

I think introducing a syntax that simultaneously specifies the target unit and rep is a long-needed and welcome addition.

I am not so sure if that is that needed. In many cases it is still safer to write value_cast<int>(q.in(U)) to express that we are forcing a representation type change but want to do not-truncating unit conversion.

I'd prefer the syntax listed in the table from #477, but I think the most important thing is to simply have some way to spell that.

Yes, me too. But the template disambiguator is preventing that for now.

I am not sure what to do with it now. The new function is needed because this issue just proved it can't be

[chiphogg]

I think introducing a syntax that simultaneously specifies the target unit and rep is a long-needed and welcome addition.

I am not so sure if that is that needed. In many cases it is still safer to write value_cast<int>(q.in(U)) to express that we are forcing a representation type change but want to do not-truncating unit conversion.

Ideally, the unit-and-rep syntax should be not-truncating. In Au, it has always been truncating, but this was a mistake on my part that stemmed from conflating two different aspects of the static_cast that I took my inspiration from: its "explicit type" aspect, and its "forcing" aspect. The solution is to introduce a "force" or "coerce" word for the versions that don't protect against truncation. (I am gradually working to fix this mistake, although I have to move slowly because it's a breaking change.)

I also think it's critically important to be able to specify the destination type at the time the conversion takes place. For example, consider value_cast<uint64_t>((1u * km).in(nm)). Because we do the unit conversion first (in a type that can't hold the result), we get 3567587328 nm. If we specified the unit and destination type simultaneously, we could perform the conversion in the wider type.

So, to sum up:

• Providing a syntax with simultaneous unit-and-destination-type is critically important.
• This syntax shouldn't skip truncation checks unless we ask it to, via "coerce" (or a synonym).