From bd06aeed8f1f8c6c7cb54ed8765cbcac5952a150 Mon Sep 17 00:00:00 2001 From: Chip Hogg Date: Fri, 21 Aug 2026 20:54:12 -0400 Subject: [PATCH 1/3] Support streaming and unit symbols for Eigen I was working on an Eigen example, and found that some basic stuff people would expect wasn't working yet. So I added tests and fixed it. On streaming: we've been using the "unary plus" for a long time in order to promote small integers that alias to `char`. The problem is that not every rep necessarily supports unary plus. Therefore, we guard it behind a utility called as `promote_for_streaming(x, 0)`, and SFINAE out the unary plus when it doesn't exist. On unit symbols: apparently, we had been constraining them to arithmetic types only. Well, we have a canonical trait for what's a valid rep: `IsValidRep`. Even if it's not fully fleshed out yet, it's the authoritative answer. So now we just use that consistently. --- au/compatibility/BUILD.bazel | 1 + au/compatibility/eigen_test.cc | 108 +++++++++++++++++++++++++++- au/io.hh | 20 ++++-- au/quantity.hh | 27 +++---- au/unit_symbol_test.cc | 39 ++++++++++ au/utility/test/type_traits_test.cc | 24 +++++++ au/utility/type_traits.hh | 18 +++++ au/wrapper_operations.hh | 10 +-- 8 files changed, 216 insertions(+), 31 deletions(-) diff --git a/au/compatibility/BUILD.bazel b/au/compatibility/BUILD.bazel index bc5c6033..6f654782 100644 --- a/au/compatibility/BUILD.bazel +++ b/au/compatibility/BUILD.bazel @@ -28,6 +28,7 @@ cc_test( deps = [ ":eigen", "//au", + "//au:io", "//au:testing", "@eigen", "@googletest//:gtest_main", diff --git a/au/compatibility/eigen_test.cc b/au/compatibility/eigen_test.cc index 85df14f1..cf695edb 100644 --- a/au/compatibility/eigen_test.cc +++ b/au/compatibility/eigen_test.cc @@ -17,14 +17,20 @@ #include #include #include +#include +#include #include "au/au.hh" +#include "au/io.hh" #include "au/testing.hh" #include "gtest/gtest.h" namespace au { -struct Meters : UnitImpl {}; +struct Meters : UnitImpl { + static constexpr const char label[] = "m"; +}; +constexpr const char Meters::label[]; constexpr auto meters = QuantityMaker{}; struct Feet : decltype(Meters{} * mag<381>() / mag<1250>()) {}; @@ -38,6 +44,7 @@ using ::testing::Eq; using ::testing::IsFalse; using ::testing::IsTrue; using ::testing::StaticAssertTypeEq; +using ::testing::StrEq; TEST(EigenCompatibility, CanCreateQuantityOfVector3d) { Eigen::Vector3d v(1.0, 2.0, 3.0); @@ -809,4 +816,103 @@ TEST(EigenFreeFunctions, CastAcceptsExpressionTemplateInput) { EXPECT_THAT(result.data_in(meters), SameTypeAndValue(Eigen::Vector3d(2.0, 4.0, 6.0))); } +template +std::string streamed(const T &x) { + std::ostringstream oss; + oss << x; + return oss.str(); +} + +TEST(EigenCompatibility, CanStreamVectorQuantity) { + const Eigen::Vector3d v{1.0, 2.0, 3.0}; + + EXPECT_THAT(streamed(meters(v)), StrEq(streamed(v) + " m")); +} + +TEST(EigenCompatibility, CanStreamExpressionRepQuantity) { + const Eigen::Vector3d v{1.0, 2.0, 3.0}; + + EXPECT_THAT(streamed(eval(transpose(meters(v)))), StrEq(streamed(v.transpose()) + " m")); +} + +// +// Unit symbols with Eigen reps. +// + +TEST(EigenUnitSymbols, SymbolOnRightMakesQuantityFromVector) { + constexpr auto m = symbol_for(meters); + + const auto q = Eigen::Vector3d{1.0, 2.0, 3.0} * m; + + StaticAssertTypeEq>(); + EXPECT_THAT(q.data_in(meters), Eq(Eigen::Vector3d(1.0, 2.0, 3.0))); +} + +TEST(EigenUnitSymbols, SymbolOnLeftMakesQuantityFromVector) { + constexpr auto m = symbol_for(meters); + + const auto q = m * Eigen::Vector3d{1.0, 2.0, 3.0}; + + StaticAssertTypeEq>(); + EXPECT_THAT(q.data_in(meters), Eq(Eigen::Vector3d(1.0, 2.0, 3.0))); +} + +TEST(EigenUnitSymbols, DividingVectorBySymbolMakesInverseUnit) { + constexpr auto s = symbol_for(secs); + + const auto q = Eigen::Vector3d{1.0, 2.0, 3.0} / s; + + StaticAssertTypeEq, Eigen::Vector3d>>(); + EXPECT_THAT(q.data_in(inverse(secs)), Eq(Eigen::Vector3d(1.0, 2.0, 3.0))); +} + +TEST(EigenUnitSymbols, ComposedSymbolsMakeCompoundUnit) { + constexpr auto m = symbol_for(meters); + constexpr auto s = symbol_for(secs); + + const auto v = Eigen::Vector3d{4.0, 5.0, 6.0} * m / s; + + StaticAssertTypeEq, Eigen::Vector3d>>(); + EXPECT_THAT(v.data_in(meters / sec), Eq(Eigen::Vector3d(4.0, 5.0, 6.0))); +} + +TEST(EigenUnitSymbols, MatrixRepWorksToo) { + constexpr auto m = symbol_for(meters); + + const auto q = Eigen::Matrix2d{{1.0, 2.0}, {3.0, 4.0}} * m; + + StaticAssertTypeEq>(); + EXPECT_THAT(q.data_in(meters), Eq(Eigen::Matrix2d({{1.0, 2.0}, {3.0, 4.0}}))); +} + +TEST(EigenUnitSymbols, IntegralScalarWorks) { + constexpr auto m = symbol_for(meters); + + const auto q = Eigen::Vector3i{1, 2, 3} * m; + + StaticAssertTypeEq>(); + EXPECT_THAT(q.data_in(meters), Eq(Eigen::Vector3i(1, 2, 3))); +} + +TEST(EigenUnitSymbols, ResultComposesWithTheRestOfTheLibrary) { + constexpr auto m = symbol_for(meters); + constexpr auto s = symbol_for(secs); + + const auto p0 = Eigen::Vector3d{1.0, 2.0, 3.0} * m; + const auto v = Eigen::Vector3d{4.0, 5.0, 6.0} * m / s; + const auto t = 2.0 * s; + + EXPECT_THAT(eval(p0 + v * t).data_in(meters), Eq(Eigen::Vector3d(9.0, 12.0, 15.0))); +} + +TEST(EigenUnitSymbols, ExpressionTemplateInputIsAcceptedAsRep) { + // An expression template names a `Scalar` too, so it qualifies -- with the usual lifetime risk. + constexpr auto m = symbol_for(meters); + const Eigen::Vector3d v{1.0, 2.0, 3.0}; + + const auto q = eval((v + v) * m); + + EXPECT_THAT(q.data_in(meters), Eq(Eigen::Vector3d(2.0, 4.0, 6.0))); +} + } // namespace au diff --git a/au/io.hh b/au/io.hh index 81091dfd..d335fc34 100644 --- a/au/io.hh +++ b/au/io.hh @@ -25,15 +25,23 @@ namespace au { +namespace detail { +// Unary `+` promotes a char-like rep (e.g. `int8_t`), so that `<<` prints a number rather than a +// character. Not every rep has one --- an Eigen vector does not --- so promote only where we can. +template +constexpr auto promote_for_streaming(const T &x, int) -> decltype(+x) { + return +x; +} +template +constexpr const T &promote_for_streaming(const T &x, ...) { + return x; +} +} // namespace detail + // Streaming output support for Quantity types. template std::ostream &operator<<(std::ostream &out, const Quantity &q) { - // In the case that the Rep is a type that resolves to 'char' (e.g. int8_t), - // the << operator will match the implementation that takes a character - // literal. Using the unary + operator will trigger an integer promotion on - // the operand, which will then match an appropriate << operator that will - // output the integer representation. - out << +q.in(U{}) << " " << unit_label(U{}); + out << detail::promote_for_streaming(q.in(U{}), 0) << " " << unit_label(U{}); return out; } diff --git a/au/quantity.hh b/au/quantity.hh index 95a255d8..3477d97f 100644 --- a/au/quantity.hh +++ b/au/quantity.hh @@ -792,19 +792,11 @@ AU_DEVICE_FUNC constexpr auto rep_cast(Zero z) { namespace detail { -// A SFINAE helper that is the identity, but only if we think a type is a valid rep. -// -// For now, we are restricting this to arithmetic types. This doesn't mean they're the only reps we -// support; it just means they're the only reps we can _construct via this method_. Later on, we -// would like to have a well-defined concept that defines what is and is not an acceptable rep for -// our `Quantity`. Once we have that, we can simply constrain on that concept. For more on this -// idea, see: https://github.com/aurora-opensource/au/issues/52 -struct NoTypeMember {}; -template -struct TypeIdentityIfLooksLikeValidRepImpl - : std::conditional_t::value, stdx::type_identity, NoTypeMember> {}; +// The identity on `T`, but only for a `T` we will accept as a rep, in `3.5 * m` and friends. +// `IsValidRep` excludes our own units, quantities, and other monovalue types, which is what keeps +// these overloads from competing with the ones meant for those. template -using TypeIdentityIfLooksLikeValidRep = typename TypeIdentityIfLooksLikeValidRepImpl::type; +using TypeIdentityIfValidRep = TypeIdentityIf<::au::IsValidRep, T>; // The unit whose `Constant` corresponds to a bare `Magnitude`: a scaled version of the unitless // unit. @@ -824,16 +816,14 @@ using UnitForMagnitude = ComputeScaledUnit, M>; // (N * M), for number N and magnitude M. template AU_DEVICE_FUNC constexpr auto operator*(T x, Magnitude) - -> Quantity>, - detail::TypeIdentityIfLooksLikeValidRep> { + -> Quantity>, detail::TypeIdentityIfValidRep> { return make_quantity>>(x); } // (M * N), for number N and magnitude M. template AU_DEVICE_FUNC constexpr auto operator*(Magnitude, T x) - -> Quantity>, - detail::TypeIdentityIfLooksLikeValidRep> { + -> Quantity>, detail::TypeIdentityIfValidRep> { return make_quantity>>(x); } @@ -841,15 +831,14 @@ AU_DEVICE_FUNC constexpr auto operator*(Magnitude, T x) template AU_DEVICE_FUNC constexpr auto operator/(T x, Magnitude) -> Quantity>>, - detail::TypeIdentityIfLooksLikeValidRep> { + detail::TypeIdentityIfValidRep> { return make_quantity>>>(x); } // (M / N), for number N and magnitude M. template AU_DEVICE_FUNC constexpr auto operator/(Magnitude, T x) - -> Quantity>, - detail::TypeIdentityIfLooksLikeValidRep> { + -> Quantity>, detail::TypeIdentityIfValidRep> { static_assert(!std::is_integral::value, "Dividing by an integer value disallowed: would almost always produce 0"); return make_quantity>>(T{1} / x); diff --git a/au/unit_symbol_test.cc b/au/unit_symbol_test.cc index a7574024..dce436be 100644 --- a/au/unit_symbol_test.cc +++ b/au/unit_symbol_test.cc @@ -14,19 +14,37 @@ #include "au/unit_symbol.hh" +#include #include +#include +#include "au/stdx/experimental/is_detected.hh" #include "au/testing.hh" #include "au/units/meters.hh" #include "au/units/seconds.hh" #include "gtest/gtest.h" +using ::testing::IsFalse; +using ::testing::IsTrue; using ::testing::StaticAssertTypeEq; namespace au { namespace { constexpr auto m = symbol_for(meters); constexpr auto s = symbol_for(seconds); + +// Detects which types a symbol will, and will not, make a quantity out of. +template +using TimesMeterSymbol = decltype(std::declval() * m); + +// A stand-in for an Eigen vector; `//au/compatibility:eigen_test` covers the real thing. +template +struct FakeEigenVector { + using Scalar = T; + T data[3]; +}; + +struct Empty {}; } // namespace TEST(SymbolFor, TakesUnitSlot) { @@ -51,6 +69,27 @@ TEST(SymbolFor, CanScaleByMagnitude) { EXPECT_THAT(3.5f / u100_m, SameTypeAndValue(inverse(meters * mag<100>())(3.5f))); } +TEST(SymbolFor, MakesQuantityFromAnyValidRep) { + EXPECT_THAT((stdx::experimental::is_detected>{}), + IsTrue()); + EXPECT_THAT((stdx::experimental::is_detected>{}), + IsTrue()); +} + +TEST(SymbolFor, RefusesTypesThatCannotBeReps) { + EXPECT_THAT((stdx::experimental::is_detected{}), IsFalse()); + + // A container of *quantities* can never be a valid rep: using it as one would nest units. + EXPECT_THAT( + (stdx::experimental::is_detected>>{}), + IsFalse()); +} + +TEST(SymbolFor, QuantityStillScalesRatherThanBecomingARep) { + // If a `Quantity` also looked like a valid rep, this would be ambiguous with `ScalesQuantity`. + EXPECT_THAT(seconds(3.0) * m, SameTypeAndValue((seconds * meters)(3.0))); +} + TEST(SymbolFor, CanApplyNamedPowerFunctions) { StaticAssertTypeEq(); } diff --git a/au/utility/test/type_traits_test.cc b/au/utility/test/type_traits_test.cc index c0c6418b..a2afc8ca 100644 --- a/au/utility/test/type_traits_test.cc +++ b/au/utility/test/type_traits_test.cc @@ -14,6 +14,9 @@ #include "au/utility/type_traits.hh" +#include + +#include "au/stdx/experimental/is_detected.hh" #include "gmock/gmock.h" #include "gtest/gtest.h" @@ -34,6 +37,27 @@ TEST(Prepend, PrependsToPack) { StaticAssertTypeEq, int>, Pack>(); } +// An overload constrained the way the library constrains its own: on the return type, so that a +// failing `Condition` removes it from the overload set instead of erroring. Declared, never +// defined; we only ever ask whether a call to it would compile. +template +auto only_for_integral(T) -> TypeIdentityIf; + +template +using CallOnlyForIntegral = decltype(only_for_integral(std::declval())); + +TEST(TypeIdentityIf, IsTheIdentityWhenTheConditionHolds) { + StaticAssertTypeEq, int>(); + StaticAssertTypeEq, double>(); +} + +TEST(TypeIdentityIf, HasNoTypeMemberWhenTheConditionFails) { + // The point of the trait: naming it is a substitution failure rather than a hard error, so an + // overload constrained on it simply drops out of the overload set. + EXPECT_THAT((stdx::experimental::is_detected{}), IsTrue()); + EXPECT_THAT((stdx::experimental::is_detected{}), IsFalse()); +} + TEST(SameTypeIgnoringCvref, IgnoresCvrefQualifiers) { EXPECT_THAT((SameTypeIgnoringCvref::value), IsTrue()); EXPECT_THAT((SameTypeIgnoringCvref::value), IsTrue()); diff --git a/au/utility/type_traits.hh b/au/utility/type_traits.hh index 27705b09..3f7e622c 100644 --- a/au/utility/type_traits.hh +++ b/au/utility/type_traits.hh @@ -52,6 +52,15 @@ constexpr bool same_type_ignoring_cvref(T, U) { template struct AlwaysFalse : std::false_type {}; +// +// `TypeIdentityIf` is `T` when `Condition` holds, and a substitution failure when +// it doesn't: a way to constrain an overload through its return type. +// +template +struct TypeIdentityIfImpl; +template