tombstone: use comparison operator instead of ad-hoc compare() functi…

…on and with_relational_operators

The comparison operator (<=>) default implementation happens to exactly
match tombstone::compare(), so use the compiler-generated defaults. Also
default operator== and operator!= (these are not brought in by operator<=>).
These become slightly faster as they perform just an equality comparison,
not three-way compare.

shadowable_tombstone and row_tombstone depend on tombstone::compare(),
so convert them too in a similar way.

with_relational_operations.hh becomes unused, so delete it.

Tests: unit (dev)
Message-Id: <20200602055626.2874801-1-avi@scylladb.com>

mutation_partition.hh

@@ -45,7 +45,6 @@
 #include "range_tombstone_list.hh"
 #include "clustering_key_filter.hh"
 #include "intrusive_set_external_comparator.hh"
-#include "utils/with_relational_operators.hh"
 #include "utils/preempt.hh"
 #include "utils/managed_ref.hh"
 
@@ -762,7 +761,7 @@ struct appending_hash<row_marker> {
 
 class clustering_row;
 
-class shadowable_tombstone : public with_relational_operators<shadowable_tombstone> {
+class shadowable_tombstone {
     tombstone _tomb;
 public:
 
@@ -774,10 +773,13 @@ public:
             : _tomb(std::move(tomb)) {
     }
 
-    int compare(const shadowable_tombstone& t) const {
-        return _tomb.compare(t._tomb);
+    std::strong_ordering operator<=>(const shadowable_tombstone& t) const {
+        return _tomb <=> t._tomb;
     }
 
+    bool operator==(const shadowable_tombstone&) const = default;
+    bool operator!=(const shadowable_tombstone&) const = default;
+
     explicit operator bool() const {
         return bool(_tomb);
     }
@@ -845,7 +847,7 @@ The rules for row_tombstones are as follows:
   - The shadowable tombstone can be erased or compacted away by a newer
     row marker.
 */
-class row_tombstone : public with_relational_operators<row_tombstone> {
+class row_tombstone {
     tombstone _regular;
     shadowable_tombstone _shadowable; // _shadowable is always >= _regular
 public:
@@ -860,8 +862,14 @@ public:
 
     row_tombstone() = default;
 
-    int compare(const row_tombstone& t) const {
-        return _shadowable.compare(t._shadowable);
+    std::strong_ordering operator<=>(const row_tombstone& t) const {
+        return _shadowable <=> t._shadowable;
+    }
+    bool operator==(const row_tombstone& t) const {
+        return _shadowable == t._shadowable;
+    }
+    bool operator!=(const row_tombstone& t) const {
+        return _shadowable != t._shadowable;
     }
 
     explicit operator bool() const {

range_tombstone_list.cc

@@ -114,7 +114,7 @@ void range_tombstone_list::insert_from(const schema& s,
             return;
         }
 
-        auto c = tomb.compare(it->tomb);
+        auto c = tomb <=> it->tomb;
         if (c == 0) {
             // same timestamp, overlapping or adjacent, so merge.
             if (less(it->start_bound(), start_bound)) {

sstables/mp_row_consumer.hh

@@ -903,7 +903,7 @@ class mp_row_consumer_m : public consumer_m {
                     format("Closing range tombstone that wasn't opened: clustering {}, kind {}, tombstone {}",
                            ck, k, t));
         }
-        if (_opened_range_tombstone->tomb.compare(t) != 0) {
+        if (_opened_range_tombstone->tomb != t) {
             throw sstables::malformed_sstable_exception(
                     format("Range tombstone with ck {} and two different tombstones at ends: {}, {}",
                            ck, _opened_range_tombstone->tomb, t));

test/lib/flat_mutation_reader_assertions.hh

@@ -41,7 +41,7 @@ private:
     static bool are_tombstones_mergeable(const schema& s, const range_tombstone& a, const range_tombstone& b) {
         const auto range_a = position_range(position_in_partition(a.position()), position_in_partition(a.end_position()));
         const auto tri_cmp = position_in_partition::tri_compare(s);
-        return a.tomb.compare(b.tomb) == 0 && (range_a.overlaps(s, b.position(), b.end_position()) ||
+        return (a.tomb <=> b.tomb) == 0 && (range_a.overlaps(s, b.position(), b.end_position()) ||
                 tri_cmp(a.end_position(), b.position()) == 0 ||
                 tri_cmp(b.end_position(), a.position()) == 0);
     }

tombstone.hh

@@ -22,17 +22,17 @@
 #pragma once
 
 #include <functional>
+#include <compare>
 
 #include "timestamp.hh"
 #include "gc_clock.hh"
 #include "hashing.hh"
-#include "utils/with_relational_operators.hh"
 
 /**
  * Represents deletion operation. Can be commuted with other tombstones via apply() method.
  * Can be empty.
  */
-struct tombstone final : public with_relational_operators<tombstone> {
+struct tombstone final {
     api::timestamp_type timestamp;
     gc_clock::time_point deletion_time;
 
@@ -45,19 +45,9 @@ struct tombstone final : public with_relational_operators<tombstone> {
         : tombstone(api::missing_timestamp, {})
     { }
 
-    int compare(const tombstone& t) const {
-        if (timestamp < t.timestamp) {
-            return -1;
-        } else if (timestamp > t.timestamp) {
-            return 1;
-        } else if (deletion_time < t.deletion_time) {
-            return -1;
-        } else if (deletion_time > t.deletion_time) {
-            return 1;
-        } else {
-            return 0;
-        }
-    }
+    std::strong_ordering operator<=>(const tombstone& t) const = default;
+    bool operator==(const tombstone&) const = default;
+    bool operator!=(const tombstone&) const = default;
 
     explicit operator bool() const {
         return timestamp != api::missing_timestamp;