fix: silent lossy conversions

[JohelEGP]

Consider quantity<dim_length, metre, float>{4518.548561};.
Compiling this generates no error nor warning on GCC with the warnings used by the CI.
If one changes the place where the double is explicitly casted to float from

constexpr quantity(const Value& v) : value_(static_cast<rep>(v)) {}

to

constexpr quantity(const Value& v) : value_(v) {}

then the following warning is emitted.

In file included from /home/johel/Documents/C++/Forks/mpusz/units/src/src/include/units/physical/si/base/time.h:28,
                 from /home/johel/Documents/C++/Forks/mpusz/units/src/src/include/units/chrono.h:26,
                 from quantity_kind_test.cpp:24:
/home/johel/Documents/C++/Forks/mpusz/units/src/src/include/units/quantity.h: In instantiation of ‘constexpr units::quantity<D, U, Rep>::quantity(const Value&) [with Value = double; D = units::physical::si::dim_length; U = units::physical::si::metre; Rep = float]’:
quantity_kind_test.cpp:249:56:   required from here
/home/johel/Documents/C++/Forks/mpusz/units/src/src/include/units/quantity.h:137:48: warning: conversion from ‘double’ to ‘float’ may change value [-Wfloat-conversion]
  137 |   constexpr quantity(const Value& v) : value_(v) {}
      |                                                ^

The user-facing warning is successfully restored. These silent lossy conversions happens at other places, and should be carefully audited.

[JohelEGP]

I remember range-v3 having a nice set of pragma macros to easily disable and reenable these warnings on the test cases that would trigger them.

[mpusz]

You are right. Are you going to provide a PR or should I do this?

BTW, we might need those macros as I already had issues with quantity_test.cpp after the last change we discussed. I am not feeling comfortable with disabling the working on a per-file basis.

[JohelEGP]

I'll look into those after I'm done writing the tests and documentation for #173.

[JohelEGP]

Seems like it's not possible to disable warnings in the library by adding a pragma before the line in user code that triggers it on GCC. I wonder if MSVC's pragma would actually disable it.

For GCC, it's possible to add the pragma in the library. But that's no different from the current state of silent lossy conversions.

[JohelEGP]

I can't find where it was, but you asked me something like how to better manage the compiler macros in hacks.h. I think Hedley can help here.

[JohelEGP]

#216 partially addresses this issue; there's room for improvement.

The other constructor uses quantity_cast. When both quantities have the same unit, only the conversion between types is needed. However, the current implementation seems general, and thus it isn't a single line change.

Anyways, I think a wrapper around the static_cast would be necessary when user-defined representation types are involved to ensure that it remains an explicit conversion. That's because the concepts required to be modeled by the representation types don't require an implicit conversion between them.

At the very least, when the units are the same and both representation types are arithmetic types, it could be left as an implicit conversion to restore user-facing warnings.

Whether to implement that or not, and to do that as a more constrained constructor or within quantity_cast, I'll leave it up to you. If done within quantity_cast, perhaps there'd be even more opportunities for similar changes I haven't noticed.

[mpusz]

@JohelEGP, do you have any ideas on how to improve here?