Skip to content

Commit

Permalink
Allow integer shifting by u64
Browse files Browse the repository at this point in the history
Rust shift operators take a rhs value of u32, which we emulated, however
this introduces a sharp edge in C++ where you shift by something like a
`sizeof()` expression. That evaluates to a `size_t` which used to
implicitly narrow to unsigned in order to shift. But with safer types
there is no implicit narrowing and you need to write

  u32::try_from(shift).unwrap()

Which is then fed to the shift operator which will check (again) that
the shift amount is in range.

Nowhere near the full range of values of `u32` is in range for a shift
operation, so the size of the type is somewhat arbitrary. So the value
inside must be checked regardless.

By allowing u64/usize types to be used on the rhs of the shift, we avoid
having to write conversions where they aren't holding weight.

> This makes the C++ operators accept things that the Rust operators do
> not. Why is C++ different here?

The answer here is that
1) casting in rust is much simpler, a `sizeof<T>() as u32` expression is
   simpler than the noise introduced by conversions in C++ like
   `sus::cast<u23>(sizeof(T))`
2) we need to rewrite a lot of code that already assumes it's fine to
   shift by a size_t
3) Rust type deduction allows Rust code to avoid naming the type of
   things at all and it can pick a u32, whereas C++ will need to pick a
   type and it will often have picked a size_t/usize.

> What mitigates this difference?

And the extra expressivity does not allow C++ to do different things or
change expectations wrt the behaviour of the shift operators.
  • Loading branch information
danakj committed Sep 27, 2023
1 parent e6e9821 commit 6611954
Show file tree
Hide file tree
Showing 15 changed files with 397 additions and 183 deletions.
40 changes: 0 additions & 40 deletions sus/num/__private/signed_integer_methods.inc
Original file line number Diff line number Diff line change
Expand Up @@ -887,46 +887,6 @@ template <class U>
requires((PrimitiveInteger<U> || PrimitiveEnum<U>) &&
!std::convertible_to<U, _self>)
friend constexpr inline _self operator^(U l, _self r) noexcept = delete;
/// Satisfies the [`Shl`]($sus::num::Shl) concept for signed integers.
///
/// This operation supports shifting with primitive signed or unsigned integers
/// that convert to the safe numeric, as well as enums.
/// However enum class is excluded as they require an explicit conversion to an
/// integer.
///
/// Thus the bound is `std::convertible_to` (implicit conversion) instead of
/// `sus::construct::From` (explicit conversion).
///
/// #[doc.overloads=signedint.<<]
[[nodiscard]] sus_pure friend constexpr inline _self operator<<(
_self l, std::convertible_to<u32> auto r) noexcept {
const auto out =
__private::shl_with_overflow(l.primitive_value, u32(r).primitive_value);
// TODO: Allow opting out of all overflow checks?
::sus::check(!out.overflow);
return out.value;
}
/// #[doc.overloads=signedint.<<]
template <class U>
requires(!std::convertible_to<U, u32>)
[[nodiscard]] sus_pure friend constexpr inline _self operator<<(
_self l, U r) noexcept = delete;
/// Satisfies the [`Shr`]($sus::num::Shr) concept for signed integers.
///
/// #[doc.overloads=signedint.>>]
[[nodiscard]] sus_pure friend constexpr inline _self operator>>(
_self l, std::convertible_to<u32> auto r) noexcept {
const auto out =
__private::shr_with_overflow(l.primitive_value, u32(r).primitive_value);
// TODO: Allow opting out of all overflow checks?
::sus::check(!out.overflow);
return out.value;
}
/// #[doc.overloads=signedint.>>]
template <class U>
requires(!std::convertible_to<U, u32>)
[[nodiscard]] sus_pure friend constexpr inline _self operator>>(
_self l, U r) noexcept = delete;

/// Satisfies the [`AddAssign<@doc.self>`]($sus::num::AddAssign) concept.
constexpr inline void operator+=(_self r) & noexcept {
Expand Down
112 changes: 41 additions & 71 deletions sus/num/__private/unsigned_integer_methods.inc
Original file line number Diff line number Diff line change
Expand Up @@ -580,33 +580,33 @@ sus_pure constexpr const _primitive* as_ptr() && noexcept = delete;

/// Satisfies the [`Eq`]($sus::cmp::Eq) concept for unsigned integers.
/// #[doc.overloads=uint.eq]
[[nodiscard]] sus_pure friend constexpr inline bool operator==(
[[nodiscard]] sus_pure_const friend constexpr inline bool operator==(
_self l, _self r) noexcept = default;
/// #[doc.overloads=uint.eq]
[[nodiscard]] sus_pure friend constexpr inline bool operator==(
[[nodiscard]] sus_pure_const friend constexpr inline bool operator==(
_self l, Unsigned auto r) noexcept {
return l.primitive_value == r.primitive_value;
}
/// #[doc.overloads=uint.eq]
[[nodiscard]] sus_pure friend constexpr inline bool operator==(
[[nodiscard]] sus_pure_const friend constexpr inline bool operator==(
_self l, UnsignedPrimitiveInteger auto r) noexcept {
return l.primitive_value == r;
}
/// Satisfies the [`StrongOrd`]($sus::cmp::StrongOrd) concept for unsigned
/// integers.
/// #[doc.overloads=uint.strongord]
[[nodiscard]] sus_pure friend constexpr inline std::strong_ordering operator<=>(
_self l, _self r) noexcept {
[[nodiscard]] sus_pure_const friend constexpr inline std::strong_ordering
operator<=>(_self l, _self r) noexcept {
return l.primitive_value <=> r.primitive_value;
}
/// #[doc.overloads=uint.strongord]
[[nodiscard]] sus_pure friend constexpr inline std::strong_ordering operator<=>(
_self l, Unsigned auto r) noexcept {
[[nodiscard]] sus_pure_const friend constexpr inline std::strong_ordering
operator<=>(_self l, Unsigned auto r) noexcept {
return l.primitive_value <=> r.primitive_value;
}
/// #[doc.overloads=uint.strongord]
[[nodiscard]] sus_pure friend constexpr inline std::strong_ordering operator<=>(
_self l, UnsignedPrimitiveInteger auto r) noexcept {
[[nodiscard]] sus_pure_const friend constexpr inline std::strong_ordering
operator<=>(_self l, UnsignedPrimitiveInteger auto r) noexcept {
return l.primitive_value <=> r;
}

Expand All @@ -625,7 +625,7 @@ sus_pure constexpr inline _self operator~() const& noexcept {
/// integer.
///
/// #[doc.overloads=unsignedint.+]
[[nodiscard]] sus_pure friend constexpr inline _self operator+(
[[nodiscard]] sus_pure_const friend constexpr inline _self operator+(
_self l, _self r) noexcept {
const auto out =
__private::add_with_overflow(l.primitive_value, r.primitive_value);
Expand All @@ -637,14 +637,14 @@ sus_pure constexpr inline _self operator~() const& noexcept {
/// #[doc.overloads=unsignedint.+]
template <Unsigned U>
requires(::sus::mem::size_of<U>() <= ::sus::mem::size_of<_primitive>())
[[nodiscard]] sus_pure friend constexpr inline _self operator+(_self l,
U r) noexcept {
[[nodiscard]] sus_pure_const friend constexpr inline _self operator+(
_self l, U r) noexcept {
return l + _primitive{r.primitive_value};
}
/// #[doc.overloads=unsignedint.+]
template <Unsigned U>
requires(::sus::mem::size_of<U>() <= ::sus::mem::size_of<_primitive>())
[[nodiscard]] sus_pure friend constexpr inline _self operator+(
[[nodiscard]] sus_pure_const friend constexpr inline _self operator+(
U l, _self r) noexcept {
return _primitive{l.primitive_value} + r;
}
Expand Down Expand Up @@ -687,7 +687,7 @@ friend constexpr inline _self operator+(U l, _self r) noexcept = delete;
/// integer.
///
/// #[doc.overloads=unsignedint.-]
[[nodiscard]] sus_pure friend constexpr inline _self operator-(
[[nodiscard]] sus_pure_const friend constexpr inline _self operator-(
_self l, _self r) noexcept {
const auto out =
__private::sub_with_overflow(l.primitive_value, r.primitive_value);
Expand All @@ -699,14 +699,14 @@ friend constexpr inline _self operator+(U l, _self r) noexcept = delete;
/// #[doc.overloads=unsignedint.-]
template <Unsigned U>
requires(::sus::mem::size_of<U>() <= ::sus::mem::size_of<_primitive>())
[[nodiscard]] sus_pure friend constexpr inline _self operator-(_self l,
U r) noexcept {
[[nodiscard]] sus_pure_const friend constexpr inline _self operator-(
_self l, U r) noexcept {
return l - _primitive{r.primitive_value};
}
/// #[doc.overloads=unsignedint.-]
template <Unsigned U>
requires(::sus::mem::size_of<U>() <= ::sus::mem::size_of<_primitive>())
[[nodiscard]] sus_pure friend constexpr inline _self operator-(
[[nodiscard]] sus_pure_const friend constexpr inline _self operator-(
U l, _self r) noexcept {
return _primitive{l.primitive_value} - r;
}
Expand Down Expand Up @@ -749,7 +749,7 @@ friend constexpr inline _self operator-(U l, _self r) noexcept = delete;
/// integer.
///
/// #[doc.overloads=unsignedint.*]
[[nodiscard]] sus_pure friend constexpr inline _self operator*(
[[nodiscard]] sus_pure_const friend constexpr inline _self operator*(
_self l, _self r) noexcept {
const auto out =
__private::mul_with_overflow(l.primitive_value, r.primitive_value);
Expand All @@ -761,14 +761,14 @@ friend constexpr inline _self operator-(U l, _self r) noexcept = delete;
/// #[doc.overloads=unsignedint.*]
template <Unsigned U>
requires(::sus::mem::size_of<U>() <= ::sus::mem::size_of<_primitive>())
[[nodiscard]] sus_pure friend constexpr inline _self operator*(_self l,
U r) noexcept {
[[nodiscard]] sus_pure_const friend constexpr inline _self operator*(
_self l, U r) noexcept {
return l * _primitive{r.primitive_value};
}
/// #[doc.overloads=unsignedint.*]
template <Unsigned U>
requires(::sus::mem::size_of<U>() <= ::sus::mem::size_of<_primitive>())
[[nodiscard]] sus_pure friend constexpr inline _self operator*(
[[nodiscard]] sus_pure_const friend constexpr inline _self operator*(
U l, _self r) noexcept {
return _primitive{l.primitive_value} * r;
}
Expand Down Expand Up @@ -811,7 +811,7 @@ friend constexpr inline _self operator*(U l, _self r) noexcept = delete;
/// integer.
///
/// #[doc.overloads=unsignedint./]
[[nodiscard]] sus_pure friend constexpr inline _self operator/(
[[nodiscard]] sus_pure_const friend constexpr inline _self operator/(
_self l, _self r) noexcept {
// TODO: Allow opting out of all overflow checks?
::sus::check(r.primitive_value != 0u);
Expand All @@ -821,14 +821,14 @@ friend constexpr inline _self operator*(U l, _self r) noexcept = delete;
/// #[doc.overloads=unsignedint./]
template <Unsigned U>
requires(::sus::mem::size_of<U>() <= ::sus::mem::size_of<_primitive>())
[[nodiscard]] sus_pure friend constexpr inline _self operator/(_self l,
U r) noexcept {
[[nodiscard]] sus_pure_const friend constexpr inline _self operator/(
_self l, U r) noexcept {
return l / _primitive{r.primitive_value};
}
/// #[doc.overloads=unsignedint./]
template <Unsigned U>
requires(::sus::mem::size_of<U>() <= ::sus::mem::size_of<_primitive>())
[[nodiscard]] sus_pure friend constexpr inline _self operator/(
[[nodiscard]] sus_pure_const friend constexpr inline _self operator/(
U l, _self r) noexcept {
return _primitive{l.primitive_value} / r;
}
Expand Down Expand Up @@ -871,7 +871,7 @@ friend constexpr inline _self operator/(U l, _self r) noexcept = delete;
/// integer.
///
/// #[doc.overloads=unsignedint.%]
[[nodiscard]] sus_pure friend constexpr inline _self operator%(
[[nodiscard]] sus_pure_const friend constexpr inline _self operator%(
_self l, _self r) noexcept {
// TODO: Allow opting out of all overflow checks?
::sus::check(r.primitive_value != 0u);
Expand All @@ -881,14 +881,14 @@ friend constexpr inline _self operator/(U l, _self r) noexcept = delete;
/// #[doc.overloads=unsignedint.%]
template <Unsigned U>
requires(::sus::mem::size_of<U>() <= ::sus::mem::size_of<_primitive>())
[[nodiscard]] sus_pure friend constexpr inline _self operator%(_self l,
U r) noexcept {
[[nodiscard]] sus_pure_const friend constexpr inline _self operator%(
_self l, U r) noexcept {
return l % _primitive{r.primitive_value};
}
/// #[doc.overloads=unsignedint.%]
template <Unsigned U>
requires(::sus::mem::size_of<U>() <= ::sus::mem::size_of<_primitive>())
[[nodiscard]] sus_pure friend constexpr inline _self operator%(
[[nodiscard]] sus_pure_const friend constexpr inline _self operator%(
U l, _self r) noexcept {
return _primitive{l.primitive_value} % r;
}
Expand Down Expand Up @@ -932,22 +932,22 @@ friend constexpr inline _self operator%(U l, _self r) noexcept = delete;
/// integer.
///
/// #[doc.overloads=unsignedint.&]
[[nodiscard]] sus_pure friend constexpr inline _self operator&(
[[nodiscard]] sus_pure_const friend constexpr inline _self operator&(
_self l, _self r) noexcept {
return _self(__private::unchecked_and(l.primitive_value, r.primitive_value));
}
#if _pointer
/// #[doc.overloads=unsignedint.&]
template <Unsigned U>
requires(::sus::mem::size_of<U>() <= ::sus::mem::size_of<_primitive>())
[[nodiscard]] sus_pure friend constexpr inline _self operator&(_self l,
U r) noexcept {
[[nodiscard]] sus_pure_const friend constexpr inline _self operator&(
_self l, U r) noexcept {
return l & _primitive{r.primitive_value};
}
/// #[doc.overloads=unsignedint.&]
template <Unsigned U>
requires(::sus::mem::size_of<U>() <= ::sus::mem::size_of<_primitive>())
[[nodiscard]] sus_pure friend constexpr inline _self operator&(
[[nodiscard]] sus_pure_const friend constexpr inline _self operator&(
U l, _self r) noexcept {
return _primitive{l.primitive_value} & r;
}
Expand Down Expand Up @@ -990,22 +990,22 @@ friend constexpr inline _self operator&(U l, _self r) noexcept = delete;
/// integer.
///
/// #[doc.overloads=unsignedint.|]
[[nodiscard]] sus_pure friend constexpr inline _self operator|(
[[nodiscard]] sus_pure_const friend constexpr inline _self operator|(
_self l, _self r) noexcept {
return _self(__private::unchecked_or(l.primitive_value, r.primitive_value));
}
#if _pointer
/// #[doc.overloads=unsignedint.|]
template <Unsigned U>
requires(::sus::mem::size_of<U>() <= ::sus::mem::size_of<_primitive>())
[[nodiscard]] sus_pure friend constexpr inline _self operator|(_self l,
U r) noexcept {
[[nodiscard]] sus_pure_const friend constexpr inline _self operator|(
_self l, U r) noexcept {
return l | _primitive{r.primitive_value};
}
/// #[doc.overloads=unsignedint.|]
template <Unsigned U>
requires(::sus::mem::size_of<U>() <= ::sus::mem::size_of<_primitive>())
[[nodiscard]] sus_pure friend constexpr inline _self operator|(
[[nodiscard]] sus_pure_const friend constexpr inline _self operator|(
U l, _self r) noexcept {
return _primitive{l.primitive_value} | r;
}
Expand Down Expand Up @@ -1048,22 +1048,22 @@ friend constexpr inline _self operator|(U l, _self r) noexcept = delete;
/// integer.
///
/// #[doc.overloads=unsignedint.^]
[[nodiscard]] sus_pure friend constexpr inline _self operator^(
[[nodiscard]] sus_pure_const friend constexpr inline _self operator^(
_self l, _self r) noexcept {
return _self(__private::unchecked_xor(l.primitive_value, r.primitive_value));
}
#if _pointer
/// #[doc.overloads=unsignedint.^]
template <Unsigned U>
requires(::sus::mem::size_of<U>() <= ::sus::mem::size_of<_primitive>())
[[nodiscard]] sus_pure friend constexpr inline _self operator^(_self l,
U r) noexcept {
[[nodiscard]] sus_pure_const friend constexpr inline _self operator^(
_self l, U r) noexcept {
return l ^ _primitive { r.primitive_value };
}
/// #[doc.overloads=unsignedint.^]
template <Unsigned U>
requires(::sus::mem::size_of<U>() <= ::sus::mem::size_of<_primitive>())
[[nodiscard]] sus_pure friend constexpr inline _self operator^(
[[nodiscard]] sus_pure_const friend constexpr inline _self operator^(
U l, _self r) noexcept {
return _primitive{l.primitive_value} ^ r;
}
Expand Down Expand Up @@ -1098,36 +1098,6 @@ template <class U>
!((UnsignedPrimitiveInteger<U> || UnsignedPrimitiveEnum<U>) && //
::sus::mem::size_of<U>() <= ::sus::mem::size_of<_primitive>()))
friend constexpr inline _self operator^(U l, _self r) noexcept = delete;
/// Satisfies the [`Shl`]($sus::num::Shl) concept for unsigned integers.
///
/// #[doc.overloads=unsignedint.<<]
[[nodiscard]] sus_pure friend constexpr inline _self operator<<(
_self l, std::convertible_to<u32> auto r) noexcept {
// TODO: Allow opting out of all overflow checks?
::sus::check(r < u32(__private::num_bits<_primitive>()));
return _self(
__private::unchecked_shl(l.primitive_value, u32(r).primitive_value));
}
/// #[doc.overloads=unsignedint.<<]
template <class U>
requires(!std::convertible_to<U, u32>)
[[nodiscard]] sus_pure friend constexpr inline _self operator<<(
_self l, U r) noexcept = delete;
/// Satisfies the [`Shr`]($sus::num::Shr) concept for unsigned integers.
///
/// #[doc.overloads=unsignedint.>>]
[[nodiscard]] sus_pure friend constexpr inline _self operator>>(
_self l, std::convertible_to<u32> auto r) noexcept {
// TODO: Allow opting out of all overflow checks?
::sus::check(r < u32(__private::num_bits<_primitive>()));
return _self(
__private::unchecked_shr(l.primitive_value, u32(r).primitive_value));
}
/// #[doc.overloads=unsignedint.<<]
template <class U>
requires(!std::convertible_to<U, u32>)
[[nodiscard]] sus_pure friend constexpr inline _self operator>>(
_self l, U r) noexcept = delete;

/// Satisfies the [`AddAssign<@doc.self>`]($sus::num::AddAssign) concept.
constexpr inline void operator+=(_self r) & noexcept {
Expand Down
4 changes: 2 additions & 2 deletions sus/num/i16_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -435,11 +435,11 @@ TEST(i16, OperatorsWithPrimitives) {
static_assert(CanShift<i16, uint8_t>);
static_assert(CanShift<i16, uint16_t>);
static_assert(CanShift<i16, uint32_t>);
static_assert(CantShift<i16, uint64_t>);
static_assert(CanShift<i16, uint64_t>);
static_assert(CanShift<i16, ENUM(, uint8_t)>);
static_assert(CanShift<i16, ENUM(, uint16_t)>);
static_assert(CanShift<i16, ENUM(, uint32_t)>);
static_assert(CantShift<i16, ENUM(, uint64_t)>);
static_assert(CanShift<i16, ENUM(, uint64_t)>);
static_assert(CantShift<i16, ENUM(class, uint8_t)>);
static_assert(CantShift<i16, ENUM(class, uint16_t)>);
static_assert(CantShift<i16, ENUM(class, uint32_t)>);
Expand Down
8 changes: 4 additions & 4 deletions sus/num/i32_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@
#include <type_traits>

#include "googletest/include/gtest/gtest.h"
#include "sus/cmp/eq.h"
#include "sus/cmp/ord.h"
#include "sus/collections/array.h"
#include "sus/construct/default.h"
#include "sus/construct/into.h"
Expand All @@ -24,8 +26,6 @@
#include "sus/num/num_concepts.h"
#include "sus/num/signed_integer.h"
#include "sus/num/unsigned_integer.h"
#include "sus/cmp/eq.h"
#include "sus/cmp/ord.h"
#include "sus/option/option.h"
#include "sus/prelude.h"
#include "sus/test/ensure_use.h"
Expand Down Expand Up @@ -449,11 +449,11 @@ TEST(i32, OperatorsWithPrimitives) {
static_assert(CanShift<i32, uint8_t>);
static_assert(CanShift<i32, uint16_t>);
static_assert(CanShift<i32, uint32_t>);
static_assert(CantShift<i32, uint64_t>);
static_assert(CanShift<i32, uint64_t>);
static_assert(CanShift<i32, ENUM(, uint8_t)>);
static_assert(CanShift<i32, ENUM(, uint16_t)>);
static_assert(CanShift<i32, ENUM(, uint32_t)>);
static_assert(CantShift<i32, ENUM(, uint64_t)>);
static_assert(CanShift<i32, ENUM(, uint64_t)>);
static_assert(CantShift<i32, ENUM(class, uint8_t)>);
static_assert(CantShift<i32, ENUM(class, uint16_t)>);
static_assert(CantShift<i32, ENUM(class, uint32_t)>);
Expand Down
Loading

0 comments on commit 6611954

Please sign in to comment.