Use <=> operator for base_uint, Issue, and Book: (#4411)

- Implement the `operator==` and the `operator<=>` (aka the spaceship
  operator) in `base_uint`, `Issue`, and `Book`. 
- C++20-compliant compilers automatically provide the remaining
  comparison operators (e.g. `operator<`, `operator<=`, ...).
- Remove the function compare() because it is no longer needed.
- Maintain the same semantics as the existing code.
- Add some unit tests to gain further confidence.
- Fix #2525.
This commit is contained in:
drlongle
2023-03-15 04:54:54 +01:00
committed by GitHub
parent f7b3ddd87b
commit 84cde3ce0b
7 changed files with 138 additions and 216 deletions

View File

@@ -146,19 +146,22 @@ getPageForToken(
return nullptr;
else
{
// This would be an ideal place for the spaceship operator...
int const relation = compare(id & nft::pageMask, cmp);
auto const relation{(id & nft::pageMask) <=> cmp};
if (relation == 0)
{
// If the passed in id belongs exactly on this (full) page
// this account simply cannot store the NFT.
return nullptr;
}
else if (relation > 0)
if (relation > 0)
{
// We need to leave the entire contents of this page in
// narr so carr stays empty. The new NFT will be
// inserted in carr. This keeps the NFTs that must be
// together all on their own page.
splitIter = narr.end();
}
// If neither of those conditions apply then put all of
// narr into carr and produce an empty narr where the new NFT
@@ -228,7 +231,7 @@ compareTokens(uint256 const& a, uint256 const& b)
// 96-bits are identical we still need a fully deterministic sort.
// So we sort on the low 96-bits first. If those are equal we sort on
// the whole thing.
if (auto const lowBitsCmp = compare(a & nft::pageMask, b & nft::pageMask);
if (auto const lowBitsCmp{(a & nft::pageMask) <=> (b & nft::pageMask)};
lowBitsCmp != 0)
return lowBitsCmp < 0;

View File

@@ -33,6 +33,7 @@
#include <ripple/beast/utility/Zero.h>
#include <boost/endian/conversion.hpp>
#include <boost/functional/hash.hpp>
#include <algorithm>
#include <array>
#include <cstring>
#include <functional>
@@ -549,103 +550,66 @@ using uint160 = base_uint<160>;
using uint256 = base_uint<256>;
template <std::size_t Bits, class Tag>
inline int
compare(base_uint<Bits, Tag> const& a, base_uint<Bits, Tag> const& b)
[[nodiscard]] inline constexpr std::strong_ordering
operator<=>(base_uint<Bits, Tag> const& lhs, base_uint<Bits, Tag> const& rhs)
{
auto ret = std::mismatch(a.cbegin(), a.cend(), b.cbegin());
// This comparison might seem wrong on a casual inspection because it
// compares data internally stored as std::uint32_t byte-by-byte. But
// note that the underlying data is stored in big endian, even if the
// plaform is little endian. This makes the comparison correct.
//
// FIXME: use std::lexicographical_compare_three_way once support is
// added to MacOS.
if (ret.first == a.cend())
return 0;
auto const ret = std::mismatch(lhs.cbegin(), lhs.cend(), rhs.cbegin());
// a > b
if (*ret.first > *ret.second)
return 1;
// a == b
if (ret.first == lhs.cend())
return std::strong_ordering::equivalent;
// a < b
return -1;
return (*ret.first > *ret.second) ? std::strong_ordering::greater
: std::strong_ordering::less;
}
template <std::size_t Bits, class Tag>
inline bool
operator<(base_uint<Bits, Tag> const& a, base_uint<Bits, Tag> const& b)
template <std::size_t Bits, typename Tag>
[[nodiscard]] inline constexpr bool
operator==(base_uint<Bits, Tag> const& lhs, base_uint<Bits, Tag> const& rhs)
{
return compare(a, b) < 0;
}
template <std::size_t Bits, class Tag>
inline bool
operator<=(base_uint<Bits, Tag> const& a, base_uint<Bits, Tag> const& b)
{
return compare(a, b) <= 0;
}
template <std::size_t Bits, class Tag>
inline bool
operator>(base_uint<Bits, Tag> const& a, base_uint<Bits, Tag> const& b)
{
return compare(a, b) > 0;
}
template <std::size_t Bits, class Tag>
inline bool
operator>=(base_uint<Bits, Tag> const& a, base_uint<Bits, Tag> const& b)
{
return compare(a, b) >= 0;
}
template <std::size_t Bits, class Tag>
inline bool
operator==(base_uint<Bits, Tag> const& a, base_uint<Bits, Tag> const& b)
{
return compare(a, b) == 0;
}
template <std::size_t Bits, class Tag>
inline bool
operator!=(base_uint<Bits, Tag> const& a, base_uint<Bits, Tag> const& b)
{
return compare(a, b) != 0;
return (lhs <=> rhs) == 0;
}
//------------------------------------------------------------------------------
template <std::size_t Bits, class Tag>
inline bool
inline constexpr bool
operator==(base_uint<Bits, Tag> const& a, std::uint64_t b)
{
return a == base_uint<Bits, Tag>(b);
}
template <std::size_t Bits, class Tag>
inline bool
operator!=(base_uint<Bits, Tag> const& a, std::uint64_t b)
{
return !(a == b);
}
//------------------------------------------------------------------------------
template <std::size_t Bits, class Tag>
inline const base_uint<Bits, Tag>
inline constexpr base_uint<Bits, Tag>
operator^(base_uint<Bits, Tag> const& a, base_uint<Bits, Tag> const& b)
{
return base_uint<Bits, Tag>(a) ^= b;
}
template <std::size_t Bits, class Tag>
inline const base_uint<Bits, Tag>
inline constexpr base_uint<Bits, Tag>
operator&(base_uint<Bits, Tag> const& a, base_uint<Bits, Tag> const& b)
{
return base_uint<Bits, Tag>(a) &= b;
}
template <std::size_t Bits, class Tag>
inline const base_uint<Bits, Tag>
inline constexpr base_uint<Bits, Tag>
operator|(base_uint<Bits, Tag> const& a, base_uint<Bits, Tag> const& b)
{
return base_uint<Bits, Tag>(a) |= b;
}
template <std::size_t Bits, class Tag>
inline const base_uint<Bits, Tag>
inline constexpr base_uint<Bits, Tag>
operator+(base_uint<Bits, Tag> const& a, base_uint<Bits, Tag> const& b)
{
return base_uint<Bits, Tag>(a) += b;

View File

@@ -65,28 +65,24 @@ hash_append(Hasher& h, Book const& b)
Book
reversed(Book const& book);
/** Ordered comparison. */
int
compare(Book const& lhs, Book const& rhs);
/** Equality comparison. */
/** @{ */
bool
operator==(Book const& lhs, Book const& rhs);
bool
operator!=(Book const& lhs, Book const& rhs);
[[nodiscard]] inline constexpr bool
operator==(Book const& lhs, Book const& rhs)
{
return (lhs.in == rhs.in) && (lhs.out == rhs.out);
}
/** @} */
/** Strict weak ordering. */
/** @{ */
bool
operator<(Book const& lhs, Book const& rhs);
bool
operator>(Book const& lhs, Book const& rhs);
bool
operator>=(Book const& lhs, Book const& rhs);
bool
operator<=(Book const& lhs, Book const& rhs);
[[nodiscard]] inline constexpr std::weak_ordering
operator<=>(Book const& lhs, Book const& rhs)
{
if (auto const c{lhs.in <=> rhs.in}; c != 0)
return c;
return lhs.out <=> rhs.out;
}
/** @} */
} // namespace ripple

View File

@@ -63,31 +63,29 @@ hash_append(Hasher& h, Issue const& r)
hash_append(h, r.currency, r.account);
}
/** Ordered comparison.
The assets are ordered first by currency and then by account,
if the currency is not XRP.
*/
int
compare(Issue const& lhs, Issue const& rhs);
/** Equality comparison. */
/** @{ */
bool
operator==(Issue const& lhs, Issue const& rhs);
bool
operator!=(Issue const& lhs, Issue const& rhs);
[[nodiscard]] inline constexpr bool
operator==(Issue const& lhs, Issue const& rhs)
{
return (lhs.currency == rhs.currency) &&
(isXRP(lhs.currency) || lhs.account == rhs.account);
}
/** @} */
/** Strict weak ordering. */
/** @{ */
bool
operator<(Issue const& lhs, Issue const& rhs);
bool
operator>(Issue const& lhs, Issue const& rhs);
bool
operator>=(Issue const& lhs, Issue const& rhs);
bool
operator<=(Issue const& lhs, Issue const& rhs);
[[nodiscard]] inline constexpr std::weak_ordering
operator<=>(Issue const& lhs, Issue const& rhs)
{
if (auto const c{lhs.currency <=> rhs.currency}; c != 0)
return c;
if (isXRP(lhs.currency))
return std::weak_ordering::equivalent;
return (lhs.account <=> rhs.account);
}
/** @} */
//------------------------------------------------------------------------------

View File

@@ -47,58 +47,4 @@ reversed(Book const& book)
return Book(book.out, book.in);
}
/** Ordered comparison. */
int
compare(Book const& lhs, Book const& rhs)
{
int const diff(compare(lhs.in, rhs.in));
if (diff != 0)
return diff;
return compare(lhs.out, rhs.out);
}
/** Equality comparison. */
/** @{ */
bool
operator==(Book const& lhs, Book const& rhs)
{
return (lhs.in == rhs.in) && (lhs.out == rhs.out);
}
bool
operator!=(Book const& lhs, Book const& rhs)
{
return (lhs.in != rhs.in) || (lhs.out != rhs.out);
}
/** @} */
/** Strict weak ordering. */
/** @{ */
bool
operator<(Book const& lhs, Book const& rhs)
{
int const diff(compare(lhs.in, rhs.in));
if (diff != 0)
return diff < 0;
return lhs.out < rhs.out;
}
bool
operator>(Book const& lhs, Book const& rhs)
{
return rhs < lhs;
}
bool
operator>=(Book const& lhs, Book const& rhs)
{
return !(lhs < rhs);
}
bool
operator<=(Book const& lhs, Book const& rhs)
{
return !(rhs < lhs);
}
} // namespace ripple

View File

@@ -43,60 +43,4 @@ operator<<(std::ostream& os, Issue const& x)
return os;
}
/** Ordered comparison.
The assets are ordered first by currency and then by account,
if the currency is not XRP.
*/
int
compare(Issue const& lhs, Issue const& rhs)
{
int diff = compare(lhs.currency, rhs.currency);
if (diff != 0)
return diff;
if (isXRP(lhs.currency))
return 0;
return compare(lhs.account, rhs.account);
}
/** Equality comparison. */
/** @{ */
bool
operator==(Issue const& lhs, Issue const& rhs)
{
return compare(lhs, rhs) == 0;
}
bool
operator!=(Issue const& lhs, Issue const& rhs)
{
return !(lhs == rhs);
}
/** @} */
/** Strict weak ordering. */
/** @{ */
bool
operator<(Issue const& lhs, Issue const& rhs)
{
return compare(lhs, rhs) < 0;
}
bool
operator>(Issue const& lhs, Issue const& rhs)
{
return rhs < lhs;
}
bool
operator>=(Issue const& lhs, Issue const& rhs)
{
return !(lhs < rhs);
}
bool
operator<=(Issue const& lhs, Issue const& rhs)
{
return !(rhs < lhs);
}
} // namespace ripple

View File

@@ -57,8 +57,76 @@ struct nonhash
struct base_uint_test : beast::unit_test::suite
{
using test96 = base_uint<96>;
static_assert(std::is_copy_constructible<test96>::value, "");
static_assert(std::is_copy_assignable<test96>::value, "");
static_assert(std::is_copy_constructible<test96>::value);
static_assert(std::is_copy_assignable<test96>::value);
void
testComparisons()
{
{
static constexpr std::
array<std::pair<std::string_view, std::string_view>, 6>
test_args{
{{"0000000000000000", "0000000000000001"},
{"0000000000000000", "ffffffffffffffff"},
{"1234567812345678", "2345678923456789"},
{"8000000000000000", "8000000000000001"},
{"aaaaaaaaaaaaaaa9", "aaaaaaaaaaaaaaaa"},
{"fffffffffffffffe", "ffffffffffffffff"}}};
for (auto const& arg : test_args)
{
ripple::base_uint<64> const u{arg.first}, v{arg.second};
BEAST_EXPECT(u < v);
BEAST_EXPECT(u <= v);
BEAST_EXPECT(u != v);
BEAST_EXPECT(!(u == v));
BEAST_EXPECT(!(u > v));
BEAST_EXPECT(!(u >= v));
BEAST_EXPECT(!(v < u));
BEAST_EXPECT(!(v <= u));
BEAST_EXPECT(v != u);
BEAST_EXPECT(!(v == u));
BEAST_EXPECT(v > u);
BEAST_EXPECT(v >= u);
BEAST_EXPECT(u == u);
BEAST_EXPECT(v == v);
}
}
{
static constexpr std::array<
std::pair<std::string_view, std::string_view>,
6>
test_args{{
{"000000000000000000000000", "000000000000000000000001"},
{"000000000000000000000000", "ffffffffffffffffffffffff"},
{"0123456789ab0123456789ab", "123456789abc123456789abc"},
{"555555555555555555555555", "55555555555a555555555555"},
{"aaaaaaaaaaaaaaa9aaaaaaaa", "aaaaaaaaaaaaaaaaaaaaaaaa"},
{"fffffffffffffffffffffffe", "ffffffffffffffffffffffff"},
}};
for (auto const& arg : test_args)
{
ripple::base_uint<96> const u{arg.first}, v{arg.second};
BEAST_EXPECT(u < v);
BEAST_EXPECT(u <= v);
BEAST_EXPECT(u != v);
BEAST_EXPECT(!(u == v));
BEAST_EXPECT(!(u > v));
BEAST_EXPECT(!(u >= v));
BEAST_EXPECT(!(v < u));
BEAST_EXPECT(!(v <= u));
BEAST_EXPECT(v != u);
BEAST_EXPECT(!(v == u));
BEAST_EXPECT(v > u);
BEAST_EXPECT(v >= u);
BEAST_EXPECT(u == u);
BEAST_EXPECT(v == v);
}
}
}
void
run() override
@@ -66,9 +134,12 @@ struct base_uint_test : beast::unit_test::suite
testcase("base_uint: general purpose tests");
static_assert(
!std::is_constructible<test96, std::complex<double>>::value, "");
!std::is_constructible<test96, std::complex<double>>::value);
static_assert(
!std::is_assignable<test96&, std::complex<double>>::value, "");
!std::is_assignable<test96&, std::complex<double>>::value);
testComparisons();
// used to verify set insertion (hashing required)
std::unordered_set<test96, hardened_hash<>> uset;
@@ -112,8 +183,8 @@ struct base_uint_test : beast::unit_test::suite
BEAST_EXPECT(d == --t);
}
BEAST_EXPECT(compare(u, v) < 0);
BEAST_EXPECT(compare(v, u) > 0);
BEAST_EXPECT(u < v);
BEAST_EXPECT(v > u);
v = u;
BEAST_EXPECT(v == u);