refactor(core): replace `wrapped_type_t` with `std::indirectly_readable_traits`

[JohelEGP]

Title: refactor(core): replace wrapped_type_t with std::indirectly_readable_traits.

Description:

See https://wg21.link/readable.traits and https://github.com/mpusz/mp-units/blob/89bafed9619a4a8320046e2be621172a6e88b586/src/core/include/mp-units/framework/customization_points.h#L52-L57

wrapped_type_t is defined at https://github.com/mpusz/mp-units/blob/89bafed9619a4a8320046e2be621172a6e88b586/src/core/include/mp-units/ext/type_traits.h#L103-L140

It seems to me that we could replace the treat_as_floating_point primary and specialization with

template<typename Rep>
constexpr bool treat_as_floating_point = std::is_floating_point_v<value_type_t<Rep>>;

and value_type with

template<typename T>
struct actual_value_type : cond-value-type<T> { };

template<typename T>
  requires(!std::is_pointer_v<T> && !std::is_array_v<T>) &&
          requires { typename std::indirectly_readable_traits<T>::value_type; }
struct actual_value_type<T> : std::indirectly_readable_traits<T> { };

to also:

• require object types,
• be const correct, and
• not prefer ::value_type over ::element_type.

[mpusz]

That's a good catch! As std::indirectly_readable_traits was introduced for iterators, I am unsure if that is a vanilla way of using them. But it does the job done 😉

[mpusz]

You are welcome to provide PR with such a change.

[JohelEGP]

I also think we should use std::chrono::treat_as_floating_point_v. I find it unlikely that a user that already specialized it wouldn't want the same behavior with our quantity types.

template<typename Rep>
constexpr bool treat_as_floating_point =
  std::chrono::treat_as_floating_point_v<typename value_type<Rep>::type>;

std::chrono::treat_as_floating_point doesn't have something like value_type. Instead, it uses Rep directly. Se we should also consider it that:

template<typename Rep>
constexpr bool treat_as_floating_point =
  std::chrono::treat_as_floating_point_v<Rep> ||
  std::chrono::treat_as_floating_point_v<typename value_type<Rep>::type>;

[JohelEGP]

The same logic applies to std::chrono::duration_values and mp_units::quantity_values. Although the former doesn't have a one member. So I think the default implementation should look like this:

template<typename Rep>
struct quantity_values : std::chrono::duration_values<Rep> {
  static constexpr Rep one() noexcept
    requires std::constructible_from<Rep, int>
  {
    return Rep(1);
  }
};

There is a difference in that duration_values' members are not specified to be SFINAE-friendly. It's a shame, but I don't consider it a big deal. Some quantity operators do require the rhs to not be zero().

We can go a step further and make the one conditional on duration_values<Rep> not providing it. That way, a user can just provide all the members in duration_values if they're already using that.

[chiphogg]

Alternatively, if we wanted to use our own trait, we could use the chrono trait as the default implementation. This might be useful if we found some undesirable behavior in the chrono approach, and we wanted to depart from it. But I think we'd be able to switch to that approach later, in any case.

[mpusz]

Regarding quantity_values, you also proposed #212 some time ago.

[JohelEGP]

Yeah, let's see what LEWG has to say, since the mentioned paper hasn't moved.

[mpusz]

quantity_values are not a part of the paper for now 😢

Also, the other changes that you proposed seem uncontroversial, so we can implement them now without waiting for LEWG.

[mpusz]

Resolved with the recent changes. Let's track #212 for breaking up the traits into pieces.