Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 1 commit into from
Mar 15, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 7 additions & 4 deletions src/ripple/app/tx/impl/details/NFTokenUtils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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;

Expand Down
86 changes: 25 additions & 61 deletions src/ripple/basics/base_uint.h
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down Expand Up @@ -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
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I really like the [[nodiscard]] and constexpr. Nice!

operator<=>(base_uint<Bits, Tag> const& lhs, base_uint<Bits, Tag> const& rhs)
{
auto ret = std::mismatch(a.cbegin(), a.cend(), b.cbegin());

if (ret.first == a.cend())
return 0;
// 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.
Comment on lines +556 to +562
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice comment!


// a > b
if (*ret.first > *ret.second)
return 1;
auto const ret = std::mismatch(lhs.cbegin(), lhs.cend(), rhs.cbegin());

// a < b
return -1;
}

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;
}
// a == b
if (ret.first == lhs.cend())
return std::strong_ordering::equivalent;

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 (*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;
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>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like all the constexpr. Thanks!

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;
Expand Down
28 changes: 12 additions & 16 deletions src/ripple/protocol/Book.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
36 changes: 17 additions & 19 deletions src/ripple/protocol/Issue.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
/** @} */

//------------------------------------------------------------------------------
Expand Down
54 changes: 0 additions & 54 deletions src/ripple/protocol/impl/Book.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Look at all this removed code. Nice work!

} // namespace ripple
56 changes: 0 additions & 56 deletions src/ripple/protocol/impl/Issue.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Loading