Extends random.h: numeric types relaxed to compatible types (with

appropriate distributions), fixes 2-parameter calling convention;
OOP interface now publishes engine and device types and allows
appropriate seed types.

These changes are the result of direct experience porting Ceph
code away from rand(), and a prerequisite for that further porting.

Signed-off-by: Jesse Williamson <jwilliamson@suse.de>

diff --git a/src/include/random.h b/src/include/random.h
index b985606bdba7bcc40166147414083f1d09fa118a..63593e8e601a7f0bc272c5ecd9c20b8df06e467b 100644 (file)
--- a/src/include/random.h
+++ b/src/include/random.h
@@ -17,23 +17,72 @@
 
 #include <mutex>
 #include <random>
+#include <optional>
 #include <type_traits>
 
-#include "boost/optional.hpp"
-
 // Basic random number facility, adapted from N3551:
-namespace ceph {
-namespace util {
+namespace ceph::util {
+
+inline namespace version_1_0_2 {
+
+namespace detail {
 
-inline namespace version_1_0 {
+template <typename T0, typename T1>
+using larger_of = typename std::conditional<
+                    sizeof(T0) >= sizeof(T1), 
+                    T0, T1>
+                  ::type;
+
+// avoid mixing floating point and integers:
+template <typename NumberT0, typename NumberT1>
+using has_compatible_numeric_types =
+            std::disjunction<
+                std::conjunction<
+                    std::is_floating_point<NumberT0>, std::is_floating_point<NumberT1>
+                >,
+                std::conjunction<
+                    std::is_integral<NumberT0>, std::is_integral<NumberT1>
+                >
+            >;
+
+
+// Select the larger of type compatible numeric types:
+template <typename NumberT0, typename NumberT1>
+using select_number_t = std::enable_if_t<detail::has_compatible_numeric_types<NumberT0, NumberT1>::value,
+                                         detail::larger_of<NumberT0, NumberT1>>;
+
+} // namespace detail
+
+namespace detail {
+
+// Choose default distribution for appropriate types:
+template <typename NumberT, 
+          bool IsIntegral>
+struct select_distribution
+{
+ using type = std::uniform_int_distribution<NumberT>;
+};
+
+template <typename NumberT>
+struct select_distribution<NumberT, false>
+{
+ using type = std::uniform_real_distribution<NumberT>;
+};
+
+template <typename NumberT>
+using default_distribution = typename
+    select_distribution<NumberT, std::is_integral<NumberT>::value>::type;
+
+} // namespace detail
 
 namespace detail {
 
 template <typename EngineT>
 EngineT& engine();
 
-template <typename MutexT, typename EngineT>
-void randomize_rng(const int seed, MutexT& m, EngineT& e)
+template <typename MutexT, typename EngineT, 
+          typename SeedT = typename EngineT::result_type>
+void randomize_rng(const SeedT seed, MutexT& m, EngineT& e)
 {
   std::lock_guard<MutexT> lg(m);
   e.seed(seed);
@@ -48,8 +97,9 @@ void randomize_rng(MutexT& m, EngineT& e)
   e.seed(rd());
 }
 
-template <typename EngineT = std::default_random_engine>
-void randomize_rng(const int n)
+template <typename EngineT = std::default_random_engine,
+          typename SeedT = typename EngineT::result_type>
+void randomize_rng(const SeedT n)
 {
   detail::engine<EngineT>().seed(n);
 }
@@ -64,7 +114,7 @@ void randomize_rng()
 template <typename EngineT>
 EngineT& engine()
 {
-  thread_local boost::optional<EngineT> rng_engine;
+  thread_local std::optional<EngineT> rng_engine;
 
   if (!rng_engine) {
     rng_engine.emplace(EngineT());
@@ -79,12 +129,12 @@ EngineT& engine()
 namespace detail {
 
 template <typename NumberT,
-          typename DistributionT,
+          typename DistributionT = detail::default_distribution<NumberT>,
           typename EngineT>
 NumberT generate_random_number(const NumberT min, const NumberT max,
                                EngineT& e)
 {
-  thread_local DistributionT d { min, max };
+  DistributionT d { min, max };
 
   using param_type = typename DistributionT::param_type;
   return d(e, param_type { min, max });
@@ -92,12 +142,12 @@ NumberT generate_random_number(const NumberT min, const NumberT max,
 
 template <typename NumberT,
           typename MutexT,
-          typename DistributionT,
+          typename DistributionT = detail::default_distribution<NumberT>,
           typename EngineT>
 NumberT generate_random_number(const NumberT min, const NumberT max,
                                MutexT& m, EngineT& e)
 {
-  thread_local DistributionT d { min, max };
+  DistributionT d { min, max };
  
   using param_type = typename DistributionT::param_type;
  
@@ -106,7 +156,7 @@ NumberT generate_random_number(const NumberT min, const NumberT max,
 }
 
 template <typename NumberT,
-          typename DistributionT,
+          typename DistributionT = detail::default_distribution<NumberT>,
           typename EngineT>
 NumberT generate_random_number(const NumberT min, const NumberT max)
 {
@@ -114,14 +164,20 @@ NumberT generate_random_number(const NumberT min, const NumberT max)
           (min, max, detail::engine<EngineT>());
 }
 
-template <typename MutexT, typename EngineT,
-          int min = 0,
-          int max = std::numeric_limits<int>::max(), 
-          typename DistributionT = std::uniform_int_distribution<int>>
-int generate_random_number(MutexT& m, EngineT& e)
+template <typename MutexT, 
+          typename EngineT,
+          typename NumberT = int,
+          typename DistributionT = detail::default_distribution<NumberT>>
+NumberT generate_random_number(MutexT& m, EngineT& e)
 {
-  return detail::generate_random_number<int, MutexT, DistributionT, EngineT>
-          (min, max, m, e);
+  return detail::generate_random_number<NumberT, MutexT, DistributionT, EngineT>
+          (0, std::numeric_limits<NumberT>::max(), m, e);
+}
+
+template <typename NumberT, typename MutexT, typename EngineT>
+NumberT generate_random_number(const NumberT max, MutexT& m, EngineT& e)
+{
+  return generate_random_number<NumberT>(0, max, m, e);
 }
 
 } // namespace detail
@@ -132,105 +188,43 @@ void randomize_rng()
   detail::randomize_rng<EngineT>();
 }
 
-template <typename IntegerT = int,
-          typename DistributionT = std::uniform_int_distribution<IntegerT>,
+template <typename NumberT = int,
+          typename DistributionT = detail::default_distribution<NumberT>,
           typename EngineT = std::default_random_engine>
-IntegerT generate_random_number()
-{
-  using limits = std::numeric_limits<IntegerT>;
-  return detail::generate_random_number<IntegerT, DistributionT, EngineT>
-          (limits::min(), limits::max());
-}
-
-template <typename IntegerT>
-IntegerT generate_random_number(const IntegerT min, const IntegerT max,
-                                std::enable_if_t<std::is_integral<IntegerT>::value>* = nullptr)
-{
-  return detail::generate_random_number<IntegerT,
-                                        std::uniform_int_distribution<IntegerT>,
-                                        std::default_random_engine>
-                                       (min, max); 
-}
-
-namespace detail {
-
-template <typename IntegerT, typename MutexT, typename EngineT>
-int generate_random_number(const IntegerT min, const IntegerT max,
-                           MutexT& m, EngineT& e,
-                           std::enable_if_t<std::is_integral<IntegerT>::value>* = nullptr)
-{
-  return detail::generate_random_number<IntegerT, MutexT,
-                                        std::uniform_int_distribution<IntegerT>,
-                                        EngineT>
-                                       (min, max, m, e);
-}
-
-template <typename IntegerT, typename MutexT, typename EngineT>
-int generate_random_number(const IntegerT max,
-                           MutexT& m, EngineT& e,
-                           std::enable_if_t<std::is_integral<IntegerT>::value>* = nullptr)
+NumberT generate_random_number()
 {
-  constexpr IntegerT zero = 0;
-  return generate_random_number(zero, max, m, e);
-}
-
-} // namespace detail
-
-template <typename IntegerT>
-int generate_random_number(const IntegerT max,
-                           std::enable_if_t<std::is_integral<IntegerT>::value>* = nullptr)
-{
-  constexpr IntegerT zero = 0;   
-  return generate_random_number(zero, max);
+  return detail::generate_random_number<NumberT, DistributionT, EngineT>
+          (0, std::numeric_limits<NumberT>::max());
 }
 
-template <typename RealT>
-RealT generate_random_number(const RealT min, const RealT max, 
-                             std::enable_if_t<std::is_floating_point<RealT>::value>* = nullptr)
+template <typename NumberT0, typename NumberT1,
+          typename NumberT = detail::select_number_t<NumberT0, NumberT1>
+         >
+NumberT generate_random_number(const NumberT0 min, const NumberT1 max)
 {
-  return detail::generate_random_number<RealT,
-                                        std::uniform_real_distribution<RealT>, 
+  return detail::generate_random_number<NumberT,
+                                        detail::default_distribution<NumberT>,
                                         std::default_random_engine>
-                                       (min, max);
-} 
-
-namespace detail {
-
-template <typename RealT, typename MutexT>
-RealT generate_random_number(const RealT max, MutexT& m,
-                             std::enable_if_t<std::is_floating_point<RealT>::value>* = nullptr)
-{
-  constexpr RealT zero = 0.0;
-  return generate_random_number(zero, max, m);
-}
-
-template <typename RealT, typename MutexT, typename EngineT>
-RealT generate_random_number(const RealT min, const RealT max, MutexT& m, EngineT& e,
-                             std::enable_if_t<std::is_floating_point<RealT>::value>* = nullptr)
-{
-  return detail::generate_random_number<RealT, MutexT, 
-                                        std::uniform_real_distribution<RealT>,
-                                        EngineT>
-                                       (min, max, m, e);
+                                       (static_cast<NumberT>(min), static_cast<NumberT>(max)); 
 }
 
-
-template <typename RealT, typename MutexT, typename EngineT>
-RealT generate_random_number(const RealT max, MutexT& m, EngineT& e,
-                             std::enable_if_t<std::is_floating_point<RealT>::value>* = nullptr)
+template <typename NumberT0, typename NumberT1,
+          typename DistributionT,
+          typename EngineT,
+          typename NumberT = detail::select_number_t<NumberT0, NumberT1>
+                >
+NumberT generate_random_number(const NumberT min, const NumberT max,
+                               EngineT& e)
 {
-  constexpr RealT zero = 0.0;
-  return generate_random_number(zero, max, m, e);
+ return detail::generate_random_number<NumberT,
+                       DistributionT,
+                       EngineT>(static_cast<NumberT>(min), static_cast<NumberT>(max), e);
 }
 
-} // namespace detail
-
-template <typename RealT>
-RealT generate_random_number(const RealT max,
-                             std::enable_if_t<std::is_floating_point<RealT>::value>* = nullptr)
+template <typename NumberT>
+NumberT generate_random_number(const NumberT max)
 {
-  constexpr RealT zero = 0.0;
-  return generate_random_number(zero, max);
+ return generate_random_number<NumberT>(0, max);
 }
 
 // Function object:
@@ -240,16 +234,24 @@ class random_number_generator final
   std::mutex l;
   std::random_device rd;
   std::default_random_engine e;
+
+  using seed_type = typename decltype(e)::result_type;
  
   public:
-  using number_type = NumberT;
+  using number_type         = NumberT;
+  using random_engine_type  = decltype(e);
+  using random_device_type  = decltype(rd);
+
+  public:
+  random_device_type& random_device() noexcept { return rd; } 
+  random_engine_type& random_engine() noexcept { return e; }
  
   public:
   random_number_generator() {
     detail::randomize_rng(l, e);
   }
  
-  explicit random_number_generator(const int seed) {
+  explicit random_number_generator(const seed_type seed) {
     detail::randomize_rng(seed, l, e);
   }
 
@@ -267,21 +269,21 @@ class random_number_generator final
   }
  
   NumberT operator()(const NumberT max) { 
-    return detail::generate_random_number(max, l, e); 
+    return detail::generate_random_number<NumberT>(max, l, e); 
   }
  
   NumberT operator()(const NumberT min, const NumberT max) { 
-    return detail::generate_random_number(min, max, l, e); 
+    return detail::generate_random_number<NumberT>(min, max, l, e); 
   }
  
   public:
-  void seed(const int n) { 
+  void seed(const seed_type n) { 
     detail::randomize_rng(n, l, e); 
   }
 };
 
-} // inline namespace version_1_0
+} // inline namespace version_*
 
-}} // namespace ceph::util
+} // namespace ceph::util
 
 #endif

diff --git a/src/test/common/test_random.cc b/src/test/common/test_random.cc
index 0b8b566af264ee54e6d5ffcafbc1048ce240b928..85bb732b4752d434094db3bba0844cf89c40294f 100644 (file)
--- a/src/test/common/test_random.cc
+++ b/src/test/common/test_random.cc
@@ -86,6 +86,25 @@ TEST(util, test_random)
        ASSERT_NE(a, b);
      }
   }
+
+  // Check that the nullary version accepts different numeric types:
+  {
+    long def = ceph::util::generate_random_number();
+    long l = ceph::util::generate_random_number<long>();
+    int64_t i = ceph::util::generate_random_number<int64_t>();
+    double d = ceph::util::generate_random_number<double>();
+
+    swallow_values(def, l, i, d);
+  }
+
+  // (optimistically) Check that the nullary and unary versions never return < 0:
+  {
+    for(long i = 0; 1000000 != i; i++) {
+     ASSERT_LE(0, ceph::util::generate_random_number());
+     ASSERT_LE(0, ceph::util::generate_random_number(1));
+     ASSERT_LE(0, ceph::util::generate_random_number<float>(1.0));
+    }
+  }
  
   {
   auto a = ceph::util::generate_random_number(1, std::numeric_limits<int>::max());
@@ -100,11 +119,16 @@ TEST(util, test_random)
   }
  
   for (auto n = 100000; n; --n) {
-     constexpr int min = 0, max = 6;
-     int a = ceph::util::generate_random_number(min, max);
+     int a = ceph::util::generate_random_number(0, 6);
      ASSERT_GT(a, -1);
      ASSERT_LT(a, 7);
    }
+
+  // Check bounding on zero (checking appropriate value for zero compiles and works):
+  for (auto n = 10; n; --n) {
+    ASSERT_EQ(0, ceph::util::generate_random_number<int>(0, 0));
+    ASSERT_EQ(0, ceph::util::generate_random_number<float>(0.0, 0.0));
+  }
  
   // Multiple types (integral):
   {
@@ -133,6 +157,37 @@ TEST(util, test_random)
     float min = 1.0, max = 0.0;
     type_check_ok(min, max);
   }
+
+  // When combining types, everything should convert to the largest type:
+  {
+    // Check with integral types:
+    {
+    int x = 0;
+    long long y = 1;
+
+    auto z = ceph::util::generate_random_number(x, y);
+
+    bool result = std::is_same_v<decltype(z), decltype(y)>;
+
+    ASSERT_TRUE(result);
+    }
+
+    // Check with floating-point types:
+    {
+    float x = 0.0;
+    long double y = 1.0;
+
+    auto z = ceph::util::generate_random_number(x, y);
+
+    bool result = std::is_same_v<decltype(z), decltype(y)>;
+
+    ASSERT_TRUE(result);
+    }
+
+    // It would be nice to have a test to check that mixing integral and floating point
+    // numbers should not compile, however we currently have no good way I know of
+    // to do such negative tests.
+  }
 }
 
 TEST(util, test_random_class_interface)