[QUEUED scylla next] Merge 'Convert all remaining int tri-compares to std::strong_ordering' from Avi Kivity

0 views
Skip to first unread message

Commit Bot

unread,
Jul 29, 2021, 3:44:36 PMJul 29
to scylla...@googlegroups.com, Tomasz Grabiec
From: Tomasz Grabiec <tgra...@scylladb.com>
Committer: Tomasz Grabiec <tgra...@scylladb.com>
Branch: next

Merge 'Convert all remaining int tri-compares to std::strong_ordering' from Avi Kivity

Convert all known tri-compares that return an int to return std::strong_ordering.
Returning an int is dangerous since the caller can treat it as a bool, and indeed
this series uncovered a minor bug (#9103).

Test: unit (dev)

Fixes #1449

Closes #9106

* github.com:scylladb/scylla:
treewide: remove redundant "x <=> 0" compares
test: mutation_test: convert internal tri-compare to std::strong_ordering
utils: int_range: change to std::strong_ordering
test: change some internal comparators to std::strong_ordering
utils: big_decimal: change to std::strong_ordering
utils: fragment_range: change to std::strong_ordering
atomic_cell: change compare_atomic_cell_for_merge() to std::strong_ordering
types: drop scaffolding erected around lexicographical_tri_compare
sstables: keys: change to std::strong_ordering internally
bytes: compare_unsigned(): change to std::strong_ordering
uuid: change comparators to std::strong_ordering
types: convert abstract_type::compare and related to std::strong_ordering
types: reduce boilerplate when comparing empty value
serialized_tri_compare: change to std::strong_ordering
compound_compat: change to std::strong-ordering
types: change lexicographical_tri_compare, prefix_equality_tri_compare to std::strong_ordering

---
diff --git a/atomic_cell.hh b/atomic_cell.hh
--- a/atomic_cell.hh
+++ b/atomic_cell.hh
@@ -396,7 +396,7 @@ public:

class column_definition;

-int compare_atomic_cell_for_merge(atomic_cell_view left, atomic_cell_view right);
+std::strong_ordering compare_atomic_cell_for_merge(atomic_cell_view left, atomic_cell_view right);
void merge_column(const abstract_type& def,
atomic_cell_or_collection& old,
const atomic_cell_or_collection& neww);
diff --git a/bytes.hh b/bytes.hh
--- a/bytes.hh
+++ b/bytes.hh
@@ -27,6 +27,7 @@
#include <optional>
#include <iosfwd>
#include <functional>
+#include <compare>
#include "utils/mutable_view.hh"
#include <xxhash.h>

@@ -112,13 +113,13 @@ struct hash<bytes_view> {
};
} // namespace std

-inline int32_t compare_unsigned(bytes_view v1, bytes_view v2) {
+inline std::strong_ordering compare_unsigned(bytes_view v1, bytes_view v2) {
auto size = std::min(v1.size(), v2.size());
if (size) {
auto n = memcmp(v1.begin(), v2.begin(), size);
if (n) {
- return n;
+ return n <=> 0;
}
}
- return (int32_t) (v1.size() - v2.size());
+ return v1.size() <=> v2.size();
}
diff --git a/cdc/log.cc b/cdc/log.cc
--- a/cdc/log.cc
+++ b/cdc/log.cc
@@ -712,17 +712,17 @@ class maybe_back_insert_iterator : public std::back_insert_iterator<Container> {
}
return false;
}
- int32_t compare(const T&, const value_type& v);
+ std::strong_ordering compare(const T&, const value_type& v);
};

template<>
-int32_t maybe_back_insert_iterator<std::vector<std::pair<managed_bytes_view, managed_bytes_view>>, managed_bytes_view>::compare(
+std::strong_ordering maybe_back_insert_iterator<std::vector<std::pair<managed_bytes_view, managed_bytes_view>>, managed_bytes_view>::compare(
const managed_bytes_view& t, const value_type& v) {
return _type.compare(t, v.first);
}

template<>
-int32_t maybe_back_insert_iterator<std::vector<managed_bytes_view>, managed_bytes_view>::compare(const managed_bytes_view& t, const value_type& v) {
+std::strong_ordering maybe_back_insert_iterator<std::vector<managed_bytes_view>, managed_bytes_view>::compare(const managed_bytes_view& t, const value_type& v) {
return _type.compare(t, v);
}

diff --git a/clustering_bounds_comparator.hh b/clustering_bounds_comparator.hh
--- a/clustering_bounds_comparator.hh
+++ b/clustering_bounds_comparator.hh
@@ -64,29 +64,29 @@ public:
std::reference_wrapper<const schema> _s;
tri_compare(const schema& s) : _s(s)
{ }
- int operator()(const clustering_key_prefix& p1, int32_t w1, const clustering_key_prefix& p2, int32_t w2) const {
+ std::strong_ordering operator()(const clustering_key_prefix& p1, int32_t w1, const clustering_key_prefix& p2, int32_t w2) const {
auto type = _s.get().clustering_key_prefix_type();
auto res = prefix_equality_tri_compare(type->types().begin(),
type->begin(p1.representation()), type->end(p1.representation()),
type->begin(p2.representation()), type->end(p2.representation()),
::tri_compare);
- if (res) {
+ if (res != 0) {
return res;
}
auto d1 = p1.size(_s);
auto d2 = p2.size(_s);
if (d1 == d2) {
- return w1 - w2;
+ return w1 <=> w2;
}
- return d1 < d2 ? w1 - (w1 <= 0) : -(w2 - (w2 <= 0));
+ return (d1 < d2 ? w1 - (w1 <= 0) : -(w2 - (w2 <= 0))) <=> 0;
}
- int operator()(const bound_view b, const clustering_key_prefix& p) const {
+ std::strong_ordering operator()(const bound_view b, const clustering_key_prefix& p) const {
return operator()(b._prefix, weight(b._kind), p, 0);
}
- int operator()(const clustering_key_prefix& p, const bound_view b) const {
+ std::strong_ordering operator()(const clustering_key_prefix& p, const bound_view b) const {
return operator()(p, 0, b._prefix, weight(b._kind));
}
- int operator()(const bound_view b1, const bound_view b2) const {
+ std::strong_ordering operator()(const bound_view b1, const bound_view b2) const {
return operator()(b1._prefix, weight(b1._kind), b2._prefix, weight(b2._kind));
}
};
diff --git a/compound.hh b/compound.hh
--- a/compound.hh
+++ b/compound.hh
@@ -248,14 +248,14 @@ public:
}
return h;
}
- int compare(managed_bytes_view b1, managed_bytes_view b2) const {
+ std::strong_ordering compare(managed_bytes_view b1, managed_bytes_view b2) const {
return with_linearized(b1, [&] (bytes_view bv1) {
return with_linearized(b2, [&] (bytes_view bv2) {
return compare(bv1, bv2);
});
});
}
- int compare(bytes_view b1, bytes_view b2) const {
+ std::strong_ordering compare(bytes_view b1, bytes_view b2) const {
if (_byte_order_comparable) {
if (_is_reversed) {
return compare_unsigned(b2, b1);
diff --git a/compound_compat.hh b/compound_compat.hh
--- a/compound_compat.hh
+++ b/compound_compat.hh
@@ -23,6 +23,7 @@

#include <boost/range/algorithm/copy.hpp>
#include <boost/range/adaptor/transformed.hpp>
+#include <compare>
#include "compound.hh"
#include "schema.hh"
#include "sstables/version.hh"
@@ -148,16 +149,16 @@ public:
{ }

// @k1 and @k2 must be serialized using @type, which was passed to the constructor.
- int operator()(managed_bytes_view k1, managed_bytes_view k2) const {
+ std::strong_ordering operator()(managed_bytes_view k1, managed_bytes_view k2) const {
if (_type.is_singular()) {
return compare_unsigned(*_type.begin(k1), *_type.begin(k2));
}
return lexicographical_tri_compare(
_type.begin(k1), _type.end(k1),
_type.begin(k2), _type.end(k2),
- [] (const managed_bytes_view& c1, const managed_bytes_view& c2) -> int {
+ [] (const managed_bytes_view& c1, const managed_bytes_view& c2) -> std::strong_ordering {
if (c1.size() != c2.size() || !c1.size()) {
- return c1.size() < c2.size() ? -1 : c1.size() ? 1 : 0;
+ return c1.size() < c2.size() ? std::strong_ordering::less : c1.size() ? std::strong_ordering::greater : std::strong_ordering::equal;
}
return compare_unsigned(c1, c2);
});
@@ -523,8 +524,8 @@ public:
struct tri_compare {
const std::vector<data_type>& _types;
tri_compare(const std::vector<data_type>& types) : _types(types) {}
- int operator()(const composite&, const composite&) const;
- int operator()(composite_view, composite_view) const;
+ std::strong_ordering operator()(const composite&, const composite&) const;
+ std::strong_ordering operator()(composite_view, composite_view) const;
};
};

@@ -640,31 +641,31 @@ std::ostream& operator<<(std::ostream& os, const composite& v) {
}

inline
-int composite::tri_compare::operator()(const composite& v1, const composite& v2) const {
+std::strong_ordering composite::tri_compare::operator()(const composite& v1, const composite& v2) const {
return (*this)(composite_view(v1), composite_view(v2));
}

inline
-int composite::tri_compare::operator()(composite_view v1, composite_view v2) const {
+std::strong_ordering composite::tri_compare::operator()(composite_view v1, composite_view v2) const {
// See org.apache.cassandra.db.composites.AbstractCType#compare
if (v1.empty()) {
- return v2.empty() ? 0 : -1;
+ return v2.empty() ? std::strong_ordering::equal : std::strong_ordering::less;
}
if (v2.empty()) {
- return 1;
+ return std::strong_ordering::greater;
}
if (v1.is_static() != v2.is_static()) {
- return v1.is_static() ? -1 : 1;
+ return v1.is_static() ? std::strong_ordering::less : std::strong_ordering::greater;
}
auto a_values = v1.components();
auto b_values = v2.components();
auto cmp = [&](const data_type& t, component_view c1, component_view c2) {
// First by value, then by EOC
auto r = t->compare(c1.first, c2.first);
- if (r) {
+ if (r != 0) {
return r;
}
- return static_cast<int>(c1.second) - static_cast<int>(c2.second);
+ return (static_cast<int>(c1.second) - static_cast<int>(c2.second)) <=> 0;
};
return lexicographical_tri_compare(_types.begin(), _types.end(),
a_values.begin(), a_values.end(),
diff --git a/cql3/column_condition.cc b/cql3/column_condition.cc
--- a/cql3/column_condition.cc
+++ b/cql3/column_condition.cc
@@ -68,7 +68,7 @@ void validate_operation_on_durations(const abstract_type& type, cql3::expr::oper
int is_satisfied_by(cql3::expr::oper_t op, const abstract_type& cell_type,
const abstract_type& param_type, const data_value& cell_value, const bytes& param) {

- int rc;
+ std::strong_ordering rc = std::strong_ordering::equal;
// For multi-cell sets and lists, cell value is represented as a map,
// thanks to collections_as_maps flag in partition_slice. param, however,
// is represented as a set or list type.
@@ -79,7 +79,7 @@ int is_satisfied_by(cql3::expr::oper_t op, const abstract_type& cell_type,
const map_type_impl& map_type = static_cast<const map_type_impl&>(cell_type);
assert(list_type.is_multi_cell());
// Inverse comparison result since the order of arguments is inverse.
- rc = -list_type.compare_with_map(map_type, param, map_type.decompose(cell_value));
+ rc = 0 <=> list_type.compare_with_map(map_type, param, map_type.decompose(cell_value));
} else {
rc = cell_type.compare(cell_type.decompose(cell_value), param);
}
diff --git a/cql3/statements/select_statement.cc b/cql3/statements/select_statement.cc
--- a/cql3/statements/select_statement.cc
+++ b/cql3/statements/select_statement.cc
@@ -1591,7 +1591,7 @@ select_statement::get_ordering_comparator(const schema& schema,
return bool(c2);
}
if (c1) {
- int result = type->compare(*c1, *c2);
+ auto result = type->compare(*c1, *c2);
if (result != 0) {
return result < 0;
}
diff --git a/cql3/update_parameters.hh b/cql3/update_parameters.hh
--- a/cql3/update_parameters.hh
+++ b/cql3/update_parameters.hh
@@ -83,7 +83,7 @@ public:
const partition_key& pk2, const clustering_key& ck2) const {

std::strong_ordering rc = pk_cmp(pk1, pk2);
- return rc != 0 ? rc : ck_cmp(ck1, ck2) <=> 0;
+ return rc != 0 ? rc : ck_cmp(ck1, ck2);
}
// Allow mixing std::pair<partition_key, clustering_key> and
// std::pair<const partition_key&, const clustering_key&> during lookup
diff --git a/database.cc b/database.cc
--- a/database.cc
+++ b/database.cc
@@ -1364,25 +1364,25 @@ database::existing_index_names(const sstring& ks_name, const sstring& cf_to_excl
// - org.apache.cassandra.db.AbstractCell#reconcile()
// - org.apache.cassandra.db.BufferExpiringCell#reconcile()
// - org.apache.cassandra.db.BufferDeletedCell#reconcile()
-int
+std::strong_ordering
compare_atomic_cell_for_merge(atomic_cell_view left, atomic_cell_view right) {
if (left.timestamp() != right.timestamp()) {
- return left.timestamp() > right.timestamp() ? 1 : -1;
+ return left.timestamp() <=> right.timestamp();
}
if (left.is_live() != right.is_live()) {
- return left.is_live() ? -1 : 1;
+ return left.is_live() ? std::strong_ordering::less : std::strong_ordering::greater;
}
if (left.is_live()) {
- auto c = compare_unsigned(left.value(), right.value());
+ auto c = compare_unsigned(left.value(), right.value()) <=> 0;
if (c != 0) {
return c;
}
if (left.is_live_and_has_ttl() != right.is_live_and_has_ttl()) {
// prefer expiring cells.
- return left.is_live_and_has_ttl() ? 1 : -1;
+ return left.is_live_and_has_ttl() ? std::strong_ordering::greater : std::strong_ordering::less;
}
if (left.is_live_and_has_ttl() && left.expiry() != right.expiry()) {
- return left.expiry() < right.expiry() ? -1 : 1;
+ return left.expiry() <=> right.expiry();
}
} else {
// Both are deleted
@@ -1392,10 +1392,10 @@ compare_atomic_cell_for_merge(atomic_cell_view left, atomic_cell_view right) {
// comparing timestamps, which in case of deleted cells will hold
// serialized expiry.
return (uint64_t) left.deletion_time().time_since_epoch().count()
- < (uint64_t) right.deletion_time().time_since_epoch().count() ? -1 : 1;
+ <=> (uint64_t) right.deletion_time().time_since_epoch().count();
}
}
- return 0;
+ return std::strong_ordering::equal;
}

future<std::tuple<lw_shared_ptr<query::result>, cache_temperature>>
diff --git a/dht/i_partitioner.cc b/dht/i_partitioner.cc
--- a/dht/i_partitioner.cc
+++ b/dht/i_partitioner.cc
@@ -103,7 +103,7 @@ decorated_key::tri_compare(const schema& s, const decorated_key& other) const {
if (r != 0) {
return r;
} else {
- return _key.legacy_tri_compare(s, other._key) <=> 0;
+ return _key.legacy_tri_compare(s, other._key);
}
}

@@ -113,7 +113,7 @@ decorated_key::tri_compare(const schema& s, const ring_position& other) const {
if (r != 0) {
return r;
} else if (other.has_key()) {
- return _key.legacy_tri_compare(s, *other.key()) <=> 0;
+ return _key.legacy_tri_compare(s, *other.key());
}
return 0 <=> other.relation_to_keys();
}
diff --git a/interval.hh b/interval.hh
--- a/interval.hh
+++ b/interval.hh
@@ -112,7 +112,7 @@ private:
template<typename Comparator>
static bool greater_than_or_equal(end_bound_ref end, start_bound_ref start, Comparator&& cmp) {
return !end.b || !start.b || require_ordering_and_on_equal_return(
- cmp(end.b->value(), start.b->value()) <=> 0,
+ cmp(end.b->value(), start.b->value()),
std::strong_ordering::greater,
end.b->is_inclusive() && start.b->is_inclusive());
}
@@ -125,23 +125,23 @@ private:
template<typename Comparator>
static bool less_than_or_equal(start_bound_ref first, start_bound_ref second, Comparator&& cmp) {
return !first.b || (second.b && require_ordering_and_on_equal_return(
- cmp(first.b->value(), second.b->value()) <=> 0,
+ cmp(first.b->value(), second.b->value()),
std::strong_ordering::less,
first.b->is_inclusive() || !second.b->is_inclusive()));
}

template<typename Comparator>
static bool less_than(start_bound_ref first, start_bound_ref second, Comparator&& cmp) {
return second.b && (!first.b || require_ordering_and_on_equal_return(
- cmp(first.b->value(), second.b->value()) <=> 0,
+ cmp(first.b->value(), second.b->value()),
std::strong_ordering::less,
first.b->is_inclusive() && !second.b->is_inclusive()));
}

template<typename Comparator>
static bool greater_than_or_equal(end_bound_ref first, end_bound_ref second, Comparator&& cmp) {
return !first.b || (second.b && require_ordering_and_on_equal_return(
- cmp(first.b->value(), second.b->value()) <=> 0,
+ cmp(first.b->value(), second.b->value()),
std::strong_ordering::greater,
first.b->is_inclusive() || !second.b->is_inclusive()));
}
@@ -284,11 +284,11 @@ public:

if (this_wraps && other_wraps) {
return require_ordering_and_on_equal_return(
- cmp(start()->value(), other.start()->value()) <=> 0,
+ cmp(start()->value(), other.start()->value()),
std::strong_ordering::less,
start()->is_inclusive() || !other.start()->is_inclusive())
&& require_ordering_and_on_equal_return(
- cmp(end()->value(), other.end()->value()) <=> 0,
+ cmp(end()->value(), other.end()->value()),
std::strong_ordering::greater,
end()->is_inclusive() || !other.end()->is_inclusive());
}
@@ -304,11 +304,11 @@ public:

// !other_wraps && this_wraps
return (other.start() && require_ordering_and_on_equal_return(
- cmp(start()->value(), other.start()->value()) <=> 0,
+ cmp(start()->value(), other.start()->value()),
std::strong_ordering::less,
start()->is_inclusive() || !other.start()->is_inclusive()))
|| (other.end() && (require_ordering_and_on_equal_return(
- cmp(end()->value(), other.end()->value()) <=> 0,
+ cmp(end()->value(), other.end()->value()),
std::strong_ordering::greater,
end()->is_inclusive() || !other.end()->is_inclusive())));
}
diff --git a/keys.cc b/keys.cc
--- a/keys.cc
+++ b/keys.cc
@@ -77,7 +77,7 @@ partition_key_view::legacy_form(const schema& s) const {
std::strong_ordering
partition_key_view::legacy_tri_compare(const schema& s, partition_key_view o) const {
auto cmp = legacy_compound_view<c_type>::tri_comparator(*get_compound_type(s));
- return cmp(this->representation(), o.representation()) <=> 0;
+ return cmp(this->representation(), o.representation());
}

std::strong_ordering
diff --git a/keys.hh b/keys.hh
--- a/keys.hh
+++ b/keys.hh
@@ -86,7 +86,7 @@ public:
typename TopLevelView::compound _t;
tri_compare(const schema &s) : _t(get_compound_type(s)) {}
std::strong_ordering operator()(const TopLevelView& k1, const TopLevelView& k2) const {
- return _t->compare(k1.representation(), k2.representation()) <=> 0;
+ return _t->compare(k1.representation(), k2.representation());
}
};

@@ -273,13 +273,13 @@ public:
typename TopLevel::compound _t;
tri_compare(const schema& s) : _t(get_compound_type(s)) {}
std::strong_ordering operator()(const TopLevel& k1, const TopLevel& k2) const {
- return _t->compare(k1.representation(), k2.representation()) <=> 0;
+ return _t->compare(k1.representation(), k2.representation());
}
std::strong_ordering operator()(const TopLevelView& k1, const TopLevel& k2) const {
- return _t->compare(k1.representation(), k2.representation()) <=> 0;
+ return _t->compare(k1.representation(), k2.representation());
}
std::strong_ordering operator()(const TopLevel& k1, const TopLevelView& k2) const {
- return _t->compare(k1.representation(), k2.representation()) <=> 0;
+ return _t->compare(k1.representation(), k2.representation());
}
};

@@ -641,7 +641,7 @@ public:
return prefix_equality_tri_compare(prefix_type->types().begin(),
prefix_type->begin(k1.representation()), prefix_type->end(k1.representation()),
prefix_type->begin(k2.representation()), prefix_type->end(k2.representation()),
- tri_compare) <=> 0;
+ tri_compare);
}
};
};
diff --git a/position_in_partition.hh b/position_in_partition.hh
--- a/position_in_partition.hh
+++ b/position_in_partition.hh
@@ -380,12 +380,12 @@ public:

composite_tri_compare(const schema& s) : _s(s) {}

- int operator()(position_in_partition_view a, position_in_partition_view b) const {
+ std::strong_ordering operator()(position_in_partition_view a, position_in_partition_view b) const {
if (a._type != b._type) {
- return rank(a._type) - rank(b._type);
+ return rank(a._type) <=> rank(b._type);
}
if (!a._ck) {
- return 0;
+ return std::strong_ordering::equal;
}
auto&& types = _s.clustering_key_type()->types();
auto cmp = [&] (const data_type& t, managed_bytes_view c1, managed_bytes_view c2) { return t->compare(c1, c2); };
@@ -395,16 +395,16 @@ public:
cmp, a.relation(), b.relation());
}

- int operator()(position_in_partition_view a, composite_view b) const {
+ std::strong_ordering operator()(position_in_partition_view a, composite_view b) const {
if (b.empty()) {
- return 1; // a cannot be empty.
+ return std::strong_ordering::greater; // a cannot be empty.
}
partition_region b_type = b.is_static() ? partition_region::static_row : partition_region::clustered;
if (a._type != b_type) {
- return rank(a._type) - rank(b_type);
+ return rank(a._type) <=> rank(b_type);
}
if (!a._ck) {
- return 0;
+ return std::strong_ordering::equal;
}
auto&& types = _s.clustering_key_type()->types();
auto b_values = b.values();
@@ -415,13 +415,13 @@ public:
cmp, a.relation(), relation_for_lower_bound(b));
}

- int operator()(composite_view a, position_in_partition_view b) const {
- return -(*this)(b, a);
+ std::strong_ordering operator()(composite_view a, position_in_partition_view b) const {
+ return 0 <=> (*this)(b, a);
}

- int operator()(composite_view a, composite_view b) const {
+ std::strong_ordering operator()(composite_view a, composite_view b) const {
if (a.is_static() != b.is_static()) {
- return a.is_static() ? -1 : 1;
+ return a.is_static() ? std::strong_ordering::less : std::strong_ordering::greater;
}
auto&& types = _s.clustering_key_type()->types();
auto a_values = a.values();
@@ -459,7 +459,7 @@ public:
if (!a._ck) {
return std::strong_ordering::equal;
}
- return _cmp(*a._ck, int8_t(a._bound_weight), *b._ck, int8_t(b._bound_weight)) <=> 0;
+ return _cmp(*a._ck, int8_t(a._bound_weight), *b._ck, int8_t(b._bound_weight));
}
public:
tri_compare(const schema& s) : _cmp(s) { }
diff --git a/sstables/key.hh b/sstables/key.hh
--- a/sstables/key.hh
+++ b/sstables/key.hh
@@ -58,15 +58,15 @@ public:
bool empty() const { return _bytes.empty(); }

std::strong_ordering tri_compare(key_view other) const {
- return compare_unsigned(_bytes, other._bytes) <=> 0;
+ return compare_unsigned(_bytes, other._bytes);
}

std::strong_ordering tri_compare(const schema& s, partition_key_view other) const {
return with_linearized([&] (bytes_view v) {
auto lf = other.legacy_form(s);
return lexicographical_tri_compare(
v.begin(), v.end(), lf.begin(), lf.end(),
- [](uint8_t b1, uint8_t b2) { return (int) b1 - b2; }) <=> 0;
+ [](uint8_t b1, uint8_t b2) { return b1 <=> b2; });
});
}
};
diff --git a/test/boost/UUID_test.cc b/test/boost/UUID_test.cc
--- a/test/boost/UUID_test.cc
+++ b/test/boost/UUID_test.cc
@@ -97,24 +97,24 @@ BOOST_AUTO_TEST_CASE(test_get_time_uuid) {
BOOST_CHECK(unix_timestamp == millis);
}

-int timeuuid_legacy_tri_compare(bytes_view o1, bytes_view o2) {
- auto compare_pos = [&] (unsigned pos, int mask, int ifequal) {
- int d = (o1[pos] & mask) - (o2[pos] & mask);
- return d ? d : ifequal;
+std::strong_ordering timeuuid_legacy_tri_compare(bytes_view o1, bytes_view o2) {
+ auto compare_pos = [&] (unsigned pos, int mask, std::strong_ordering ifequal) {
+ auto d = (o1[pos] & mask) <=> (o2[pos] & mask);
+ return d != 0 ? d : ifequal;
};
- int res = compare_pos(6, 0xf,
+ auto res = compare_pos(6, 0xf,
compare_pos(7, 0xff,
compare_pos(4, 0xff,
compare_pos(5, 0xff,
compare_pos(0, 0xff,
compare_pos(1, 0xff,
compare_pos(2, 0xff,
- compare_pos(3, 0xff, 0))))))));
+ compare_pos(3, 0xff, std::strong_ordering::equal))))))));
if (res == 0) {
res = lexicographical_tri_compare(o1.begin(), o1.end(), o2.begin(), o2.end(),
- [] (const int8_t& a, const int8_t& b) { return a - b; });
+ [] (const int8_t& a, const int8_t& b) { return a <=> b; });
}
- return res < 0 ? -1 : res > 0;
+ return res;
}

BOOST_AUTO_TEST_CASE(test_timeuuid_msb_is_monotonic) {
diff --git a/test/boost/btree_test.cc b/test/boost/btree_test.cc
--- a/test/boost/btree_test.cc
+++ b/test/boost/btree_test.cc
@@ -36,7 +36,7 @@ class test_key : public tree_test_key_base {
struct tri_compare {
test_key_tri_compare _cmp;
template <typename A, typename B>
- std::strong_ordering operator()(const A& a, const B& b) const noexcept { return _cmp(a, b) <=> 0; }
+ std::strong_ordering operator()(const A& a, const B& b) const noexcept { return _cmp(a, b); }
};
using test_tree = tree<test_key, &test_key::b_hook, tri_compare, 4, 5, key_search::both, with_debug::yes>;
test_key(int nr, int cookie) noexcept : tree_test_key_base(nr, cookie) {}
diff --git a/test/boost/mutation_test.cc b/test/boost/mutation_test.cc
--- a/test/boost/mutation_test.cc
+++ b/test/boost/mutation_test.cc
@@ -2253,7 +2253,7 @@ class clustering_fragment_summary::tri_cmp {
}
// Range tombstones can have the same start position. In this case use
// the end bound to decide who's "less".
- return _bv_cmp(a.end_bound(), b.end_bound()) <=> 0;
+ return _bv_cmp(a.end_bound(), b.end_bound());
}

public:
@@ -2283,7 +2283,7 @@ class clustering_fragment_summary::less_cmp {
}
};

-using collection_element_tri_cmp_type = std::function<int(const std::pair<bytes, cell_summary>&, const std::pair<bytes, cell_summary>&)>;
+using collection_element_tri_cmp_type = std::function<std::strong_ordering(const std::pair<bytes, cell_summary>&, const std::pair<bytes, cell_summary>&)>;

collection_element_tri_cmp_type
collection_element_tri_cmp(const abstract_type& type) {
@@ -2298,13 +2298,7 @@ collection_element_tri_cmp(const abstract_type& type) {
return [] (const std::pair<bytes, cell_summary>& a, const std::pair<bytes, cell_summary>& b) {
auto ai = deserialize_field_index(a.first);
auto bi = deserialize_field_index(b.first);
- if (ai < bi) {
- return -1;
- }
- if (ai == bi) {
- return 0;
- }
- return 1;
+ return ai <=> bi;
};
},
[] (const abstract_type& o) -> collection_element_tri_cmp_type {
@@ -2607,33 +2601,14 @@ void merge_container(
}
}

-// Temporary variant of merge_container() that accepts int as the return value of merge_func,
-// until we convert all users to std::strong_ordering.
-template <typename Container, typename OutputIt>
-void merge_container(
- Container&& a,
- Container&& b,
- OutputIt oit,
- std::function<int (const typename Container::value_type&, const typename Container::value_type&)> tri_cmp,
- std::function<typename Container::value_type(typename Container::value_type, typename Container::value_type)> merge_func) {
- return merge_container(
- std::forward<Container>(a),
- std::forward<Container>(b),
- oit,
- [tri_cmp] (const typename Container::value_type& a, const typename Container::value_type& b) {
- return tri_cmp(a, b) <=> 0;
- },
- std::move(merge_func));
-}
-
row_summary merge(const schema& schema, column_kind kind, row_summary a, row_summary b) {
row_summary merged;
merge_container(
std::move(a),
std::move(b),
std::inserter(merged, merged.end()),
- [] (const std::pair<const column_id, value_summary>& a, const std::pair<const column_id, value_summary>& b) -> int {
- return a.first - b.first;
+ [] (const std::pair<const column_id, value_summary>& a, const std::pair<const column_id, value_summary>& b) -> std::strong_ordering {
+ return a.first <=> b.first;
},
[&schema, kind] (std::pair<const column_id, value_summary> a, std::pair<const column_id, value_summary> b) {
const auto& cdef = schema.column_at(kind, a.first);
diff --git a/test/boost/nonwrapping_range_test.cc b/test/boost/nonwrapping_range_test.cc
--- a/test/boost/nonwrapping_range_test.cc
+++ b/test/boost/nonwrapping_range_test.cc
@@ -100,7 +100,7 @@ BOOST_AUTO_TEST_CASE(test_range_with_equal_value_but_opposite_inclusiveness_is_a
}

BOOST_AUTO_TEST_CASE(test_range_contains) {
- auto cmp = [] (int i1, int i2) -> int { return i1 - i2; };
+ auto cmp = [] (int i1, int i2) -> std::strong_ordering { return i1 <=> i2; };

auto check_contains = [&] (nonwrapping_range<int> enclosing, nonwrapping_range<int> enclosed) {
BOOST_REQUIRE(enclosing.contains(enclosed, cmp));
@@ -145,7 +145,7 @@ BOOST_AUTO_TEST_CASE(test_range_contains) {
}

BOOST_AUTO_TEST_CASE(test_range_subtract) {
- auto cmp = [] (int i1, int i2) -> int { return i1 - i2; };
+ auto cmp = [] (int i1, int i2) -> std::strong_ordering { return i1 <=> i2; };
using r = nonwrapping_range<int>;
using vec = std::vector<r>;

@@ -188,8 +188,8 @@ BOOST_AUTO_TEST_CASE(test_range_subtract) {
}

struct unsigned_comparator {
- int operator()(unsigned u1, unsigned u2) const {
- return (u1 > u2 ? 1 : (u1 == u2 ? 0 : -1));
+ std::strong_ordering operator()(unsigned u1, unsigned u2) const {
+ return u1 <=> u2;
}
};

diff --git a/test/boost/range_test.cc b/test/boost/range_test.cc
--- a/test/boost/range_test.cc
+++ b/test/boost/range_test.cc
@@ -106,7 +106,7 @@ BOOST_AUTO_TEST_CASE(test_range_with_positions_within_the_same_token) {
}

BOOST_AUTO_TEST_CASE(test_range_contains) {
- auto cmp = [] (int i1, int i2) -> int { return i1 - i2; };
+ auto cmp = [] (int i1, int i2) -> std::strong_ordering { return i1 <=> i2; };

auto check_contains = [&] (range<int> enclosing, range<int> enclosed) {
BOOST_REQUIRE(enclosing.contains(enclosed, cmp));
@@ -165,7 +165,7 @@ BOOST_AUTO_TEST_CASE(test_range_contains) {
}

BOOST_AUTO_TEST_CASE(test_range_subtract) {
- auto cmp = [] (int i1, int i2) -> int { return i1 - i2; };
+ auto cmp = [] (int i1, int i2) -> std::strong_ordering { return i1 <=> i2; };
using r = range<int>;
using vec = std::vector<r>;

@@ -216,8 +216,8 @@ BOOST_AUTO_TEST_CASE(test_range_subtract) {
}

struct unsigned_comparator {
- int operator()(unsigned u1, unsigned u2) const {
- return (u1 > u2 ? 1 : (u1 == u2 ? 0 : -1));
+ std::strong_ordering operator()(unsigned u1, unsigned u2) const {
+ return u1 <=> u2;
}
};

diff --git a/test/boost/total_order_check.hh b/test/boost/total_order_check.hh
--- a/test/boost/total_order_check.hh
+++ b/test/boost/total_order_check.hh
@@ -44,7 +44,7 @@ private:
seastar::visit(right_e, [&] (auto&& b) {
testlog.trace("cmp({}, {}) == {}", a, b, order);
auto r = _cmp(a, b);
- auto actual = r <=> 0;
+ auto actual = r;
if (actual != order) {
BOOST_FAIL(format("Expected cmp({}, {}) == {}, but got {}", a, b, order, actual));
}
diff --git a/test/boost/types_test.cc b/test/boost/types_test.cc
--- a/test/boost/types_test.cc
+++ b/test/boost/types_test.cc
@@ -734,7 +734,7 @@ BOOST_AUTO_TEST_CASE(test_create_reversed_type) {
auto val_gt = bytes_type->decompose(data_value(bytes("b")));
auto straight_comp = bytes_type->compare(bytes_view(val_lt), bytes_view(val_gt));
auto reverse_comp = ri->compare(bytes_view(val_lt), bytes_view(val_gt));
- BOOST_REQUIRE(straight_comp == -reverse_comp);
+ BOOST_REQUIRE(straight_comp == (0 <=> reverse_comp));
}

BOOST_AUTO_TEST_CASE(test_reversed_type_to_string) {
@@ -766,7 +766,7 @@ BOOST_AUTO_TEST_CASE(test_create_reverse_collection_type) {

auto straight_comp = my_set_type->compare(bytes_view(bv1), bytes_view(bv2));
auto reverse_comp = ri->compare(bytes_view(bv2), bytes_view(bv2));
- BOOST_REQUIRE(straight_comp == -reverse_comp);
+ BOOST_REQUIRE(straight_comp == (0 <=> reverse_comp));
}

BOOST_AUTO_TEST_CASE(test_parse_reversed_type) {
@@ -782,7 +782,7 @@ BOOST_AUTO_TEST_CASE(test_parse_reversed_type) {
auto val_gt = int32_type->decompose(2);
auto straight_comp = int32_type->compare(bytes_view(val_lt), bytes_view(val_gt));
auto reverse_comp = ri->compare(bytes_view(val_lt), bytes_view(val_gt));
- BOOST_REQUIRE(straight_comp == -reverse_comp);
+ BOOST_REQUIRE(straight_comp == (0 <=> reverse_comp));
}

BOOST_AUTO_TEST_CASE(test_reversed_type_value_compatibility) {
diff --git a/test/perf/perf_collection.cc b/test/perf/perf_collection.cc
--- a/test/perf/perf_collection.cc
+++ b/test/perf/perf_collection.cc
@@ -34,14 +34,8 @@ struct key_compare {
};

struct key_tri_compare {
- int operator()(const per_key_t& a, const per_key_t& b) const noexcept {
- if (a > b) {
- return 1;
- } else if (a < b) {
- return -1;
- } else {
- return 0;
- }
+ std::strong_ordering operator()(const per_key_t& a, const per_key_t& b) const noexcept {
+ return a <=> b;
}
};

@@ -65,10 +59,10 @@ class perf_intrusive_key {

struct tri_compare {
key_tri_compare _cmp;
- std::strong_ordering operator()(const per_key_t a, const per_key_t b) const noexcept { return _cmp(a, b) <=> 0; }
- std::strong_ordering operator()(const perf_intrusive_key& a, const perf_intrusive_key& b) const noexcept { return _cmp(a._k, b._k) <=> 0; }
- std::strong_ordering operator()(const per_key_t a, const perf_intrusive_key& b) const noexcept { return _cmp(a, b._k) <=> 0; }
- std::strong_ordering operator()(const perf_intrusive_key& a, const per_key_t b) const noexcept { return _cmp(a._k, b) <=> 0; }
+ std::strong_ordering operator()(const per_key_t a, const per_key_t b) const noexcept { return _cmp(a, b); }
+ std::strong_ordering operator()(const perf_intrusive_key& a, const perf_intrusive_key& b) const noexcept { return _cmp(a._k, b._k); }
+ std::strong_ordering operator()(const per_key_t a, const perf_intrusive_key& b) const noexcept { return _cmp(a, b._k); }
+ std::strong_ordering operator()(const perf_intrusive_key& a, const per_key_t b) const noexcept { return _cmp(a._k, b); }
};
};

diff --git a/test/perf/perf_fast_forward.cc b/test/perf/perf_fast_forward.cc
--- a/test/perf/perf_fast_forward.cc
+++ b/test/perf/perf_fast_forward.cc
@@ -1198,7 +1198,7 @@ static unsigned cardinality(std::optional<int_range> ropt) {

static std::optional<int_range> intersection(int_range a, int_range b) {
auto int_tri_cmp = [] (int x, int y) {
- return x < y ? -1 : (x > y ? 1 : 0);
+ return x <=> y;
};
return a.intersection(b, int_tri_cmp);
}
diff --git a/tools/scylla-types.cc b/tools/scylla-types.cc
--- a/tools/scylla-types.cc
+++ b/tools/scylla-types.cc
@@ -74,13 +74,13 @@ void compare_handler(type_variant type, std::vector<bytes> values) {
struct {
bytes_view lhs, rhs;

- int operator()(const data_type& type) {
+ std::strong_ordering operator()(const data_type& type) {
return type->compare(lhs, rhs);
}
- int operator()(const compound_type<allow_prefixes::yes>& type) {
+ std::strong_ordering operator()(const compound_type<allow_prefixes::yes>& type) {
return type.compare(lhs, rhs);
}
- int operator()(const compound_type<allow_prefixes::no>& type) {
+ std::strong_ordering operator()(const compound_type<allow_prefixes::no>& type) {
return type.compare(lhs, rhs);
}
} compare_visitor{values[0], values[1]};
diff --git a/types.cc b/types.cc
--- a/types.cc
+++ b/types.cc
@@ -512,14 +512,14 @@ listlike_collection_type_impl::listlike_collection_type_impl(
kind k, sstring name, data_type elements, bool is_multi_cell)
: collection_type_impl(k, name, is_multi_cell), _elements(elements) {}

-int listlike_collection_type_impl::compare_with_map(const map_type_impl& map_type, bytes_view list, bytes_view map) const
+std::strong_ordering listlike_collection_type_impl::compare_with_map(const map_type_impl& map_type, bytes_view list, bytes_view map) const
{
assert((is_set() && map_type.get_keys_type() == _elements) || (!is_set() && map_type.get_values_type() == _elements));

if (list.empty()) {
- return map.empty() ? 0 : -1;
+ return map.empty() ? std::strong_ordering::equal : std::strong_ordering::less;
} else if (map.empty()) {
- return 1;
+ return std::strong_ordering::greater;
}

const abstract_type& element_type = *_elements;
@@ -545,7 +545,7 @@ int listlike_collection_type_impl::compare_with_map(const map_type_impl& map_typ
return cmp;
}
}
- return list_size == map_size ? 0 : (list_size < map_size ? -1 : 1);
+ return list_size <=> map_size;
}

bytes listlike_collection_type_impl::serialize_map(const map_type_impl& map_type, const data_value& value) const
@@ -1078,12 +1078,12 @@ map_type_impl::is_value_compatible_with_frozen(const collection_type_impl& previ
&& _values->is_value_compatible_with(*p->_values);
}

-int32_t
+std::strong_ordering
map_type_impl::compare_maps(data_type keys, data_type values, managed_bytes_view o1, managed_bytes_view o2) {
if (o1.empty()) {
- return o2.empty() ? 0 : -1;
+ return o2.empty() ? std::strong_ordering::equal : std::strong_ordering::less;
} else if (o2.empty()) {
- return 1;
+ return std::strong_ordering::greater;
}
auto sf = cql_serialization_format::internal();
int size1 = read_collection_size(o1, sf);
@@ -1103,7 +1103,7 @@ map_type_impl::compare_maps(data_type keys, data_type values, managed_bytes_view
return cmp;
}
}
- return size1 == size2 ? 0 : (size1 < size2 ? -1 : 1);
+ return size1 <=> size2;
}

static size_t map_serialized_size(const map_type_impl::native_type* m) {
@@ -2141,7 +2141,7 @@ template data_value abstract_type::deserialize_impl<>(single_fragmented_view) co
template data_value abstract_type::deserialize_impl<>(ser::buffer_view<bytes_ostream::fragment_iterator>) const;
template data_value abstract_type::deserialize_impl<>(managed_bytes_view) const;

-int32_t compare_aux(const tuple_type_impl& t, const managed_bytes_view& v1, const managed_bytes_view& v2) {
+std::strong_ordering compare_aux(const tuple_type_impl& t, const managed_bytes_view& v1, const managed_bytes_view& v2) {
// This is a slight modification of lexicographical_tri_compare:
// when the only difference between the tuples is that one of them has additional trailing nulls,
// we consider them equal. For example, in the following CQL scenario:
@@ -2163,7 +2163,7 @@ int32_t compare_aux(const tuple_type_impl& t, const managed_bytes_view& v1, cons
auto last2 = tuple_deserializing_iterator::finish(v2);

while (types_first != types_last && first1 != last1 && first2 != last2) {
- if (auto c = tri_compare_opt(*types_first, *first1, *first2)) {
+ if (auto c = tri_compare_opt(*types_first, *first1, *first2); c != 0) {
return c;
}

@@ -2174,7 +2174,7 @@ int32_t compare_aux(const tuple_type_impl& t, const managed_bytes_view& v1, cons

while (types_first != types_last && first1 != last1) {
if (*first1) {
- return 1;
+ return std::strong_ordering::greater;
}

++first1;
@@ -2183,75 +2183,81 @@ int32_t compare_aux(const tuple_type_impl& t, const managed_bytes_view& v1, cons

while (types_first != types_last && first2 != last2) {
if (*first2) {
- return -1;
+ return std::strong_ordering::less;
}

++first2;
++types_first;
}

- return 0;
+ return std::strong_ordering::equal;
}

namespace {
+
struct compare_visitor {
managed_bytes_view v1;
managed_bytes_view v2;
- template <typename T> int32_t operator()(const simple_type_impl<T>&) {
+
+ template <std::invocable<> Func>
+ requires std::same_as<std::strong_ordering, std::invoke_result_t<Func>>
+ std::strong_ordering with_empty_checks(Func func) {
if (v1.empty()) {
- return v2.empty() ? 0 : -1;
+ return v2.empty() ? std::strong_ordering::equal : std::strong_ordering::less;
}
if (v2.empty()) {
- return 1;
+ return std::strong_ordering::greater;
}
+ return func();
+ }
+
+ template <typename T> std::strong_ordering operator()(const simple_type_impl<T>&) {
+ return with_empty_checks([&] {
T a = simple_type_traits<T>::read_nonempty(v1);
T b = simple_type_traits<T>::read_nonempty(v2);
- return a == b ? 0 : a < b ? -1 : 1;
- }
- int32_t operator()(const string_type_impl&) { return compare_unsigned(v1, v2); }
- int32_t operator()(const bytes_type_impl&) { return compare_unsigned(v1, v2); }
- int32_t operator()(const duration_type_impl&) { return compare_unsigned(v1, v2); }
- int32_t operator()(const inet_addr_type_impl&) { return compare_unsigned(v1, v2); }
- int32_t operator()(const date_type_impl&) {
+ return a <=> b;
+ });
+ }
+ std::strong_ordering operator()(const string_type_impl&) { return compare_unsigned(v1, v2); }
+ std::strong_ordering operator()(const bytes_type_impl&) { return compare_unsigned(v1, v2); }
+ std::strong_ordering operator()(const duration_type_impl&) { return compare_unsigned(v1, v2); }
+ std::strong_ordering operator()(const inet_addr_type_impl&) { return compare_unsigned(v1, v2); }
+ std::strong_ordering operator()(const date_type_impl&) {
// This is not the same behaviour as timestamp_type_impl
return compare_unsigned(v1, v2);
}
- int32_t operator()(const timeuuid_type_impl&) {
- if (v1.empty()) {
- return v2.empty() ? 0 : -1;
- }
- if (v2.empty()) {
- return 1;
- }
+ std::strong_ordering operator()(const timeuuid_type_impl&) {
+ return with_empty_checks([&] {
return with_linearized(v1, [&] (bytes_view v1) {
return with_linearized(v2, [&] (bytes_view v2) {
return utils::timeuuid_tri_compare(v1, v2);
});
});
+ });
}
- int32_t operator()(const listlike_collection_type_impl& l) {
+ std::strong_ordering operator()(const listlike_collection_type_impl& l) {
using llpdi = listlike_partial_deserializing_iterator;
auto sf = cql_serialization_format::internal();
return lexicographical_tri_compare(llpdi::begin(v1, sf), llpdi::end(v1, sf), llpdi::begin(v2, sf),
llpdi::end(v2, sf),
[&] (const managed_bytes_view& o1, const managed_bytes_view& o2) { return l.get_elements_type()->compare(o1, o2); });
}
- int32_t operator()(const map_type_impl& m) {
+ std::strong_ordering operator()(const map_type_impl& m) {
return map_type_impl::compare_maps(m.get_keys_type(), m.get_values_type(), v1, v2);
}
- int32_t operator()(const uuid_type_impl&) {
+ std::strong_ordering operator()(const uuid_type_impl&) {
if (v1.size() < 16) {
- return v2.size() < 16 ? 0 : -1;
+ return v2.size() < 16 ? std::strong_ordering::equal : std::strong_ordering::less;
}
if (v2.size() < 16) {

- return 1;
+ return std::strong_ordering::greater;
}
auto c1 = (v1[6] >> 4) & 0x0f;
auto c2 = (v2[6] >> 4) & 0x0f;

if (c1 != c2) {
- return c1 - c2;
+ return c1 <=> c2;
}

if (c1 == 1) {
@@ -2263,71 +2269,60 @@ struct compare_visitor {
}
return compare_unsigned(v1, v2);
}
- int32_t operator()(const empty_type_impl&) { return 0; }
- int32_t operator()(const tuple_type_impl& t) { return compare_aux(t, v1, v2); }
- int32_t operator()(const counter_type_impl&) {
+ std::strong_ordering operator()(const empty_type_impl&) { return std::strong_ordering::equal; }
+ std::strong_ordering operator()(const tuple_type_impl& t) { return compare_aux(t, v1, v2); }
+ std::strong_ordering operator()(const counter_type_impl&) {
// untouched (empty) counter evaluates as 0
const auto a = v1.empty() ? 0 : simple_type_traits<int64_t>::read_nonempty(v1);
const auto b = v2.empty() ? 0 : simple_type_traits<int64_t>::read_nonempty(v2);
- return a == b ? 0 : a < b ? -1 : 1;
+ return a <=> b;
}
- int32_t operator()(const decimal_type_impl& d) {
- if (v1.empty()) {
- return v2.empty() ? 0 : -1;
- }
- if (v2.empty()) {
- return 1;
- }
+ std::strong_ordering operator()(const decimal_type_impl& d) {
+ return with_empty_checks([&] {
auto a = deserialize_value(d, v1);
auto b = deserialize_value(d, v2);
return a.compare(b);
+ });
}
- int32_t operator()(const varint_type_impl& v) {
- if (v1.empty()) {
- return v2.empty() ? 0 : -1;
- }
- if (v2.empty()) {
- return 1;
- }
+ std::strong_ordering operator()(const varint_type_impl& v) {
+ return with_empty_checks([&] {
auto a = deserialize_value(v, v1);
auto b = deserialize_value(v, v2);
- return a == b ? 0 : a < b ? -1 : 1;
+ return a == b ? std::strong_ordering::equal : a < b ? std::strong_ordering::less : std::strong_ordering::greater;
+ });
}
- template <typename T> int32_t operator()(const floating_type_impl<T>&) {
- if (v1.empty()) {
- return v2.empty() ? 0 : -1;
- }
- if (v2.empty()) {
- return 1;
- }
+ template <typename T> std::strong_ordering operator()(const floating_type_impl<T>&) {
+ return with_empty_checks([&] {
T a = simple_type_traits<T>::read_nonempty(v1);
T b = simple_type_traits<T>::read_nonempty(v2);

// in java world NaN == NaN and NaN is greater than anything else
if (std::isnan(a) && std::isnan(b)) {
- return 0;
+ return std::strong_ordering::equal;
} else if (std::isnan(a)) {
- return 1;
+ return std::strong_ordering::greater;
} else if (std::isnan(b)) {
- return -1;
+ return std::strong_ordering::less;
}
// also -0 < 0
if (std::signbit(a) && !std::signbit(b)) {
- return -1;
+ return std::strong_ordering::less;
} else if (!std::signbit(a) && std::signbit(b)) {
- return 1;
+ return std::strong_ordering::greater;
}
- return a == b ? 0 : a < b ? -1 : 1;
+ // note: float <=> returns std::partial_ordering
+ return a == b ? std::strong_ordering::equal : a < b ? std::strong_ordering::less : std::strong_ordering::greater;
+ });
}
- int32_t operator()(const reversed_type_impl& r) { return r.underlying_type()->compare(v2, v1); }
+ std::strong_ordering operator()(const reversed_type_impl& r) { return r.underlying_type()->compare(v2, v1); }
};
}

-int32_t abstract_type::compare(bytes_view v1, bytes_view v2) const {
+std::strong_ordering abstract_type::compare(bytes_view v1, bytes_view v2) const {
return compare(managed_bytes_view(v1), managed_bytes_view(v2));
}

-int32_t abstract_type::compare(managed_bytes_view v1, managed_bytes_view v2) const {
+std::strong_ordering abstract_type::compare(managed_bytes_view v1, managed_bytes_view v2) const {
try {
return visit(*this, compare_visitor{v1, v2});
} catch (const marshal_exception&) {
diff --git a/types.hh b/types.hh
--- a/types.hh
+++ b/types.hh
@@ -111,23 +111,19 @@ bool lexicographical_compare(TypesIterator types, InputIt1 first1, InputIt1 last
// Like std::lexicographical_compare but injects values from shared sequence
// (types) to the comparator. Compare is an abstract_type-aware trichotomic
// comparator, which takes the type as first argument.
-//
-// A trichotomic comparator returns an integer which is less, equal or greater
-// than zero when the first value is respectively smaller, equal or greater
-// than the second value.
template <std::input_iterator TypesIterator, std::input_iterator InputIt1, std::input_iterator InputIt2, typename Compare>
requires requires (TypesIterator types, InputIt1 i1, InputIt2 i2, Compare cmp) {
- { cmp(*types, *i1, *i2) } -> std::same_as<int>;
+ { cmp(*types, *i1, *i2) } -> std::same_as<std::strong_ordering>;
}
-int lexicographical_tri_compare(TypesIterator types_first, TypesIterator types_last,
+std::strong_ordering lexicographical_tri_compare(TypesIterator types_first, TypesIterator types_last,
InputIt1 first1, InputIt1 last1,
InputIt2 first2, InputIt2 last2,
Compare comp,
lexicographical_relation relation1 = lexicographical_relation::before_all_strictly_prefixed,
lexicographical_relation relation2 = lexicographical_relation::before_all_strictly_prefixed) {
while (types_first != types_last && first1 != last1 && first2 != last2) {
auto c = comp(*types_first, *first1, *first2);
- if (c) {
+ if (c != 0) {
return c;
}
++first1;
@@ -137,30 +133,30 @@ int lexicographical_tri_compare(TypesIterator types_first, TypesIterator types_l
bool e1 = first1 == last1;
bool e2 = first2 == last2;
if (e1 && e2) {
- return static_cast<int>(relation1) - static_cast<int>(relation2);
+ return static_cast<int>(relation1) <=> static_cast<int>(relation2);
}
if (e2) {
- return relation2 == lexicographical_relation::after_all_prefixed ? -1 : 1;
+ return relation2 == lexicographical_relation::after_all_prefixed ? std::strong_ordering::less : std::strong_ordering::greater;
} else if (e1) {
- return relation1 == lexicographical_relation::after_all_prefixed ? 1 : -1;
+ return relation1 == lexicographical_relation::after_all_prefixed ? std::strong_ordering::greater : std::strong_ordering::less;
} else {
- return 0;
+ return std::strong_ordering::equal;
}
}

// Trichotomic version of std::lexicographical_compare()
-//
-// Returns an integer which is less, equal or greater than zero when the first value
-// is respectively smaller, equal or greater than the second value.
template <typename InputIt1, typename InputIt2, typename Compare>
-int lexicographical_tri_compare(InputIt1 first1, InputIt1 last1,
+requires requires (InputIt1 i1, InputIt2 i2, Compare c) {
+ { c(*i1, *i2) } -> std::same_as<std::strong_ordering>;
+}
+std::strong_ordering lexicographical_tri_compare(InputIt1 first1, InputIt1 last1,
InputIt2 first2, InputIt2 last2,
Compare comp,
lexicographical_relation relation1 = lexicographical_relation::before_all_strictly_prefixed,
lexicographical_relation relation2 = lexicographical_relation::before_all_strictly_prefixed) {
while (first1 != last1 && first2 != last2) {
auto c = comp(*first1, *first2);
- if (c) {
+ if (c != 0) {
return c;
}
++first1;
@@ -169,12 +165,12 @@ int lexicographical_tri_compare(InputIt1 first1, InputIt1 last1,
bool e1 = first1 == last1;
bool e2 = first2 == last2;
if (e1 == e2) {
- return static_cast<int>(relation1) - static_cast<int>(relation2);
+ return (static_cast<int>(relation1) - static_cast<int>(relation2)) <=> 0;
}
if (e2) {
- return relation2 == lexicographical_relation::after_all_prefixed ? -1 : 1;
+ return relation2 == lexicographical_relation::after_all_prefixed ? std::strong_ordering::less : std::strong_ordering::greater;
} else {
- return relation1 == lexicographical_relation::after_all_prefixed ? 1 : -1;
+ return relation1 == lexicographical_relation::after_all_prefixed ? std::strong_ordering::greater : std::strong_ordering::less;
}
}

@@ -186,18 +182,21 @@ int lexicographical_tri_compare(InputIt1 first1, InputIt1 last1,
// type as first argument.
//
template <typename TypesIterator, typename InputIt1, typename InputIt2, typename Compare>
-int prefix_equality_tri_compare(TypesIterator types, InputIt1 first1, InputIt1 last1,
+requires requires (TypesIterator ti, InputIt1 i1, InputIt2 i2, Compare c) {
+ { c(*ti, *i1, *i2) } -> std::same_as<std::strong_ordering>;
+}
+std::strong_ordering prefix_equality_tri_compare(TypesIterator types, InputIt1 first1, InputIt1 last1,
InputIt2 first2, InputIt2 last2, Compare comp) {
while (first1 != last1 && first2 != last2) {
auto c = comp(*types, *first1, *first2);
- if (c) {
+ if (c != 0) {
return c;
}
++first1;
++first2;
++types;
}
- return 0;
+ return std::strong_ordering::equal;
}

// Returns true iff the second sequence is a prefix of the first sequence
@@ -500,8 +499,8 @@ public:
size_t hash(managed_bytes_view v) const;
bool equal(bytes_view v1, bytes_view v2) const;
bool equal(managed_bytes_view v1, managed_bytes_view v2) const;
- int32_t compare(bytes_view v1, bytes_view v2) const;
- int32_t compare(managed_bytes_view v1, managed_bytes_view v2) const;
+ std::strong_ordering compare(bytes_view v1, bytes_view v2) const;
+ std::strong_ordering compare(managed_bytes_view v1, managed_bytes_view v2) const;

private:
// Explicitly instantiated in .cc
@@ -729,15 +728,15 @@ bool less_compare(data_type t, bytes_view e1, bytes_view e2) {
}

static inline
-int tri_compare(data_type t, managed_bytes_view e1, managed_bytes_view e2) {
+std::strong_ordering tri_compare(data_type t, managed_bytes_view e1, managed_bytes_view e2) {
return t->compare(e1, e2);
}

inline
-int
+std::strong_ordering
tri_compare_opt(data_type t, managed_bytes_view_opt v1, managed_bytes_view_opt v2) {
if (!v1 || !v2) {
- return int(bool(v1)) - int(bool(v2));
+ return int(bool(v1)) - int(bool(v2)) <=> 0;
} else {
return tri_compare(std::move(t), *v1, *v2);
}
@@ -869,11 +868,11 @@ class serialized_tri_compare {
data_type _type;
public:
serialized_tri_compare(data_type type) : _type(type) {}
- int operator()(const bytes_view& v1, const bytes_view& v2) const {
- return _type->compare(v1, v2);
+ std::strong_ordering operator()(const bytes_view& v1, const bytes_view& v2) const {
+ return _type->compare(v1, v2) <=> 0;
}
- int operator()(const managed_bytes_view& v1, const managed_bytes_view& v2) const {
- return _type->compare(v1, v2);
+ std::strong_ordering operator()(const managed_bytes_view& v1, const managed_bytes_view& v2) const {
+ return _type->compare(v1, v2) <=> 0;
}
};

diff --git a/types/collection.hh b/types/collection.hh
--- a/types/collection.hh
+++ b/types/collection.hh
@@ -105,7 +105,7 @@ public:
//
// This function is used to compare receiver with a literal or parameter marker during condition
// evaluation.
- int32_t compare_with_map(const map_type_impl& map_type, bytes_view list, bytes_view map) const;
+ std::strong_ordering compare_with_map(const map_type_impl& map_type, bytes_view list, bytes_view map) const;
// A list or set value can be represented as a vector<pair<timeuuid, data_value>> or
// vector<pair<data_value, empty>> respectively. Serialize this representation
// as a vector of values, not as a vector of pairs.
diff --git a/types/map.hh b/types/map.hh
--- a/types/map.hh
+++ b/types/map.hh
@@ -52,7 +52,7 @@ public:
virtual data_type freeze() const override;
virtual bool is_compatible_with_frozen(const collection_type_impl& previous) const override;
virtual bool is_value_compatible_with_frozen(const collection_type_impl& previous) const override;
- static int32_t compare_maps(data_type keys_comparator, data_type values_comparator,
+ static std::strong_ordering compare_maps(data_type keys_comparator, data_type values_comparator,
managed_bytes_view o1, managed_bytes_view o2);
using abstract_type::deserialize;
using collection_type_impl::deserialize;
diff --git a/utils/UUID.hh b/utils/UUID.hh
--- a/utils/UUID.hh
+++ b/utils/UUID.hh
@@ -27,6 +27,7 @@
#include <cassert>
#include <array>
#include <iosfwd>
+#include <compare>

#include <seastar/core/sstring.hh>
#include <seastar/core/print.hh>
@@ -141,8 +142,8 @@ public:

UUID make_random_uuid();

-inline int uint64_t_tri_compare(uint64_t a, uint64_t b) {
- return a < b ? -1 : a > b;
+inline std::strong_ordering uint64_t_tri_compare(uint64_t a, uint64_t b) {
+ return a <=> b;
}

// Read 8 most significant bytes of timeuuid from serialized bytes
@@ -178,11 +179,11 @@ inline uint64_t uuid_read_lsb(const int8_t *b) {
// To avoid breaking ordering in existing sstables, Scylla preserves
// Cassandra compare order.
//
-inline int timeuuid_tri_compare(bytes_view o1, bytes_view o2) {
+inline std::strong_ordering timeuuid_tri_compare(bytes_view o1, bytes_view o2) {
auto timeuuid_read_lsb = [](bytes_view o) -> uint64_t {
return uuid_read_lsb(o.begin()) ^ 0x8080808080808080;
};
- int res = uint64_t_tri_compare(timeuuid_read_msb(o1.begin()), timeuuid_read_msb(o2.begin()));
+ auto res = uint64_t_tri_compare(timeuuid_read_msb(o1.begin()), timeuuid_read_msb(o2.begin()));
if (res == 0) {
res = uint64_t_tri_compare(timeuuid_read_lsb(o1), timeuuid_read_lsb(o2));
}
@@ -196,8 +197,8 @@ inline int timeuuid_tri_compare(bytes_view o1, bytes_view o2) {
// which is both faster and monotonic, so should be preferred
// to @timeuuid_tri_compare() used for all new features.
//
-inline int uuid_tri_compare_timeuuid(bytes_view o1, bytes_view o2) {
- int res = uint64_t_tri_compare(timeuuid_read_msb(o1.begin()), timeuuid_read_msb(o2.begin()));
+inline std::strong_ordering uuid_tri_compare_timeuuid(bytes_view o1, bytes_view o2) {
+ auto res = uint64_t_tri_compare(timeuuid_read_msb(o1.begin()), timeuuid_read_msb(o2.begin()));
if (res == 0) {
res = uint64_t_tri_compare(uuid_read_lsb(o1.begin()), uuid_read_lsb(o2.begin()));
}
diff --git a/utils/big_decimal.cc b/utils/big_decimal.cc
--- a/utils/big_decimal.cc
+++ b/utils/big_decimal.cc
@@ -148,13 +148,13 @@ sstring big_decimal::to_string() const
return str;
}

-int big_decimal::compare(const big_decimal& other) const
+std::strong_ordering big_decimal::compare(const big_decimal& other) const
{
auto max_scale = std::max(_scale, other._scale);
boost::multiprecision::cpp_int rescale(10);
boost::multiprecision::cpp_int x = _unscaled_value * boost::multiprecision::pow(rescale, max_scale - _scale);
boost::multiprecision::cpp_int y = other._unscaled_value * boost::multiprecision::pow(rescale, max_scale - other._scale);
- return x == y ? 0 : x < y ? -1 : 1;
+ return x == y ? std::strong_ordering::equal : x < y ? std::strong_ordering::less : std::strong_ordering::greater;
}

big_decimal& big_decimal::operator+=(const big_decimal& other)
diff --git a/utils/big_decimal.hh b/utils/big_decimal.hh
--- a/utils/big_decimal.hh
+++ b/utils/big_decimal.hh
@@ -24,6 +24,7 @@
#include "multiprecision_int.hh"
#include <boost/multiprecision/cpp_int.hpp>
#include <ostream>
+#include <compare>

#include "bytes.hh"

@@ -48,7 +49,7 @@ public:

sstring to_string() const;

- int compare(const big_decimal& other) const;
+ std::strong_ordering compare(const big_decimal& other) const;

big_decimal& operator+=(const big_decimal& other);
big_decimal& operator-=(const big_decimal& other);
diff --git a/utils/fragment_range.hh b/utils/fragment_range.hh
--- a/utils/fragment_range.hh
+++ b/utils/fragment_range.hh
@@ -22,6 +22,7 @@
#pragma once

#include <concepts>
+#include <compare>
#include <boost/range/algorithm/copy.hpp>
#include <seastar/net/byteorder.hh>
#include <seastar/core/print.hh>
@@ -286,18 +287,18 @@ void skip_empty_fragments(View& v) {
}

template<FragmentedView V1, FragmentedView V2>
-int compare_unsigned(V1 v1, V2 v2) {
+std::strong_ordering compare_unsigned(V1 v1, V2 v2) {
while (!v1.empty() && !v2.empty()) {
size_t n = std::min(v1.current_fragment().size(), v2.current_fragment().size());
if (int d = memcmp(v1.current_fragment().data(), v2.current_fragment().data(), n)) {
- return d;
+ return d <=> 0;
}
v1.remove_prefix(n);
v2.remove_prefix(n);
skip_empty_fragments(v1);
skip_empty_fragments(v2);
}
- return v1.size_bytes() - v2.size_bytes();
+ return v1.size_bytes() <=> v2.size_bytes();
}

template<FragmentedView V1, FragmentedView V2>
diff --git a/utils/int_range.hh b/utils/int_range.hh
--- a/utils/int_range.hh
+++ b/utils/int_range.hh
@@ -43,7 +43,7 @@ unsigned cardinality(const std::optional<int_range>& ropt) {
inline
std::optional<int_range> intersection(const int_range& a, const int_range& b) {
auto int_tri_cmp = [] (int x, int y) {
- return x < y ? -1 : (x > y ? 1 : 0);
+ return x <=> y;
};
return a.intersection(b, int_tri_cmp);
}

Commit Bot

unread,
Jul 29, 2021, 9:40:25 PMJul 29
to scylla...@googlegroups.com, Tomasz Grabiec
From: Tomasz Grabiec <tgra...@scylladb.com>
Committer: Tomasz Grabiec <tgra...@scylladb.com>
Branch: master
Reply all
Reply to author
Forward
0 new messages