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

mp-units/src/core/include/mp-units/framework/customization_points.h

Lines 52 to 57 in 89bafed

	template<typename Rep>
	constexpr bool treat_as_floating_point = std::is_floating_point_v<Rep>;
	template<typename Rep>
	requires requires { typename wrapped_type_t<Rep>; }
	constexpr bool treat_as_floating_point<Rep> = treat_as_floating_point<wrapped_type_t<Rep>>;

wrapped_type_t is defined at

mp-units/src/core/include/mp-units/ext/type_traits.h

Lines 103 to 140 in 89bafed

	namespace detail {
	template<typename T>
	struct get_value_type {
	using type = T::value_type;
	};
	template<typename T>
	struct get_element_type {
	using type = std::remove_reference_t<typename T::element_type>;
	};
	} // namespace detail
	template<typename T>
	requires requires { typename T::value_type; } || requires { typename T::element_type; }
	struct wrapped_type {
	using type =
	conditional<requires { typename T::value_type; }, detail::get_value_type<T>, detail::get_element_type<T>>::type;
	};
	template<typename T>
	requires requires { typename T::value_type; } || requires { typename T::element_type; }
	using wrapped_type_t = wrapped_type<T>::type;
	template<typename T>
	struct value_type {
	using type = T;
	};
	template<typename T>
	requires requires { typename wrapped_type_t<T>; }
	struct value_type<T> {
	using type = wrapped_type_t<T>;
	};
	template<typename T>
	using value_type_t = value_type<T>::type;

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.