Improve error messages for bad maker calls (#294)

This covers situations where somebody calls a `QuantityMaker`, or
`QuantityPointMaker`, on a value that isn't a valid rep.  (One of the
most common use cases is when the value is _already_ a core Au type, a
`Quantity` or `QuantityMaker`, and the call is redundant.

The first approach is a blanket approach: we start enforcing the "valid
rep" definition that we had previously added.  This already makes the
error messages a lot nicer and shorter.

We then go further for the most common use case of redundant maker
calls.  By adding templated overloads to each maker for `Quantity` and
`QuantityPoint`, we can directly tell users what they did wrong.  In
order to implement these overloads, we needed an `AlwaysFalse` utliity,
which we added along with unit tests.

Helps #288.  We will follow up to close this out by updating the
troubleshooting guide.

---------

Co-authored-by: Geoffrey Viola <[email protected]>

au/BUILD.bazel

@@ -159,9 +159,9 @@ cc_library(
     hdrs = ["code/au/chrono_interop.hh"],
     includes = ["code"],
     deps = [
-        ":units",
         ":prefix",
         ":quantity",
+        ":units",
     ],
 )
 
@@ -417,6 +417,7 @@ cc_library(
         ":operators",
         ":rep",
         ":unit_of_measure",
+        ":utility",
         ":zero",
     ],
 )

au/code/au/CMakeLists.txt

@@ -117,6 +117,7 @@ header_only_library(
     operators
     rep
     unit_of_measure
+    utility
     zero
   GTEST_SRCS
     quantity_chrono_policy_correspondence_test.cc

au/code/au/quantity.hh

@@ -22,6 +22,7 @@
 #include "au/rep.hh"
 #include "au/stdx/functional.hh"
 #include "au/unit_of_measure.hh"
+#include "au/utility/type_traits.hh"
 #include "au/zero.hh"
 
 namespace au {
@@ -115,6 +116,8 @@ class Quantity {
     using Unit = UnitT;
     static constexpr auto unit = Unit{};
 
+    static_assert(IsValidRep<Rep>::value, "Rep must meet our requirements for a rep");
+
     // IMPLICIT constructor for another Quantity of the same Dimension.
     template <typename OtherUnit,
               typename OtherRep,
@@ -481,6 +484,11 @@ using QuantityI64 = Quantity<UnitT, int64_t>;
 template <typename UnitT>
 using QuantityU64 = Quantity<UnitT, uint64_t>;
 
+// Forward declare `QuantityPoint` here, so that we can give better error messages when users try to
+// make it into a quantity.
+template <typename U, typename R>
+class QuantityPoint;
+
 template <typename UnitT>
 struct QuantityMaker {
     using Unit = UnitT;
@@ -491,6 +499,18 @@ struct QuantityMaker {
         return {value};
     }
 
+    template <typename U, typename R>
+    constexpr void operator()(Quantity<U, R>) const {
+        constexpr bool is_not_already_a_quantity = detail::AlwaysFalse<U, R>::value;
+        static_assert(is_not_already_a_quantity, "Input to QuantityMaker is already a Quantity");
+    }
+
+    template <typename U, typename R>
+    constexpr void operator()(QuantityPoint<U, R>) const {
+        constexpr bool is_not_a_quantity_point = detail::AlwaysFalse<U, R>::value;
+        static_assert(is_not_a_quantity_point, "Input to QuantityMaker is a QuantityPoint");
+    }
+
     template <typename... BPs>
     constexpr auto operator*(Magnitude<BPs...> m) const {
         return QuantityMaker<decltype(unit * m)>{};

au/code/au/quantity_point.hh

@@ -287,6 +287,19 @@ struct QuantityPointMaker {
         return QuantityPoint<Unit, T>{make_quantity<Unit>(value)};
     }
 
+    template <typename U, typename R>
+    constexpr void operator()(Quantity<U, R>) const {
+        constexpr bool is_not_a_quantity = detail::AlwaysFalse<U, R>::value;
+        static_assert(is_not_a_quantity, "Input to QuantityPointMaker is a Quantity");
+    }
+
+    template <typename U, typename R>
+    constexpr void operator()(QuantityPoint<U, R>) const {
+        constexpr bool is_not_already_a_quantity_point = detail::AlwaysFalse<U, R>::value;
+        static_assert(is_not_already_a_quantity_point,
+                      "Input to QuantityPointMaker is already a QuantityPoint");
+    }
+
     template <typename... BPs>
     constexpr auto operator*(Magnitude<BPs...> m) const {
         return QuantityPointMaker<decltype(unit * m)>{};

au/code/au/utility/test/type_traits_test.cc

@@ -44,5 +44,12 @@ TEST(SameTypeIgnoringCvref, CanTakeInstances) {
     EXPECT_FALSE(same_type_ignoring_cvref(1.0, 2.0f));
 }
 
+TEST(AlwaysFalse, IsAlwaysFalse) {
+    EXPECT_FALSE(AlwaysFalse<int>::value);
+    EXPECT_FALSE(AlwaysFalse<void>::value);
+    EXPECT_FALSE(AlwaysFalse<>::value);
+    EXPECT_FALSE((AlwaysFalse<int, char, double>::value));
+}
+
 }  // namespace detail
 }  // namespace au

au/code/au/utility/type_traits.hh

@@ -34,6 +34,9 @@ constexpr bool same_type_ignoring_cvref(T, U) {
     return SameTypeIgnoringCvref<T, U>::value;
 }
 
+template <typename... Ts>
+struct AlwaysFalse : std::false_type {};
+
 ////////////////////////////////////////////////////////////////////////////////////////////////////
 // Implementation details below.