From 7445585db8dfe882c9260191e92193a45817052b Mon Sep 17 00:00:00 2001 From: Mateusz Pusz Date: Tue, 29 Oct 2024 09:18:32 +0100 Subject: [PATCH] fix: compound assignment operations on quantities now behave the same as on the underying representation types Resolves #137 --- .../include/mp-units/framework/quantity.h | 40 ++++++++++++------- test/static/chrono_test.cpp | 4 +- test/static/quantity_test.cpp | 29 +++++++++++--- 3 files changed, 50 insertions(+), 23 deletions(-) diff --git a/src/core/include/mp-units/framework/quantity.h b/src/core/include/mp-units/framework/quantity.h index 09357ddc..4892dcb6 100644 --- a/src/core/include/mp-units/framework/quantity.h +++ b/src/core/include/mp-units/framework/quantity.h @@ -377,40 +377,50 @@ public: } // compound assignment operators - template Q = std::remove_cvref_t> - requires requires(rep a, rep b) { + template Q = std::remove_cvref_t> + requires detail::QuantityConvertibleTo, quantity> && requires(rep a, Rep2 b) { { a += b } -> std::same_as; } - friend constexpr decltype(auto) operator+=(FwdQ&& lhs, const quantity& rhs) + friend constexpr decltype(auto) operator+=(FwdQ&& lhs, const quantity& rhs) { - lhs.numerical_value_is_an_implementation_detail_ += rhs.numerical_value_is_an_implementation_detail_; + if constexpr (equivalent(unit, get_unit(R2))) + lhs.numerical_value_is_an_implementation_detail_ += rhs.numerical_value_is_an_implementation_detail_; + else + lhs.numerical_value_is_an_implementation_detail_ += rhs.in(lhs.unit).numerical_value_is_an_implementation_detail_; return std::forward(lhs); } - template Q = std::remove_cvref_t> - requires requires(rep a, rep b) { + template Q = std::remove_cvref_t> + requires detail::QuantityConvertibleTo, quantity> && requires(rep a, Rep2 b) { { a -= b } -> std::same_as; } - friend constexpr decltype(auto) operator-=(FwdQ&& lhs, const quantity& rhs) + friend constexpr decltype(auto) operator-=(FwdQ&& lhs, const quantity& rhs) { - lhs.numerical_value_is_an_implementation_detail_ -= rhs.numerical_value_is_an_implementation_detail_; + if constexpr (equivalent(unit, get_unit(R2))) + lhs.numerical_value_is_an_implementation_detail_ -= rhs.numerical_value_is_an_implementation_detail_; + else + lhs.numerical_value_is_an_implementation_detail_ -= rhs.in(lhs.unit).numerical_value_is_an_implementation_detail_; return std::forward(lhs); } - template Q = std::remove_cvref_t> - requires(!treat_as_floating_point) && requires(rep a, rep b) { - { a %= b } -> std::same_as; - } - friend constexpr decltype(auto) operator%=(FwdQ&& lhs, const quantity& rhs) + template Q = std::remove_cvref_t> + requires detail::QuantityConvertibleTo, quantity> && (!treat_as_floating_point) && + requires(rep a, Rep2 b) { + { a %= b } -> std::same_as; + } + friend constexpr decltype(auto) operator%=(FwdQ&& lhs, const quantity& rhs) { MP_UNITS_EXPECTS_DEBUG(rhs != zero()); - lhs.numerical_value_is_an_implementation_detail_ %= rhs.numerical_value_is_an_implementation_detail_; + if constexpr (equivalent(unit, get_unit(R2))) + lhs.numerical_value_is_an_implementation_detail_ %= rhs.numerical_value_is_an_implementation_detail_; + else + lhs.numerical_value_is_an_implementation_detail_ %= rhs.in(lhs.unit).numerical_value_is_an_implementation_detail_; return std::forward(lhs); } template Q = std::remove_cvref_t> - requires(!Quantity) && requires(rep a, const Value b) { + requires(!Quantity) && requires(rep a, Value b) { { a *= b } -> std::same_as; } friend constexpr decltype(auto) operator*=(FwdQ&& lhs, const Value& v) diff --git a/test/static/chrono_test.cpp b/test/static/chrono_test.cpp index fb2a531d..409833bd 100644 --- a/test/static/chrono_test.cpp +++ b/test/static/chrono_test.cpp @@ -137,8 +137,8 @@ static_assert(std::chrono::nanoseconds(quantity{1 * ns}) == 1ns); static_assert(std::chrono::nanoseconds(quantity{1 * s}) == 1s); // operators -static_assert((1 * s += 1s) == 2 * s); -static_assert((2 * s -= 1s) == 1 * s); +static_assert((1 * s += quantity{1s}) == 2 * s); +static_assert((2 * s -= quantity{1s}) == 1 * s); static_assert(quantity{1s} + 1 * s == 2 * s); static_assert(quantity{1s} + 1 * min == 61 * s); static_assert(1 * s + quantity{1s} == 2 * s); diff --git a/test/static/quantity_test.cpp b/test/static/quantity_test.cpp index 74b91daf..7f44d5d5 100644 --- a/test/static/quantity_test.cpp +++ b/test/static/quantity_test.cpp @@ -456,7 +456,7 @@ static_assert((1 * m *= 2 * one).numerical_value_in(m) == 2); static_assert((2 * m /= 2 * one).numerical_value_in(m) == 1); static_assert((7 * m %= 2 * m).numerical_value_in(m) == 1); -// different types +// different representation types static_assert((2.5 * m += 3 * m).numerical_value_in(m) == 5.5); static_assert((123 * m += 1 * km).numerical_value_in(m) == 1123); static_assert((5.5 * m -= 3 * m).numerical_value_in(m) == 2.5); @@ -467,12 +467,26 @@ static_assert((2.5 * m *= 3 * one).numerical_value_in(m) == 7.5); static_assert((7.5 * m /= 3 * one).numerical_value_in(m) == 2.5); static_assert((3500 * m %= 1 * km).numerical_value_in(m) == 500); +// convertible quantity types +static_assert((isq::length(1 * m) += isq::height(1 * m)).numerical_value_in(m) == 2); +static_assert((isq::length(2 * m) -= isq::height(1 * m)).numerical_value_in(m) == 1); +static_assert((isq::length(7 * m) %= isq::height(2 * m)).numerical_value_in(m) == 1); + +// different representation types with truncation +// clang-format off +static_assert((3 * m += 2.5 * m).numerical_value_in(m) == []{ auto v = 3; v += 2.5; return v; }()); +static_assert((3 * m -= 1.5 * m).numerical_value_in(m) == []{ auto v = 3; v -= 1.5; return v; }()); +static_assert((2 * m *= 2.5).numerical_value_in(m) == []{ auto v = 2; v *= 2.5; return v; }()); +static_assert((10 * m /= 2.5).numerical_value_in(m) == []{ auto v = 10; v /= 2.5; return v; }()); +static_assert((2 * m *= 2.5 * one).numerical_value_in(m) == []{ auto v = 2; v *= 2.5; return v; }()); +static_assert((10 * m /= 2.5 * one).numerical_value_in(m) == []{ auto v = 10; v /= 2.5; return v; }()); +// clang-format on + // static_assert((std::uint8_t{255} * m %= 256 * m).numerical_value_in(m) == [] { // std::uint8_t ui(255); // return ui %= 256; // }()); // UB -// TODO: Fix -static_assert((std::uint8_t{255}* m %= 257 * m).numerical_value_in(m) != [] { +static_assert((std::uint8_t{255}* m %= 257 * m).numerical_value_in(m) == [] { std::uint8_t ui(255); return ui %= 257; }()); @@ -490,15 +504,18 @@ static_assert((22 * m /= 3.33 * one).numerical_value_in(m) == 6); template typename Q> concept invalid_compound_assignments = requires() { // truncating not allowed - requires !requires(Q l) { l += 2.5 * m; }; - requires !requires(Q l) { l -= 2.5 * m; }; requires !requires(Q l) { l += 2 * isq::length[m]; }; requires !requires(Q l) { l -= 2 * isq::length[m]; }; requires !requires(Q l) { l %= 2 * isq::length[m]; }; requires !requires(Q l) { l %= 2 * percent; }; requires !requires(Q l) { l %= 2. * percent; }; - // TODO: accept non-truncating argument + // compound assignment with a non-convertible quantity not allowed + requires !requires(Q l) { l += 2 * isq::length[m]; }; + requires !requires(Q l) { l -= 2 * isq::length[m]; }; + requires !requires(Q l) { l %= 2 * isq::length[m]; }; + + // dimensionless quantities with a unit different than `one` requires !requires(Q l) { l *= 1 * (km / m); }; requires !requires(Q l) { l /= 1 * (km / m); }; requires !requires(Q l) { l %= 1 * (km / m); };