From e6d67260f11c6936578cda67779eda1bfbe253cb Mon Sep 17 00:00:00 2001
From: "Alan M. Carroll" <amc@apache.org>
Date: Thu, 3 Aug 2017 16:18:07 -0500
Subject: [PATCH] Fixes to scalar for better handling of division and
 comparisons with built in types.

---
 cmd/traffic_cache_tool/CacheTool.cc |   2 +-
 lib/ts/Scalar.h                     | 236 ++--------------------------
 lib/ts/test_Scalar.cc               |  26 +++
 3 files changed, 43 insertions(+), 221 deletions(-)

diff --git a/cmd/traffic_cache_tool/CacheTool.cc b/cmd/traffic_cache_tool/CacheTool.cc
index e73e20f8d2e..ae35363f41e 100644
--- a/cmd/traffic_cache_tool/CacheTool.cc
+++ b/cmd/traffic_cache_tool/CacheTool.cc
@@ -1253,7 +1253,7 @@ main(int argc, char *argv[])
   Commands.setArgIndex(optind);
 
   if (help) {
-    Commands.helpMessage(argc-1, argv+1);
+    Commands.helpMessage(argc - 1, argv + 1);
     exit(1);
   }
 
diff --git a/lib/ts/Scalar.h b/lib/ts/Scalar.h
index e7319678296..e22fed9d0d8 100644
--- a/lib/ts/Scalar.h
+++ b/lib/ts/Scalar.h
@@ -493,21 +493,14 @@ Scalar<N, C, T>::scale()
 }
 
 // --- Compare operators
-// These optimize nicely due to dead code elimination.
+// These optimize nicely because if R::num or R::den is 1 the compiler will drop it.
 
 template <intmax_t N, typename C1, intmax_t S, typename I, typename T>
 bool
 operator<(Scalar<N, C1, T> const &lhs, Scalar<S, I, T> const &rhs)
 {
   typedef std::ratio<N, S> R;
-  if (N == S)
-    return lhs.count() < rhs.count();
-  else if (R::den == 1)
-    return lhs.count() * R::num < rhs.count();
-  else if (R::num == 1)
-    return lhs.count() < rhs.count() * R::den;
-  else
-    return lhs.value() < rhs.value();
+  return lhs.count() * R::num < rhs.count() * R::den;
 }
 
 template <intmax_t N, typename C1, intmax_t S, typename I, typename T>
@@ -515,14 +508,7 @@ bool
 operator==(Scalar<N, C1, T> const &lhs, Scalar<S, I, T> const &rhs)
 {
   typedef std::ratio<N, S> R;
-  if (N == S)
-    return lhs.count() == rhs.count();
-  else if (R::den == 1)
-    return lhs.count() * R::num == rhs.count();
-  else if (R::num == 1)
-    return lhs.count() == rhs.count() * R::den;
-  else
-    return lhs.value() == rhs.value();
+  return lhs.count() * R::num == rhs.count() * R::den;
 }
 
 template <intmax_t N, typename C1, intmax_t S, typename I, typename T>
@@ -530,222 +516,24 @@ bool
 operator<=(Scalar<N, C1, T> const &lhs, Scalar<S, I, T> const &rhs)
 {
   typedef std::ratio<N, S> R;
-  if (N == S)
-    return lhs.count() <= rhs.count();
-  else if (R::den == 1)
-    return lhs.count() * R::num <= rhs.count();
-  else if (R::num == 1)
-    return lhs.count() <= rhs.count() * R::den;
-  else
-    return lhs.value() <= rhs.value();
+  return lhs.count() * R::num <= rhs.count() * R::den;
 }
 
 // Derived compares.
-
-template <intmax_t N, typename C1, intmax_t S, typename I, typename T>
+template <intmax_t N, typename C, intmax_t S, typename I, typename T>
 bool
-operator>(Scalar<N, C1, T> const &lhs, Scalar<S, I, T> const &rhs)
+operator>(Scalar<N, C, T> const &lhs, Scalar<S, I, T> const &rhs)
 {
   return rhs < lhs;
 }
 
-template <intmax_t N, typename C1, intmax_t S, typename I, typename T>
+template <intmax_t N, typename C, intmax_t S, typename I, typename T>
 bool
-operator>=(Scalar<N, C1, T> const &lhs, Scalar<S, I, T> const &rhs)
+operator>=(Scalar<N, C, T> const &lhs, Scalar<S, I, T> const &rhs)
 {
   return rhs <= lhs;
 }
 
-// Do the integer compares.
-// A bit ugly to handle the issue that integers without explicit type are <int>. Therefore suppport
-// must be provided for comparison not just to the counter type C but also explicitly <int>, otherwise
-// function template argument deduction may fail (because it can't figure out what to use for <C>).
-
-template <intmax_t N, typename C, typename T>
-bool
-operator<(Scalar<N, C, T> const &lhs, C n)
-{
-  return lhs.value() < n;
-}
-template <intmax_t N, typename C, typename T>
-bool
-operator<(C n, Scalar<N, C, T> const &rhs)
-{
-  return n < rhs.value();
-}
-template <intmax_t N, typename C, typename T>
-bool
-operator<(Scalar<N, C, T> const &lhs, int n)
-{
-  return lhs.value() < static_cast<C>(n);
-}
-template <intmax_t N, typename C, typename T>
-bool
-operator<(int n, Scalar<N, C, T> const &rhs)
-{
-  return static_cast<C>(n) < rhs.value();
-}
-template <intmax_t N>
-bool
-operator<(Scalar<N, int> const &lhs, int n)
-{
-  return lhs.value() < n;
-}
-template <intmax_t N>
-bool
-operator<(int n, Scalar<N, int> const &rhs)
-{
-  return n < rhs.value();
-}
-
-template <intmax_t N, typename C, typename T>
-bool
-operator==(Scalar<N, C, T> const &lhs, C n)
-{
-  return lhs.value() == n;
-}
-template <intmax_t N, typename C, typename T>
-bool
-operator==(C n, Scalar<N, C, T> const &rhs)
-{
-  return n == rhs.value();
-}
-template <intmax_t N, typename C, typename T>
-bool
-operator==(Scalar<N, C, T> const &lhs, int n)
-{
-  return lhs.value() == static_cast<C>(n);
-}
-template <intmax_t N, typename C, typename T>
-bool
-operator==(int n, Scalar<N, C, T> const &rhs)
-{
-  return static_cast<C>(n) == rhs.value();
-}
-template <intmax_t N>
-bool
-operator==(Scalar<N, int> const &lhs, int n)
-{
-  return lhs.value() == n;
-}
-template <intmax_t N>
-bool
-operator==(int n, Scalar<N, int> const &rhs)
-{
-  return n == rhs.value();
-}
-
-template <intmax_t N, typename C, typename T>
-bool
-operator>(Scalar<N, C, T> const &lhs, C n)
-{
-  return lhs.value() > n;
-}
-template <intmax_t N, typename C, typename T>
-bool
-operator>(C n, Scalar<N, C, T> const &rhs)
-{
-  return n > rhs.value();
-}
-template <intmax_t N, typename C, typename T>
-bool
-operator>(Scalar<N, C, T> const &lhs, int n)
-{
-  return lhs.value() > static_cast<C>(n);
-}
-template <intmax_t N, typename C, typename T>
-bool
-operator>(int n, Scalar<N, C, T> const &rhs)
-{
-  return static_cast<C>(n) > rhs.value();
-}
-template <intmax_t N>
-bool
-operator>(Scalar<N, int> const &lhs, int n)
-{
-  return lhs.value() > n;
-}
-template <intmax_t N>
-bool
-operator>(int n, Scalar<N, int> const &rhs)
-{
-  return n > rhs.value();
-}
-
-template <intmax_t N, typename C, typename T>
-bool
-operator<=(Scalar<N, C, T> const &lhs, C n)
-{
-  return lhs.value() <= n;
-}
-template <intmax_t N, typename C, typename T>
-bool
-operator<=(C n, Scalar<N, C, T> const &rhs)
-{
-  return n <= rhs.value();
-}
-template <intmax_t N, typename C, typename T>
-bool
-operator<=(Scalar<N, C, T> const &lhs, int n)
-{
-  return lhs.value() <= static_cast<C>(n);
-}
-template <intmax_t N, typename C, typename T>
-bool
-operator<=(int n, Scalar<N, C, T> const &rhs)
-{
-  return static_cast<C>(n) <= rhs.value();
-}
-template <intmax_t N>
-bool
-operator<=(Scalar<N, int> const &lhs, int n)
-{
-  return lhs.value() <= n;
-}
-template <intmax_t N>
-bool
-operator<=(int n, Scalar<N, int> const &rhs)
-{
-  return n <= rhs.value();
-}
-
-template <intmax_t N, typename C, typename T>
-bool
-operator>=(Scalar<N, C, T> const &lhs, C n)
-{
-  return lhs.value() >= n;
-}
-template <intmax_t N, typename C, typename T>
-bool
-operator>=(C n, Scalar<N, C, T> const &rhs)
-{
-  return n >= rhs.value();
-}
-template <intmax_t N, typename C, typename T>
-bool
-operator>=(Scalar<N, C, T> const &lhs, int n)
-{
-  return lhs.value() >= static_cast<C>(n);
-}
-template <intmax_t N, typename C, typename T>
-bool
-operator>=(int n, Scalar<N, C, T> const &rhs)
-{
-  return static_cast<C>(n) >= rhs.value();
-}
-template <intmax_t N>
-bool
-operator>=(Scalar<N, int> const &lhs, int n)
-{
-  return lhs.value() >= n;
-}
-template <intmax_t N>
-bool
-operator>=(int n, Scalar<N, int> const &rhs)
-{
-  return n >= rhs.value();
-}
-
 // Arithmetic operators
 template <intmax_t N, typename C, typename T>
 auto
@@ -1066,6 +854,14 @@ Scalar<N, C, T>::operator/=(C n) -> self &
   return *this;
 }
 
+template <intmax_t N, typename C, intmax_t S, typename I, typename T>
+auto
+operator/(Scalar<N, C, T> lhs, Scalar<S, I, T> rhs) -> typename std::common_type<C, I>::type
+{
+  using R = std::ratio<N, S>;
+  return (lhs.count() * R::num) / (rhs.count() * R::den);
+}
+
 template <intmax_t N, typename C, typename T, typename I>
 Scalar<N, C, T>
 operator/(Scalar<N, C, T> lhs, I n)
diff --git a/lib/ts/test_Scalar.cc b/lib/ts/test_Scalar.cc
index 369610325b9..0aa364eae81 100644
--- a/lib/ts/test_Scalar.cc
+++ b/lib/ts/test_Scalar.cc
@@ -343,6 +343,31 @@ struct KBytes_tag {
 };
 std::string const KBytes_tag::label(" bytes");
 
+void
+Test_8()
+{
+  using ts::Scalar;
+  using S16 = Scalar<16>;
+  using S48 = Scalar<48>;
+  using S64 = Scalar<64>;
+  using S91 = Scalar<91>;
+
+  TestBox test("TS.Scalar: division tests");
+
+  S16 s16(9);
+  S48 s48(27);
+  S64 s64(54);
+  S91 s91(17);
+
+  test.equal(s16 / 9, S16::scale());
+  test.equal(s64 / s16, 24);
+  test.equal(s91 / 16, S91::scale());
+  test.equal(s48 / 24, 48); // correct because of rounding.
+  test.equal(s48 / 12, 96);
+  test.equal(s48 / 3, 432);
+  test.equal(s91 / s16, 10);
+}
+
 void
 Test_IO()
 {
@@ -401,6 +426,7 @@ main(int, char **)
   Test_5();
   Test_6();
   Test_7();
+  Test_8();
   Test_IO();
   TestBox::print_summary();
   return 0;