tombstone: use comparison operator instead of ad-hoc compare() function 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;

utils/with_relational_operators.hh

@@ -1,65 +0,0 @@
-/*
- * Copyright (C) 2017 ScyllaDB
- */
-
-/*
- * This file is part of Scylla.
- *
- * Scylla is free software: you can redistribute it and/or modify
- * it under the terms of the GNU Affero General Public License as published by
- * the Free Software Foundation, either version 3 of the License, or
- * (at your option) any later version.
- *
- * Scylla is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with Scylla.  If not, see <http://www.gnu.org/licenses/>.
- */
-
-#pragma once
-
-#include <type_traits>
-#include <concepts>
-
-template<typename T>
-concept HasTriCompare =
-    requires(const T& t) {
-        { t.compare(t) } -> std::same_as<int>;
-    } && std::is_same<std::result_of_t<decltype(&T::compare)(T, T)>, int>::value; //FIXME: #1449
-
-template<typename T>
-class with_relational_operators {
-private:
-    template<typename U>
-    requires HasTriCompare<U>
-    int do_compare(const U& t) const {
-        return static_cast<const U*>(this)->compare(t);
-    }
-public:
-    bool operator<(const T& t) const {
-        return do_compare(t) < 0;
-    }
-
-    bool operator<=(const T& t) const {
-        return do_compare(t) <= 0;
-    }
-
-    bool operator>(const T& t) const {
-        return do_compare(t) > 0;
-    }
-
-    bool operator>=(const T& t) const {
-        return do_compare(t) >= 0;
-    }
-
-    bool operator==(const T& t) const {
-        return do_compare(t) == 0;
-    }
-
-    bool operator!=(const T& t) const {
-        return do_compare(t) != 0;
-    }
-};