From 5000d814afeefd7eb7f250888b76d6723790f762 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Tue, 18 Oct 2022 12:54:53 -0400 Subject: fixed_point: Use variable templates and concepts where applicable Makes a few things a little less noisy and removes the need for SFINAE in quite a few functions. --- src/common/concepts.h | 8 ++++ src/common/fixed_point.h | 120 +++++++++++++++++++---------------------------- 2 files changed, 56 insertions(+), 72 deletions(-) diff --git a/src/common/concepts.h b/src/common/concepts.h index a97555f6a..e8ce30dfe 100644 --- a/src/common/concepts.h +++ b/src/common/concepts.h @@ -34,4 +34,12 @@ concept DerivedFrom = requires { template concept ConvertibleTo = std::is_convertible_v; +// No equivalents in the stdlib + +template +concept IsArithmetic = std::is_arithmetic_v; + +template +concept IsIntegral = std::is_integral_v; + } // namespace Common diff --git a/src/common/fixed_point.h b/src/common/fixed_point.h index 6eb6afe2f..b2e489841 100644 --- a/src/common/fixed_point.h +++ b/src/common/fixed_point.h @@ -12,6 +12,8 @@ #include #include +#include + namespace Common { template @@ -50,8 +52,8 @@ struct type_from_size<64> { static constexpr size_t size = 64; using value_type = int64_t; - using unsigned_type = std::make_unsigned::type; - using signed_type = std::make_signed::type; + using unsigned_type = std::make_unsigned_t; + using signed_type = std::make_signed_t; using next_size = type_from_size<128>; }; @@ -61,8 +63,8 @@ struct type_from_size<32> { static constexpr size_t size = 32; using value_type = int32_t; - using unsigned_type = std::make_unsigned::type; - using signed_type = std::make_signed::type; + using unsigned_type = std::make_unsigned_t; + using signed_type = std::make_signed_t; using next_size = type_from_size<64>; }; @@ -72,8 +74,8 @@ struct type_from_size<16> { static constexpr size_t size = 16; using value_type = int16_t; - using unsigned_type = std::make_unsigned::type; - using signed_type = std::make_signed::type; + using unsigned_type = std::make_unsigned_t; + using signed_type = std::make_signed_t; using next_size = type_from_size<32>; }; @@ -83,8 +85,8 @@ struct type_from_size<8> { static constexpr size_t size = 8; using value_type = int8_t; - using unsigned_type = std::make_unsigned::type; - using signed_type = std::make_signed::type; + using unsigned_type = std::make_unsigned_t; + using signed_type = std::make_signed_t; using next_size = type_from_size<16>; }; @@ -101,7 +103,7 @@ struct divide_by_zero : std::exception {}; template constexpr FixedPoint divide( FixedPoint numerator, FixedPoint denominator, FixedPoint& remainder, - typename std::enable_if::next_size::is_specialized>::type* = nullptr) { + std::enable_if_t::next_size::is_specialized>* = nullptr) { using next_type = typename FixedPoint::next_type; using base_type = typename FixedPoint::base_type; @@ -121,7 +123,7 @@ constexpr FixedPoint divide( template constexpr FixedPoint divide( FixedPoint numerator, FixedPoint denominator, FixedPoint& remainder, - typename std::enable_if::next_size::is_specialized>::type* = nullptr) { + std::enable_if_t::next_size::is_specialized>* = nullptr) { using unsigned_type = typename FixedPoint::unsigned_type; @@ -191,7 +193,7 @@ constexpr FixedPoint divide( template constexpr FixedPoint multiply( FixedPoint lhs, FixedPoint rhs, - typename std::enable_if::next_size::is_specialized>::type* = nullptr) { + std::enable_if_t::next_size::is_specialized>* = nullptr) { using next_type = typename FixedPoint::next_type; using base_type = typename FixedPoint::base_type; @@ -210,7 +212,7 @@ constexpr FixedPoint multiply( template constexpr FixedPoint multiply( FixedPoint lhs, FixedPoint rhs, - typename std::enable_if::next_size::is_specialized>::type* = nullptr) { + std::enable_if_t::next_size::is_specialized>* = nullptr) { using base_type = typename FixedPoint::base_type; @@ -270,10 +272,8 @@ public: // constructors FixedPoint(FixedPoint&&) = default; FixedPoint& operator=(const FixedPoint&) = default; - template - constexpr FixedPoint( - Number n, typename std::enable_if::value>::type* = nullptr) - : data_(static_cast(n * one)) {} + template + constexpr FixedPoint(Number n) : data_(static_cast(n * one)) {} public: // conversion template @@ -411,15 +411,13 @@ public: // binary math operators, effects underlying bit pattern since these return *this; } - template ::value>::type> + template constexpr FixedPoint& operator>>=(Integer n) { data_ >>= n; return *this; } - template ::value>::type> + template constexpr FixedPoint& operator<<=(Integer n) { data_ <<= n; return *this; @@ -490,10 +488,10 @@ public: // if we have the same fractional portion, but differing integer portions, we trivially upgrade the // smaller type template -constexpr typename std::conditional= I2, FixedPoint, FixedPoint>::type operator+( +constexpr std::conditional_t= I2, FixedPoint, FixedPoint> operator+( FixedPoint lhs, FixedPoint rhs) { - using T = typename std::conditional= I2, FixedPoint, FixedPoint>::type; + using T = std::conditional_t= I2, FixedPoint, FixedPoint>; const T l = T::from_base(lhs.to_raw()); const T r = T::from_base(rhs.to_raw()); @@ -501,10 +499,10 @@ constexpr typename std::conditional= I2, FixedPoint, FixedPoint -constexpr typename std::conditional= I2, FixedPoint, FixedPoint>::type operator-( +constexpr std::conditional_t= I2, FixedPoint, FixedPoint> operator-( FixedPoint lhs, FixedPoint rhs) { - using T = typename std::conditional= I2, FixedPoint, FixedPoint>::type; + using T = std::conditional_t= I2, FixedPoint, FixedPoint>; const T l = T::from_base(lhs.to_raw()); const T r = T::from_base(rhs.to_raw()); @@ -512,10 +510,10 @@ constexpr typename std::conditional= I2, FixedPoint, FixedPoint -constexpr typename std::conditional= I2, FixedPoint, FixedPoint>::type operator*( +constexpr std::conditional_t= I2, FixedPoint, FixedPoint> operator*( FixedPoint lhs, FixedPoint rhs) { - using T = typename std::conditional= I2, FixedPoint, FixedPoint>::type; + using T = std::conditional_t= I2, FixedPoint, FixedPoint>; const T l = T::from_base(lhs.to_raw()); const T r = T::from_base(rhs.to_raw()); @@ -523,10 +521,10 @@ constexpr typename std::conditional= I2, FixedPoint, FixedPoint -constexpr typename std::conditional= I2, FixedPoint, FixedPoint>::type operator/( +constexpr std::conditional_t= I2, FixedPoint, FixedPoint> operator/( FixedPoint lhs, FixedPoint rhs) { - using T = typename std::conditional= I2, FixedPoint, FixedPoint>::type; + using T = std::conditional_t= I2, FixedPoint, FixedPoint>; const T l = T::from_base(lhs.to_raw()); const T r = T::from_base(rhs.to_raw()); @@ -561,54 +559,46 @@ constexpr FixedPoint operator/(FixedPoint lhs, FixedPoint rhs) return lhs; } -template ::value>::type> +template constexpr FixedPoint operator+(FixedPoint lhs, Number rhs) { lhs += FixedPoint(rhs); return lhs; } -template ::value>::type> +template constexpr FixedPoint operator-(FixedPoint lhs, Number rhs) { lhs -= FixedPoint(rhs); return lhs; } -template ::value>::type> +template constexpr FixedPoint operator*(FixedPoint lhs, Number rhs) { lhs *= FixedPoint(rhs); return lhs; } -template ::value>::type> +template constexpr FixedPoint operator/(FixedPoint lhs, Number rhs) { lhs /= FixedPoint(rhs); return lhs; } -template ::value>::type> +template constexpr FixedPoint operator+(Number lhs, FixedPoint rhs) { FixedPoint tmp(lhs); tmp += rhs; return tmp; } -template ::value>::type> +template constexpr FixedPoint operator-(Number lhs, FixedPoint rhs) { FixedPoint tmp(lhs); tmp -= rhs; return tmp; } -template ::value>::type> +template constexpr FixedPoint operator*(Number lhs, FixedPoint rhs) { FixedPoint tmp(lhs); tmp *= rhs; return tmp; } -template ::value>::type> +template constexpr FixedPoint operator/(Number lhs, FixedPoint rhs) { FixedPoint tmp(lhs); tmp /= rhs; @@ -616,78 +606,64 @@ constexpr FixedPoint operator/(Number lhs, FixedPoint rhs) { } // shift operators -template ::value>::type> +template constexpr FixedPoint operator<<(FixedPoint lhs, Integer rhs) { lhs <<= rhs; return lhs; } -template ::value>::type> +template constexpr FixedPoint operator>>(FixedPoint lhs, Integer rhs) { lhs >>= rhs; return lhs; } // comparison operators -template ::value>::type> +template constexpr bool operator>(FixedPoint lhs, Number rhs) { return lhs > FixedPoint(rhs); } -template ::value>::type> +template constexpr bool operator<(FixedPoint lhs, Number rhs) { return lhs < FixedPoint(rhs); } -template ::value>::type> +template constexpr bool operator>=(FixedPoint lhs, Number rhs) { return lhs >= FixedPoint(rhs); } -template ::value>::type> +template constexpr bool operator<=(FixedPoint lhs, Number rhs) { return lhs <= FixedPoint(rhs); } -template ::value>::type> +template constexpr bool operator==(FixedPoint lhs, Number rhs) { return lhs == FixedPoint(rhs); } -template ::value>::type> +template constexpr bool operator!=(FixedPoint lhs, Number rhs) { return lhs != FixedPoint(rhs); } -template ::value>::type> +template constexpr bool operator>(Number lhs, FixedPoint rhs) { return FixedPoint(lhs) > rhs; } -template ::value>::type> +template constexpr bool operator<(Number lhs, FixedPoint rhs) { return FixedPoint(lhs) < rhs; } -template ::value>::type> +template constexpr bool operator>=(Number lhs, FixedPoint rhs) { return FixedPoint(lhs) >= rhs; } -template ::value>::type> +template constexpr bool operator<=(Number lhs, FixedPoint rhs) { return FixedPoint(lhs) <= rhs; } -template ::value>::type> +template constexpr bool operator==(Number lhs, FixedPoint rhs) { return FixedPoint(lhs) == rhs; } -template ::value>::type> +template constexpr bool operator!=(Number lhs, FixedPoint rhs) { return FixedPoint(lhs) != rhs; } -- cgit v1.2.3 From 9393f90ccfd3131a021a1c5f3e42f20a287a6560 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Tue, 18 Oct 2022 13:01:36 -0400 Subject: fixed_point: Use defaulted comparisons Collapses all of the comparison functions down to a single line. --- src/common/fixed_point.h | 24 +----------------------- 1 file changed, 1 insertion(+), 23 deletions(-) diff --git a/src/common/fixed_point.h b/src/common/fixed_point.h index b2e489841..7f00dd7b5 100644 --- a/src/common/fixed_point.h +++ b/src/common/fixed_point.h @@ -301,29 +301,7 @@ public: } public: // comparison operators - constexpr bool operator==(FixedPoint rhs) const { - return data_ == rhs.data_; - } - - constexpr bool operator!=(FixedPoint rhs) const { - return data_ != rhs.data_; - } - - constexpr bool operator<(FixedPoint rhs) const { - return data_ < rhs.data_; - } - - constexpr bool operator>(FixedPoint rhs) const { - return data_ > rhs.data_; - } - - constexpr bool operator<=(FixedPoint rhs) const { - return data_ <= rhs.data_; - } - - constexpr bool operator>=(FixedPoint rhs) const { - return data_ >= rhs.data_; - } + friend constexpr auto operator<=>(FixedPoint lhs, FixedPoint rhs) = default; public: // unary operators constexpr bool operator!() const { -- cgit v1.2.3 From 0101ef9fb115e59f1b9b7a28890104b115eb184d Mon Sep 17 00:00:00 2001 From: Lioncash Date: Tue, 18 Oct 2022 13:05:08 -0400 Subject: fixed_point: Make to_uint() non-const This calls round_up(), which is a non-const member function, so if a fixed-point instantiation ever calls to_uint(), it'll result in a compiler error. This allows the member function to work. While we're at it, we can actually mark to_long_floor() as const, since it's not modifying any member state. --- src/common/fixed_point.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/common/fixed_point.h b/src/common/fixed_point.h index 7f00dd7b5..116dfbb33 100644 --- a/src/common/fixed_point.h +++ b/src/common/fixed_point.h @@ -411,7 +411,7 @@ public: // conversion to basic types return static_cast((data_ & integer_mask) >> fractional_bits); } - constexpr unsigned int to_uint() const { + constexpr unsigned int to_uint() { round_up(); return static_cast((data_ & integer_mask) >> fractional_bits); } @@ -425,7 +425,7 @@ public: // conversion to basic types return static_cast((data_ & integer_mask) >> fractional_bits); } - constexpr int64_t to_long_floor() { + constexpr int64_t to_long_floor() const { return static_cast((data_ & integer_mask) >> fractional_bits); } -- cgit v1.2.3 From 2cc9d94060bbf4a6395ea1ebaa06ce65bb8fc07d Mon Sep 17 00:00:00 2001 From: Lioncash Date: Tue, 18 Oct 2022 13:10:33 -0400 Subject: fixed_point: Mark relevant member function [[nodiscard]] Marks member functions as discard, where ignoring the return value would be indicative of a bug or dead code. --- src/common/fixed_point.h | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/src/common/fixed_point.h b/src/common/fixed_point.h index 116dfbb33..cbe76fbf1 100644 --- a/src/common/fixed_point.h +++ b/src/common/fixed_point.h @@ -304,11 +304,11 @@ public: // comparison operators friend constexpr auto operator<=>(FixedPoint lhs, FixedPoint rhs) = default; public: // unary operators - constexpr bool operator!() const { + [[nodiscard]] constexpr bool operator!() const { return !data_; } - constexpr FixedPoint operator~() const { + [[nodiscard]] constexpr FixedPoint operator~() const { // NOTE(eteran): this will often appear to "just negate" the value // that is not an error, it is because -x == (~x+1) // and that "+1" is adding an infinitesimally small fraction to the @@ -316,11 +316,11 @@ public: // unary operators return FixedPoint::from_base(~data_); } - constexpr FixedPoint operator-() const { + [[nodiscard]] constexpr FixedPoint operator-() const { return FixedPoint::from_base(-data_); } - constexpr FixedPoint operator+() const { + [[nodiscard]] constexpr FixedPoint operator+() const { return FixedPoint::from_base(+data_); } @@ -406,42 +406,42 @@ public: // conversion to basic types data_ += (data_ & fractional_mask) >> 1; } - constexpr int to_int() { + [[nodiscard]] constexpr int to_int() { round_up(); return static_cast((data_ & integer_mask) >> fractional_bits); } - constexpr unsigned int to_uint() { + [[nodiscard]] constexpr unsigned int to_uint() { round_up(); return static_cast((data_ & integer_mask) >> fractional_bits); } - constexpr int64_t to_long() { + [[nodiscard]] constexpr int64_t to_long() { round_up(); return static_cast((data_ & integer_mask) >> fractional_bits); } - constexpr int to_int_floor() const { + [[nodiscard]] constexpr int to_int_floor() const { return static_cast((data_ & integer_mask) >> fractional_bits); } - constexpr int64_t to_long_floor() const { + [[nodiscard]] constexpr int64_t to_long_floor() const { return static_cast((data_ & integer_mask) >> fractional_bits); } - constexpr unsigned int to_uint_floor() const { + [[nodiscard]] constexpr unsigned int to_uint_floor() const { return static_cast((data_ & integer_mask) >> fractional_bits); } - constexpr float to_float() const { + [[nodiscard]] constexpr float to_float() const { return static_cast(data_) / FixedPoint::one; } - constexpr double to_double() const { + [[nodiscard]] constexpr double to_double() const { return static_cast(data_) / FixedPoint::one; } - constexpr base_type to_raw() const { + [[nodiscard]] constexpr base_type to_raw() const { return data_; } @@ -449,7 +449,7 @@ public: // conversion to basic types data_ &= fractional_mask; } - constexpr base_type get_frac() const { + [[nodiscard]] constexpr base_type get_frac() const { return data_ & fractional_mask; } -- cgit v1.2.3 From 0cfd90004bf058a278ff1db3442b523e2bb1b0fa Mon Sep 17 00:00:00 2001 From: Lioncash Date: Tue, 18 Oct 2022 13:13:26 -0400 Subject: fixed_point: Mark std::swap and move constructor as noexcept These shouldn't throw and can influence how some standard algorithms will work. --- src/common/fixed_point.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/common/fixed_point.h b/src/common/fixed_point.h index cbe76fbf1..ad0d71e50 100644 --- a/src/common/fixed_point.h +++ b/src/common/fixed_point.h @@ -269,7 +269,7 @@ public: public: // constructors FixedPoint() = default; FixedPoint(const FixedPoint&) = default; - FixedPoint(FixedPoint&&) = default; + FixedPoint(FixedPoint&&) noexcept = default; FixedPoint& operator=(const FixedPoint&) = default; template @@ -454,7 +454,7 @@ public: // conversion to basic types } public: - constexpr void swap(FixedPoint& rhs) { + constexpr void swap(FixedPoint& rhs) noexcept { using std::swap; swap(data_, rhs.data_); } -- cgit v1.2.3 From b6119a55f9bbc306593afccc0820165d74c2fadc Mon Sep 17 00:00:00 2001 From: Lioncash Date: Tue, 18 Oct 2022 15:33:47 -0400 Subject: fixed_point: Mark copy/move assignment operators and constructors as constexpr Given these are just moving a raw value around, these can sensibly be made constexpr to make the interface more useful. --- src/common/fixed_point.h | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/common/fixed_point.h b/src/common/fixed_point.h index ad0d71e50..5aafa273b 100644 --- a/src/common/fixed_point.h +++ b/src/common/fixed_point.h @@ -268,9 +268,12 @@ public: public: // constructors FixedPoint() = default; - FixedPoint(const FixedPoint&) = default; - FixedPoint(FixedPoint&&) noexcept = default; - FixedPoint& operator=(const FixedPoint&) = default; + + constexpr FixedPoint(const FixedPoint&) = default; + constexpr FixedPoint& operator=(const FixedPoint&) = default; + + constexpr FixedPoint(FixedPoint&&) noexcept = default; + constexpr FixedPoint& operator=(FixedPoint&&) noexcept = default; template constexpr FixedPoint(Number n) : data_(static_cast(n * one)) {} -- cgit v1.2.3 From 6e1c6297a3f8fcd243ec6b31d3832d84c7df474c Mon Sep 17 00:00:00 2001 From: Lioncash Date: Tue, 18 Oct 2022 15:37:37 -0400 Subject: fixed_point: Mark default constructor as constexpr Ensures that a fixed-point value is always initialized This likely also fixes several cases of uninitialized values being operated on, since we have multiple areas in the codebase where the default constructor is being used like: Common::FixedPoint<50, 14> current_sample{}; and is then followed up with an arithmetic operation like += or something else, which operates directly on FixedPoint's internal data member, which would previously be uninitialized. --- src/common/fixed_point.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/common/fixed_point.h b/src/common/fixed_point.h index 5aafa273b..f899b0d54 100644 --- a/src/common/fixed_point.h +++ b/src/common/fixed_point.h @@ -267,7 +267,7 @@ public: static constexpr base_type one = base_type(1) << fractional_bits; public: // constructors - FixedPoint() = default; + constexpr FixedPoint() = default; constexpr FixedPoint(const FixedPoint&) = default; constexpr FixedPoint& operator=(const FixedPoint&) = default; @@ -463,7 +463,7 @@ public: } public: - base_type data_; + base_type data_{}; }; // if we have the same fractional portion, but differing integer portions, we trivially upgrade the -- cgit v1.2.3