Compute magnitudes in the "real part" of a type (#278)

A `Magnitude` is _defined_ to be a _real_ number.  But users can ask for
its value in, say, `std::complex<int>` (either explicitly or
implicitly).  This is a problem, because in computing that value, we
check for overflow, which involves less-than comparisons... but complex
numbers are not ordered, so there is no meaningful sense of `<`!

To fix this, we introduce the notion of `RealPart<T>`, a type trait that
gives a type related to `T` that does have a notion of comparability.
(Since `Magnitude` is defined to be a real number, calling it "real
part" is a good fit.)  `RealPart<T>` is just `T` in almost all cases,
but it becomes "the type of `T{}.real()`" whenever that exists.  This
lets us support both `std::complex` and complex number types from other
libraries.

The core of the fix was to make sure all of our `Magnitude` operations
are taking place in `RealPart<T>` rather than `T`.  This basically boils
down to the _call_ to `base_power_value`, and the _implementations_ for
`SafeCastingChecker`.  We also use the real type throughout the
`:apply_magnitude` targets, for two reasons.  First, there's an actual
bug in clang related to complex-complex division.  Second, it should be
faster at runtime to only divide by the real part.

This change also has knock-on effects for implicit conversion policy. It
turns out the old implementation of `CoreImplicitConversionPolicy` was
always silently assuming that the type was already a real number type.
Therefore, we rename it by adding an `AssumingReal` suffix on the end.
The new `CoreImplicitConversionPolicy` passes `RealPart<Rep>` along to
this, after first checking for a new pitfall to guard against: namely,
implicitly converting a complex type to a real type.

Fixes #228.

au/code/au/apply_magnitude.hh

@@ -103,7 +103,7 @@ struct ApplyMagnitudeImpl<Mag, ApplyAs::INTEGER_MULTIPLY, T, is_T_integral> {
     static_assert(is_T_integral == std::is_integral<T>::value,
                   "Mismatched instantiation (should never be done manually)");
 
-    constexpr T operator()(const T &x) { return x * get_value<T>(Mag{}); }
+    constexpr T operator()(const T &x) { return x * get_value<RealPart<T>>(Mag{}); }
 
     static constexpr bool would_overflow(const T &x) {
         constexpr auto mag_value_result = get_value_result<T>(Mag{});
@@ -122,7 +122,7 @@ struct ApplyMagnitudeImpl<Mag, ApplyAs::INTEGER_DIVIDE, T, is_T_integral> {
     static_assert(is_T_integral == std::is_integral<T>::value,
                   "Mismatched instantiation (should never be done manually)");
 
-    constexpr T operator()(const T &x) { return x / get_value<T>(MagInverseT<Mag>{}); }
+    constexpr T operator()(const T &x) { return x / get_value<RealPart<T>>(MagInverseT<Mag>{}); }
 
     static constexpr bool would_overflow(const T &) { return false; }
 
@@ -165,8 +165,8 @@ struct ApplyMagnitudeImpl<Mag, ApplyAs::RATIONAL_MULTIPLY, T, true> {
 
     constexpr T operator()(const T &x) {
         using P = PromotedType<T>;
-        return static_cast<T>(x * get_value<P>(numerator(Mag{})) /
-                              get_value<P>(denominator(Mag{})));
+        return static_cast<T>(x * get_value<RealPart<P>>(numerator(Mag{})) /
+                              get_value<RealPart<P>>(denominator(Mag{})));
     }
 
     static constexpr bool would_overflow(const T &x) {
@@ -188,7 +188,7 @@ struct ApplyMagnitudeImpl<Mag, ApplyAs::RATIONAL_MULTIPLY, T, false> {
     static_assert(!std::is_integral<T>::value,
                   "Mismatched instantiation (should never be done manually)");
 
-    constexpr T operator()(const T &x) { return x * get_value<T>(Mag{}); }
+    constexpr T operator()(const T &x) { return x * get_value<RealPart<T>>(Mag{}); }
 
     static constexpr bool would_overflow(const T &x) {
         constexpr auto mag_value_result = get_value_result<T>(Mag{});
@@ -209,7 +209,7 @@ struct ApplyMagnitudeImpl<Mag, ApplyAs::IRRATIONAL_MULTIPLY, T, is_T_integral> {
     static_assert(is_T_integral == std::is_integral<T>::value,
                   "Mismatched instantiation (should never be done manually)");
 
-    constexpr T operator()(const T &x) { return x * get_value<T>(Mag{}); }
+    constexpr T operator()(const T &x) { return x * get_value<RealPart<T>>(Mag{}); }
 
     static constexpr bool would_overflow(const T &x) {
         constexpr auto mag_value_result = get_value_result<T>(Mag{});

au/code/au/conversion_policy.hh

@@ -52,7 +52,7 @@ template <typename U1, typename U2>
 struct SameDimension : stdx::bool_constant<U1::dim_ == U2::dim_> {};
 
 template <typename Rep, typename ScaleFactor, typename SourceRep>
-struct CoreImplicitConversionPolicyImpl
+struct CoreImplicitConversionPolicyImplAssumingReal
     : stdx::disjunction<
           std::is_floating_point<Rep>,
           stdx::conjunction<std::is_integral<SourceRep>,
@@ -61,7 +61,24 @@ struct CoreImplicitConversionPolicyImpl
 
 // Always permit the identity scaling.
 template <typename Rep>
-struct CoreImplicitConversionPolicyImpl<Rep, Magnitude<>, Rep> : std::true_type {};
+struct CoreImplicitConversionPolicyImplAssumingReal<Rep, Magnitude<>, Rep> : std::true_type {};
+
+// `SettingPureRealFromMixedReal<A, B>` tests whether `A` is a pure real type, _and_ `B` is a type
+// that has a real _part_, but is not purely real (call it a "mixed-real" type).
+//
+// The point is to guard against situations where we're _implicitly_ converting a "mixed-real" type
+// (i.e., typically a complex number) to a pure real type.
+template <typename Rep, typename SourceRep>
+struct SettingPureRealFromMixedReal
+    : stdx::conjunction<stdx::negation<std::is_same<SourceRep, RealPart<SourceRep>>>,
+                        std::is_same<Rep, RealPart<Rep>>> {};
+
+template <typename Rep, typename ScaleFactor, typename SourceRep>
+struct CoreImplicitConversionPolicyImpl
+    : stdx::conjunction<stdx::negation<SettingPureRealFromMixedReal<Rep, SourceRep>>,
+                        CoreImplicitConversionPolicyImplAssumingReal<RealPart<Rep>,
+                                                                     ScaleFactor,
+                                                                     RealPart<SourceRep>>> {};
 
 template <typename Rep, typename ScaleFactor, typename SourceRep>
 using CoreImplicitConversionPolicy = CoreImplicitConversionPolicyImpl<Rep, ScaleFactor, SourceRep>;

au/code/au/conversion_policy_test.cc

@@ -14,6 +14,8 @@
 
 #include "au/conversion_policy.hh"
 
+#include <complex>
+
 #include "au/unit_of_measure.hh"
 #include "gtest/gtest.h"
 
@@ -106,6 +108,17 @@ TEST(ImplicitRepPermitted, FunctionalInterfaceWorksAsExpected) {
     EXPECT_TRUE(implicit_rep_permitted_from_source_to_target<float>(Grams{}, Kilograms{}));
 }
 
+TEST(ImplicitRepPermitted, HandlesComplexRep) {
+    // These test cases are the same as the ones in `FunctionalInterfaceWorksAsExpected`, except
+    // that we replace the target type `T` with `std::complex<T>`.
+    EXPECT_TRUE(
+        implicit_rep_permitted_from_source_to_target<std::complex<int>>(Kilograms{}, Grams{}));
+    EXPECT_FALSE(
+        implicit_rep_permitted_from_source_to_target<std::complex<int>>(Grams{}, Kilograms{}));
+    EXPECT_TRUE(
+        implicit_rep_permitted_from_source_to_target<std::complex<float>>(Grams{}, Kilograms{}));
+}
+
 TEST(ConstructionPolicy, PermitImplicitFromWideVarietyOfTypesForFloatingPointTargets) {
     using gigagrams_float_policy = ConstructionPolicy<Gigagrams, float>;
     EXPECT_TRUE((gigagrams_float_policy::PermitImplicitFrom<Grams, int>::value));
@@ -126,6 +139,19 @@ TEST(ConstructionPolicy, PermitsImplicitFromIntegralTypesIffTargetScaleDividesSo
     EXPECT_FALSE((grams_int_policy::PermitImplicitFrom<Milligrams, int>::value));
 }
 
+TEST(ConstructionPolicy, ComplexToRealPreventsImplicitConversion) {
+    // `complex<int>` -> `float`: forbid, although `int` -> `float` is allowed.
+    using gigagrams_float_policy = ConstructionPolicy<Gigagrams, float>;
+    ASSERT_TRUE((gigagrams_float_policy::PermitImplicitFrom<Grams, int>::value));
+    EXPECT_FALSE((gigagrams_float_policy::PermitImplicitFrom<Grams, std::complex<int>>::value));
+
+    // (`int` or `complex<int>`) -> `complex<float>`: both allowed.
+    using gigagrams_complex_float_policy = ConstructionPolicy<Gigagrams, std::complex<float>>;
+    EXPECT_TRUE((gigagrams_complex_float_policy::PermitImplicitFrom<Grams, int>::value));
+    EXPECT_TRUE(
+        (gigagrams_complex_float_policy::PermitImplicitFrom<Grams, std::complex<int>>::value));
+}
+
 TEST(ConstructionPolicy, ForbidsImplicitConstructionOfIntegralTypeFromFloatingPtType) {
     using grams_int_policy = ConstructionPolicy<Grams, int>;
     EXPECT_FALSE((grams_int_policy::PermitImplicitFrom<Grams, double>::value));

au/code/au/magnitude.hh

@@ -15,6 +15,7 @@
 #pragma once
 
 #include <limits>
+#include <utility>
 
 #include "au/packs.hh"
 #include "au/power_aliases.hh"
@@ -467,12 +468,26 @@ constexpr bool all(const bool (&values)[N]) {
     return true;
 }
 
+// `RealPart<T>` is `T` itself, unless that type has a `.real()` member.
+template <typename T>
+using TypeOfRealMember = decltype(std::declval<T>().real());
+template <typename T>
+// `RealPartImpl` is basically equivalent to the `detected_or<T, TypeOfRealMember, T>` part at the
+// end.  But we special-case `is_arithmetic` to get a fast short-circuit for the overwhelmingly most
+// common case.
+struct RealPartImpl : std::conditional<std::is_arithmetic<T>::value,
+                                       T,
+                                       stdx::experimental::detected_or_t<T, TypeOfRealMember, T>> {
+};
+template <typename T>
+using RealPart = typename RealPartImpl<T>::type;
+
 template <typename Target, typename Enable = void>
 struct SafeCastingChecker {
     template <typename T>
     constexpr bool operator()(T x) {
-        return stdx::cmp_less_equal(std::numeric_limits<Target>::lowest(), x) &&
-               stdx::cmp_greater_equal(std::numeric_limits<Target>::max(), x);
+        return stdx::cmp_less_equal(std::numeric_limits<RealPart<Target>>::lowest(), x) &&
+               stdx::cmp_greater_equal(std::numeric_limits<RealPart<Target>>::max(), x);
     }
 };
 
@@ -481,8 +496,8 @@ struct SafeCastingChecker<Target, std::enable_if_t<std::is_integral<Target>::val
     template <typename T>
     constexpr bool operator()(T x) {
         return std::is_integral<T>::value &&
-               stdx::cmp_less_equal(std::numeric_limits<Target>::lowest(), x) &&
-               stdx::cmp_greater_equal(std::numeric_limits<Target>::max(), x);
+               stdx::cmp_less_equal(std::numeric_limits<RealPart<Target>>::lowest(), x) &&
+               stdx::cmp_greater_equal(std::numeric_limits<RealPart<Target>>::max(), x);
     }
 };
 
@@ -501,8 +516,8 @@ constexpr MagRepresentationOrError<T> get_value_result(Magnitude<BPs...>) {
     }
 
     // Force the expression to be evaluated in a constexpr context.
-    constexpr auto widened_result =
-        product({base_power_value<T, ExpT<BPs>::num, static_cast<std::uintmax_t>(ExpT<BPs>::den)>(
+    constexpr auto widened_result = product(
+        {base_power_value<RealPart<T>, ExpT<BPs>::num, static_cast<std::uintmax_t>(ExpT<BPs>::den)>(
             BaseT<BPs>::value())...});
 
     if ((widened_result.outcome != MagRepresentationOutcome::OK) ||

au/code/au/quantity.hh

@@ -339,16 +339,21 @@ class Quantity {
         return *this;
     }
 
-    // Short-hand multiplication assignment.
     template <typename T>
-    constexpr Quantity &operator*=(T s) {
+    constexpr void perform_shorthand_checks() {
         static_assert(
-            std::is_arithmetic<T>::value,
-            "This overload is only for scalar multiplication-assignment with arithmetic types");
+            IsValidRep<T>::value,
+            "This overload is only for scalar mult/div-assignment with raw numeric types");
 
-        static_assert(
-            std::is_floating_point<Rep>::value || std::is_integral<T>::value,
-            "We don't support compound multiplication of integral types by floating point");
+        static_assert((!std::is_integral<detail::RealPart<Rep>>::value) ||
+                          std::is_integral<detail::RealPart<T>>::value,
+                      "We don't support compound mult/div of integral types by floating point");
+    }
+
+    // Short-hand multiplication assignment.
+    template <typename T>
+    constexpr Quantity &operator*=(T s) {
+        perform_shorthand_checks<T>();
 
         value_ *= s;
         return *this;
@@ -357,11 +362,7 @@ class Quantity {
     // Short-hand division assignment.
     template <typename T>
     constexpr Quantity &operator/=(T s) {
-        static_assert(std::is_arithmetic<T>::value,
-                      "This overload is only for scalar division-assignment with arithmetic types");
-
-        static_assert(std::is_floating_point<Rep>::value || std::is_integral<T>::value,
-                      "We don't support compound division of integral types by floating point");
+        perform_shorthand_checks<T>();
 
         value_ /= s;
         return *this;

au/code/au/quantity_test.cc

@@ -463,6 +463,40 @@ TEST(Quantity, SupportsDivisionOfRealQuantityIntoComplexCoefficient) {
     EXPECT_THAT(quotient.imag(), DoubleEq(4.0));
 }
 
+TEST(Quantity, SupportsConvertingUnitsForComplexQuantity) {
+    constexpr auto a = meters(std::complex<double>{-3.0, 4.0});
+    const auto b = a.as(centi(meters));
+    EXPECT_THAT(b, SameTypeAndValue(centi(meters)(std::complex<double>{-300.0, 400.0})));
+}
+
+TEST(Quantity, SupportsExplicitRepConversionToComplexRep) {
+    constexpr auto a = feet(15'000.0);
+    const auto b = a.as<std::complex<int>>(miles);
+    EXPECT_THAT(b, SameTypeAndValue(miles(std::complex<int>{2, 0})));
+}
+
+TEST(Quantity, ShorthandMultiplicationAssignmentWorksForComplexRepAndScalar) {
+    auto test = meters(std::complex<float>{1.5f, 0.5f});
+    test *= std::complex<float>{2.0f, 1.0f};
+    EXPECT_THAT(test, SameTypeAndValue(meters(std::complex<float>{2.5f, 2.5f})));
+}
+
+template <typename T>
+constexpr T double_by_shorthand(T x) {
+    return x *= 2.0;
+}
+
+TEST(Quantity, ShorthandMultiplicationSupportsConstexpr) {
+    constexpr auto x = double_by_shorthand(feet(3.0));
+    EXPECT_THAT(x, SameTypeAndValue(feet(6.0)));
+}
+
+TEST(Quantity, ShorthandDivisionAssignmentWorksForComplexRepAndScalar) {
+    auto test = meters(std::complex<float>{25.0f, 12.5f});
+    test /= std::complex<float>{3.0f, 4.0f};
+    EXPECT_THAT(test, SameTypeAndValue(meters(std::complex<float>{5.0f, -2.5f})));
+}
+
 TEST(Quantity, CanDivideArbitraryQuantities) {
     constexpr auto d = feet(6.);
     constexpr auto t = hours(3.);