Merge PR ceph#62200 into main

* refs/pull/62200/head:
	common/include: Add map type election and fmt::format to interval_map

Reviewed-by: Patrick Donnelly <[email protected]>

Author: batrick

src/common/interval_map.h

@@ -18,7 +18,6 @@
 #include "include/interval_set.h"
 #include <initializer_list>
 
-template <typename K, typename V, typename S>
 /**
  * interval_map
  *
@@ -31,13 +30,14 @@ template <typename K, typename V, typename S>
  * commutativity, which doesn't work if we want more recent insertions
  * to overwrite previous ones.
  */
+template <typename K, typename V, typename S, template<typename, typename, typename ...> class C = std::map>
 class interval_map {
   S s;
-  using map = std::map<K, std::pair<K, V> >;
-  using mapiter = typename std::map<K, std::pair<K, V> >::iterator;
-  using cmapiter = typename std::map<K, std::pair<K, V> >::const_iterator;
-  map m;
-  std::pair<mapiter, mapiter> get_range(K off, K len) {
+  using Map = C<K, std::pair<K, V> >;
+  using Mapiter = typename Map::iterator;
+  using Cmapiter = typename Map::const_iterator;
+  Map m;
+  std::pair<Mapiter, Mapiter> get_range(K off, K len) {
     // fst is first iterator with end after off (may be end)
     auto fst = m.upper_bound(off);
     if (fst != m.begin())
@@ -49,15 +49,15 @@ class interval_map {
     auto lst = m.lower_bound(off + len);
     return std::make_pair(fst, lst);
   }
-  std::pair<cmapiter, cmapiter> get_range(K off, K len) const {
+  std::pair<Cmapiter, Cmapiter> get_range(K off, K len) const {
     // fst is first iterator with end after off (may be end)
     auto fst = get_range_fst(off);
 
     // lst is first iterator with start after off + len (may be end)
     auto lst = m.lower_bound(off + len);
     return std::make_pair(fst, lst);
   }
-  cmapiter get_range_fst(K off) const {
+  Cmapiter get_range_fst(K off) const {
     // fst is first iterator with end after off (may be end)
     auto fst = m.upper_bound(off);
     if (fst != m.begin())
@@ -67,7 +67,7 @@ class interval_map {
 
     return fst;
   }
-  void try_merge(mapiter niter) {
+  void try_merge(Mapiter niter) {
     if (niter != m.begin()) {
       auto prev = niter;
       prev--;
@@ -109,7 +109,7 @@ class interval_map {
   }
 public:
   interval_map() = default;
-  interval_map(std::initializer_list<typename map::value_type> l) {
+  interval_map(std::initializer_list<typename Map::value_type> l) {
     for (auto& v : l) {
       insert(v.first, v.second.first, v.second.second);
     }
@@ -202,17 +202,23 @@ class interval_map {
   bool empty() const {
     return m.empty();
   }
-  interval_set<K> get_interval_set() const {
-    interval_set<K> ret;
+  interval_set<K, C> get_interval_set() const {
+    interval_set<K, C> ret;
     for (auto &&i: *this) {
       ret.insert(i.get_off(), i.get_len());
     }
     return ret;
   }
+  template<bool strict = true>
+  void to_interval_set(interval_set<K, C, strict> &set) const {
+    for (auto &&i: *this) {
+      set.insert(i.get_off(), i.get_len());
+    }
+  }
   class const_iterator {
-    cmapiter it;
-    const_iterator(cmapiter &&it) : it(std::move(it)) {}
-    const_iterator(const cmapiter &it) : it(it) {}
+    Cmapiter it;
+    const_iterator(Cmapiter &&it) : it(std::move(it)) {}
+    const_iterator(const Cmapiter &it) : it(it) {}
 
     friend class interval_map;
   public:
@@ -302,25 +308,41 @@ class interval_map {
     return m == rhs.m;
   }
 
-  std::ostream &print(std::ostream &out) const {
+  void print(std::ostream &os) const {
     bool first = true;
-    out << "{";
+    os << "{";
     for (auto &&i: *this) {
       if (first) {
 	first = false;
       } else {
-	out << ",";
+	os << ",";
       }
-      out << i.get_off() << "~" << i.get_len() << "("
+      os << i.get_off() << "~" << i.get_len() << "("
 	  << s.length(i.get_val()) << ")";
     }
-    return out << "}";
+    os << "}";
+  }
+
+  std::string fmt_print() const
+  requires has_formatter<K> {
+    std::string str = "{";
+    bool first = true;
+    for (auto &&i: *this) {
+      if (first) {
+        first = false;
+      } else {
+        str += ",";
+      }
+      str += fmt::format("{}~{}({})", i.get_off(), i.get_len(),
+        s.length(i.get_val()));
+    }
+    str += "}";
+    return str;
   }
 };
 
-template <typename K, typename V, typename S>
-std::ostream &operator<<(std::ostream &out, const interval_map<K, V, S> &m) {
-  return m.print(out);
-}
+// make sure fmt::range would not try (and fail) to treat interval_map as a range
+template <typename K, typename V, typename S, template<typename, typename, typename ...> class C>
+struct fmt::is_range<interval_map<K, V, S, C>, char> : std::false_type {};
 
 #endif

src/include/interval_set.h

@@ -19,6 +19,9 @@
 #include <iterator>
 #include <map>
 #include <ostream>
+#include <fmt/ranges.h>
+#include "common/fmt_common.h"
+#include "common/dout.h"
 
 #include "encoding.h"
 
@@ -246,6 +249,35 @@ class interval_set {
     return const_iterator(m.end());
   }
 
+  void print(std::ostream& os) const {
+    os << "[";
+    bool first = true;
+    for (const auto& [start, len] : *this) {
+      if (!first) {
+        os << ",";
+      }
+      os << start << "~" << len;
+      first = false;
+    }
+    os << "]";
+  }
+
+  std::string fmt_print() const
+  requires has_formatter<T> {
+    std::string s = "[";
+    bool first = true;
+    for (const auto& [start, len] : *this) {
+      if (!first) {
+        s += ",";
+      } else {
+        first = false;
+      }
+      s += fmt::format("{}~{}", start, len);
+    }
+    s += "]";
+    return s;
+  }
+
   // helpers
  private:
   auto find_inc(T start) const {
@@ -963,19 +995,8 @@ struct denc_traits<interval_set<T, C>> {
   }
 };
 
-
-template<typename T, template<typename, typename, typename ...> class C>
-inline std::ostream& operator<<(std::ostream& out, const interval_set<T,C> &s) {
-  out << "[";
-  bool first = true;
-  for (const auto& [start, len] : s) {
-    if (!first) out << ",";
-    out << start << "~" << len;
-    first = false;
-  }
-  out << "]";
-  return out;
-}
-
+// make sure fmt::range would not try (and fail) to treat interval_set as a range
+template<typename T, template<typename, typename, typename ...> class C, bool strict>
+struct fmt::is_range<interval_set<T, C, strict>, char> : std::false_type {};
 
 #endif

src/test/common/test_interval_map.cc

@@ -27,11 +27,15 @@ class IntervalMapTest : public ::testing::Test {
   using TestType = T;
 };
 
-template <typename _key>
+template <typename _key, typename _can_merge, template<typename, typename, typename ...> class _map = std::map>
 struct bufferlist_test_type {
   using key = _key;
   using value = bufferlist;
 
+  static constexpr bool can_merge()  {
+    return _can_merge::value;
+  }
+
   struct make_splitter {
     template <typename merge_t>
     struct apply {
@@ -57,6 +61,9 @@ struct bufferlist_test_type {
     };
   };
 
+  using splitter = boost::mpl::apply<make_splitter, _can_merge>;
+  using imap = interval_map<key, value, splitter, _map>;
+
   struct generate_random {
     bufferlist operator()(key len) {
       bufferlist bl;
@@ -70,21 +77,27 @@ struct bufferlist_test_type {
   };
 };
 
-using IntervalMapTypes = ::testing::Types< bufferlist_test_type<uint64_t> >;
+using IntervalMapTypes = ::testing::Types<
+  bufferlist_test_type<uint64_t, std::false_type>,
+  bufferlist_test_type<uint64_t, std::true_type>,
+  bufferlist_test_type<uint64_t, std::false_type, boost::container::flat_map>,
+  bufferlist_test_type<uint64_t, std::true_type, boost::container::flat_map>
+>;
 
 TYPED_TEST_SUITE(IntervalMapTest, IntervalMapTypes);
 
+// Someone with more C++ foo can probably figure out a better way of doing this.
+// However, for now, we skip some tests if using the wrong merge.
 #define USING(_can_merge)					 \
   using TT = typename TestFixture::TestType;                     \
   using key = typename TT::key; (void)key(0);	                 \
   using val = typename TT::value; (void)val(0);			 \
-  using splitter = typename boost::mpl::apply<                   \
-    typename TT::make_splitter,                                  \
-    _can_merge>;                                                 \
-  using imap = interval_map<key, val, splitter>; (void)imap();	 \
+  using splitter = TT::splitter;                                 \
+  using imap = TT::imap; (void)imap();	                         \
   typename TT::generate_random gen;                              \
   val v(gen(5));	                                         \
-  splitter split; (void)split.split(0, 0, v);
+  splitter split; (void)split.split(0, 0, v);                    \
+  { _can_merge cm; if(TT::can_merge() != cm()) return; }
 
 #define USING_NO_MERGE USING(std::false_type)
 #define USING_WITH_MERGE USING(std::true_type)
@@ -371,4 +384,24 @@ TYPED_TEST(IntervalMapTest, get_start_end_off)
   m.insert(20,5, gen(5));
   ASSERT_EQ(5, m.get_start_off());
   ASSERT_EQ(25, m.get_end_off());
-}
+}
+
+TYPED_TEST(IntervalMapTest, print) {
+  USING_NO_MERGE;
+  imap m;
+
+  {
+    std::ostringstream out;
+    m.insert(0, 5, gen(5));
+    out << m;
+    ASSERT_EQ("{0~5(5)}", fmt::format("{}", m));
+    EXPECT_EQ("{0~5(5)}", out.str() );
+  }
+  {
+    std::ostringstream out;
+    m.insert(10, 5, gen(5));
+    out << m;
+    ASSERT_EQ("{0~5(5),10~5(5)}", fmt::format("{}", m));
+    EXPECT_EQ("{0~5(5),10~5(5)}", out.str() );
+  }
+}

src/test/common/test_interval_set.cc

@@ -1859,4 +1859,34 @@ TYPED_TEST(IntervalSetTest, subtract) {
 
     ASSERT_STRICT_DEATH(iset1.subtract(iset2), ASSERT_EQ(iset1, iset3));
   }
+
+  // Subtract identical
+  {
+    ISet iset1, iset2;
+    iset1.union_insert(0, 5);
+    iset2.union_insert(0, 5);
+
+    iset1.subtract(iset2);
+    ASSERT_TRUE(iset1.empty());
+  }
+}
+
+TYPED_TEST(IntervalSetTest, print) {
+  typedef typename TestFixture::ISet ISet;
+
+  ISet iset;
+  {
+    std::ostringstream out;
+    iset.insert(0, 5);
+    out << iset;
+    ASSERT_EQ("[0~5]", fmt::format("{}", iset));
+    EXPECT_EQ("[0~5]", out.str() );
+  }
+  {
+    std::ostringstream out;
+    iset.insert(10, 5);
+    out << iset;
+    ASSERT_EQ("[0~5,10~5]", fmt::format("{}", iset));
+    EXPECT_EQ("[0~5,10~5]", out.str() );
+  }
 }