change rcutils_time_point_value_t type from uint64_t to int64_t (ros2…

…#429)

* change rcutils_time_point_value_t type from uint64_t to int64_t

* small style changes

* fix test time datatype

* Update time primatives to int64_t

* change time primitive datatype to signed

* A few more instances of UL to L

rclcpp/include/rclcpp/time.hpp

@@ -35,7 +35,7 @@ class Time
   Time(int32_t seconds, uint32_t nanoseconds, rcl_clock_type_t clock_type = RCL_SYSTEM_TIME);
 
   RCLCPP_PUBLIC
-  explicit Time(uint64_t nanoseconds = 0, rcl_clock_type_t clock = RCL_SYSTEM_TIME);
+  explicit Time(int64_t nanoseconds = 0, rcl_clock_type_t clock = RCL_SYSTEM_TIME);
 
   RCLCPP_PUBLIC
   Time(const Time & rhs);

rclcpp/include/rclcpp/utilities.hpp

@@ -17,6 +17,7 @@
 
 #include <chrono>
 #include <functional>
+#include <limits>
 
 #include "rclcpp/visibility_control.hpp"
 
@@ -109,6 +110,74 @@ RCLCPP_PUBLIC
 bool
 sleep_for(const std::chrono::nanoseconds & nanoseconds);
 
+/// Safely check if addition will overflow.
+/**
+ * The type of the operands, T, should have defined
+ * std::numeric_limits<T>::max(), `>`, `<` and `-` operators.
+ *
+ * \param[in] x is the first addend.
+ * \param[in] y is the second addend.
+ * \tparam T is type of the operands.
+ * \return True if the x + y sum is greater than T::max value.
+ */
+template<typename T>
+bool
+add_will_overflow(const T x, const T y)
+{
+  return (y > 0) && (x > (std::numeric_limits<T>::max() - y));
+}
+
+/// Safely check if addition will underflow.
+/**
+ * The type of the operands, T, should have defined
+ * std::numeric_limits<T>::min(), `>`, `<` and `-` operators.
+ *
+ * \param[in] x is the first addend.
+ * \param[in] y is the second addend.
+ * \tparam T is type of the operands.
+ * \return True if the x + y sum is less than T::min value.
+ */
+template<typename T>
+bool
+add_will_underflow(const T x, const T y)
+{
+  return (y < 0) && (x < (std::numeric_limits<T>::min() - y));
+}
+
+/// Safely check if subtraction will overflow.
+/**
+ * The type of the operands, T, should have defined
+ * std::numeric_limits<T>::max(), `>`, `<` and `+` operators.
+ *
+ * \param[in] x is the minuend.
+ * \param[in] y is the subtrahend.
+ * \tparam T is type of the operands.
+ * \return True if the difference `x - y` sum is grater than T::max value.
+ */
+template<typename T>
+bool
+sub_will_overflow(const T x, const T y)
+{
+  return (y < 0) && (x > (std::numeric_limits<T>::max() + y));
+}
+
+/// Safely check if subtraction will underflow.
+/**
+ * The type of the operands, T, should have defined
+ * std::numeric_limits<T>::min(), `>`, `<` and `+` operators.
+ *
+ * \param[in] x is the minuend.
+ * \param[in] y is the subtrahend.
+ * \tparam T is type of the operands.
+ * \return True if the difference `x - y` sum is less than T::min value.
+ */
+template<typename T>
+bool
+sub_will_underflow(const T x, const T y)
+{
+  return (y > 0) && (x < (std::numeric_limits<T>::min() + y));
+}
+
 }  // namespace rclcpp
 
 #endif  // RCLCPP__UTILITIES_HPP_

rclcpp/src/rclcpp/time.cpp

@@ -26,6 +26,8 @@
 
 #include "rcutils/logging_macros.h"
 
+#include "rclcpp/utilities.hpp"
+
 namespace
 {
 
@@ -50,11 +52,11 @@ Time::Time(int32_t seconds, uint32_t nanoseconds, rcl_clock_type_t clock_type)
     throw std::runtime_error("cannot store a negative time point in rclcpp::Time");
   }
 
-  rcl_time_.nanoseconds = RCL_S_TO_NS(static_cast<uint64_t>(seconds));
+  rcl_time_.nanoseconds = RCL_S_TO_NS(static_cast<int64_t>(seconds));
   rcl_time_.nanoseconds += nanoseconds;
 }
 
-Time::Time(uint64_t nanoseconds, rcl_clock_type_t clock_type)
+Time::Time(int64_t nanoseconds, rcl_clock_type_t clock_type)
 : rcl_time_(init_time_point(clock_type))
 {
   rcl_time_.nanoseconds = nanoseconds;
@@ -75,7 +77,7 @@ Time::Time(
     throw std::runtime_error("cannot store a negative time point in rclcpp::Time");
   }
 
-  rcl_time_.nanoseconds = RCL_S_TO_NS(static_cast<uint64_t>(time_msg.sec));
+  rcl_time_.nanoseconds = RCL_S_TO_NS(static_cast<int64_t>(time_msg.sec));
   rcl_time_.nanoseconds += time_msg.nanosec;
 }
 
@@ -115,7 +117,7 @@ Time::operator=(const builtin_interfaces::msg::Time & time_msg)
   rcl_clock_type_t ros_time = RCL_ROS_TIME;
   rcl_time_ = init_time_point(ros_time);  // TODO(tfoote) hard coded ROS here
 
-  rcl_time_.nanoseconds = RCL_S_TO_NS(static_cast<uint64_t>(time_msg.sec));
+  rcl_time_.nanoseconds = RCL_S_TO_NS(static_cast<int64_t>(time_msg.sec));
   rcl_time_.nanoseconds += time_msg.nanosec;
   return *this;
 }
@@ -179,11 +181,11 @@ Time::operator>(const rclcpp::Time & rhs) const
 Time
 Time::operator+(const rclcpp::Duration & rhs) const
 {
-  if (rhs.nanoseconds() > 0 && (uint64_t)rhs.nanoseconds() >
-    std::numeric_limits<rcl_time_point_value_t>::max() -
-    (rcl_time_point_value_t)this->nanoseconds())
-  {
-    throw std::overflow_error("addition leads to uint64_t overflow");
+  if (rclcpp::add_will_overflow(rhs.nanoseconds(), this->nanoseconds())) {
+    throw std::overflow_error("addition leads to int64_t overflow");
+  }
+  if (rclcpp::add_will_underflow(rhs.nanoseconds(), this->nanoseconds())) {
+    throw std::underflow_error("addition leads to int64_t underflow");
   }
   return Time(this->nanoseconds() + rhs.nanoseconds(), this->get_clock_type());
 }
@@ -195,17 +197,12 @@ Time::operator-(const rclcpp::Time & rhs) const
     throw std::runtime_error("can't subtract times with different time sources");
   }
 
-  if (rcl_time_.nanoseconds >
-    (uint64_t)std::numeric_limits<rcl_duration_value_t>::max() + rhs.rcl_time_.nanoseconds)
-  {
-    throw std::underflow_error("time subtraction leads to int64_t overflow");
+  if (rclcpp::sub_will_overflow(rcl_time_.nanoseconds, rhs.rcl_time_.nanoseconds)) {
+    throw std::overflow_error("time subtraction leads to int64_t overflow");
   }
 
-  if (rcl_time_.nanoseconds < rhs.rcl_time_.nanoseconds) {
-    rcl_time_point_value_t negative_delta = rhs.rcl_time_.nanoseconds - rcl_time_.nanoseconds;
-    if (negative_delta > (uint64_t) std::numeric_limits<rcl_duration_value_t>::min()) {
-      throw std::underflow_error("time subtraction leads to int64_t underflow");
-    }
+  if (rclcpp::sub_will_underflow(rcl_time_.nanoseconds, rhs.rcl_time_.nanoseconds)) {
+    throw std::underflow_error("time subtraction leads to int64_t underflow");
   }
 
   return Duration(rcl_time_.nanoseconds - rhs.rcl_time_.nanoseconds);
@@ -214,21 +211,17 @@ Time::operator-(const rclcpp::Time & rhs) const
 Time
 Time::operator-(const rclcpp::Duration & rhs) const
 {
-  if (rhs.nanoseconds() > 0 && rcl_time_.nanoseconds >
-    std::numeric_limits<rcl_time_point_value_t>::max() - (uint64_t)rhs.nanoseconds())
-  {
-    throw std::underflow_error("time subtraction leads to uint64_t overflow");
+  if (rclcpp::sub_will_overflow(rcl_time_.nanoseconds, rhs.nanoseconds())) {
+    throw std::overflow_error("time subtraction leads to int64_t overflow");
   }
-  if (rcl_time_.nanoseconds < (uint64_t) std::numeric_limits<rcl_duration_value_t>::max() &&
-    (int64_t)rcl_time_.nanoseconds < (int64_t)rhs.nanoseconds())
-  {
-    throw std::underflow_error("time subtraction leads to uint64_t underflow");
+  if (rclcpp::sub_will_underflow(rcl_time_.nanoseconds, rhs.nanoseconds())) {
+    throw std::underflow_error("time subtraction leads to int64_t underflow");
   }
 
   return Time(rcl_time_.nanoseconds - rhs.nanoseconds(), rcl_time_.clock_type);
 }
 
-uint64_t
+int64_t
 Time::nanoseconds() const
 {
   return rcl_time_.nanoseconds;
@@ -243,10 +236,11 @@ Time::get_clock_type() const
 Time
 operator+(const rclcpp::Duration & lhs, const rclcpp::Time & rhs)
 {
-  if (rhs.nanoseconds() >
-    std::numeric_limits<rcl_time_point_value_t>::max() - (rcl_time_point_value_t)lhs.nanoseconds())
-  {
-    throw std::overflow_error("addition leads to uint64_t overflow");
+  if (rclcpp::add_will_overflow(rhs.nanoseconds(), lhs.nanoseconds())) {
+    throw std::overflow_error("addition leads to int64_t overflow");
+  }
+  if (rclcpp::add_will_underflow(rhs.nanoseconds(), lhs.nanoseconds())) {
+    throw std::underflow_error("addition leads to int64_t underflow");
   }
   return Time(lhs.nanoseconds() + rhs.nanoseconds(), rhs.get_clock_type());
 }

rclcpp/test/test_node.cpp

@@ -99,5 +99,5 @@ TEST_F(TestNode, now) {
   auto now_builtin = node->now().nanoseconds();
   auto now_external = clock->now().nanoseconds();
   EXPECT_GE(now_external, now_builtin);
-  EXPECT_LT(now_external - now_builtin, 50000ul);
+  EXPECT_LT(now_external - now_builtin, 50000L);
 }

rclcpp/test/test_time.cpp

@@ -23,6 +23,18 @@
 #include "rclcpp/clock.hpp"
 #include "rclcpp/rclcpp.hpp"
 #include "rclcpp/time.hpp"
+#include "rclcpp/utilities.hpp"
+
+namespace
+{
+
+bool logical_eq(const bool a, const bool b)
+{
+  return (a && b) || ((!a) && !(b));
+}
+
+}  // namespace
+
 
 class TestTime : public ::testing::Test
 {
@@ -77,9 +89,9 @@ TEST(TestTime, conversions) {
 
   rclcpp::Time time = msg;
   EXPECT_EQ(
-    RCL_S_TO_NS(static_cast<uint64_t>(msg.sec)) + static_cast<uint64_t>(msg.nanosec),
+    RCL_S_TO_NS(static_cast<int64_t>(msg.sec)) + static_cast<int64_t>(msg.nanosec),
     time.nanoseconds());
-  EXPECT_EQ(static_cast<uint64_t>(msg.sec), RCL_NS_TO_S(time.nanoseconds()));
+  EXPECT_EQ(static_cast<int64_t>(msg.sec), RCL_NS_TO_S(time.nanoseconds()));
 
   builtin_interfaces::msg::Time negative_time_msg;
   negative_time_msg.sec = -1;
@@ -148,11 +160,87 @@ TEST(TestTime, operators) {
   }
 }
 
+TEST(TestTime, overflow_detectors) {
+  /////////////////////////////////////////////////////////////////////////////
+  // Test logical_eq call first:
+  EXPECT_TRUE(logical_eq(false, false));
+  EXPECT_FALSE(logical_eq(false, true));
+  EXPECT_FALSE(logical_eq(true, false));
+  EXPECT_TRUE(logical_eq(true, true));
+
+  /////////////////////////////////////////////////////////////////////////////
+  // Exhaustive test of all int8_t values
+  using test_type_t = int8_t;
+  // big_type_t encompasses test_type_t:
+  //  big_type_t::min < test_type_t::min
+  //  big_type_t::max > test_type_t::max
+  using big_type_t = int16_t;
+  const big_type_t min_val = std::numeric_limits<test_type_t>::min();
+  const big_type_t max_val = std::numeric_limits<test_type_t>::max();
+  // 256 * 256 = 64K total loops, should be pretty fast on everything
+  for (big_type_t y = min_val; y <= max_val; ++y) {
+    for (big_type_t x = min_val; x <= max_val; ++x) {
+      const big_type_t sum = x + y;
+      const big_type_t diff = x - y;
+
+      const bool add_will_overflow =
+        rclcpp::add_will_overflow(test_type_t(x), test_type_t(y));
+      const bool add_did_overflow = sum > max_val;
+      EXPECT_TRUE(logical_eq(add_will_overflow, add_did_overflow));
+
+      const bool add_will_underflow =
+        rclcpp::add_will_underflow(test_type_t(x), test_type_t(y));
+      const bool add_did_underflow = sum < min_val;
+      EXPECT_TRUE(logical_eq(add_will_underflow, add_did_underflow));
+
+      const bool sub_will_overflow =
+        rclcpp::sub_will_overflow(test_type_t(x), test_type_t(y));
+      const bool sub_did_overflow = diff > max_val;
+      EXPECT_TRUE(logical_eq(sub_will_overflow, sub_did_overflow));
+
+      const bool sub_will_underflow =
+        rclcpp::sub_will_underflow(test_type_t(x), test_type_t(y));
+      const bool sub_did_underflow = diff < min_val;
+      EXPECT_TRUE(logical_eq(sub_will_underflow, sub_did_underflow));
+    }
+  }
+
+  // Few selected tests for int64_t
+  EXPECT_TRUE(rclcpp::add_will_overflow<int64_t>(INT64_MAX, 1));
+  EXPECT_FALSE(rclcpp::add_will_overflow<int64_t>(INT64_MAX, -1));
+  EXPECT_TRUE(rclcpp::add_will_underflow<int64_t>(INT64_MIN, -1));
+  EXPECT_FALSE(rclcpp::add_will_underflow<int64_t>(INT64_MIN, 1));
+
+  EXPECT_FALSE(rclcpp::sub_will_overflow<int64_t>(INT64_MAX, 1));
+  EXPECT_TRUE(rclcpp::sub_will_overflow<int64_t>(INT64_MAX, -1));
+  EXPECT_FALSE(rclcpp::sub_will_underflow<int64_t>(INT64_MIN, -1));
+  EXPECT_TRUE(rclcpp::sub_will_underflow<int64_t>(INT64_MIN, 1));
+}
+
 TEST(TestTime, overflows) {
   rclcpp::Time max_time(std::numeric_limits<rcl_time_point_value_t>::max());
   rclcpp::Time min_time(std::numeric_limits<rcl_time_point_value_t>::min());
   rclcpp::Duration one(1);
+  rclcpp::Duration two(2);
 
+  // Cross min/max
   EXPECT_THROW(max_time + one, std::overflow_error);
   EXPECT_THROW(min_time - one, std::underflow_error);
+  EXPECT_THROW(max_time - min_time, std::overflow_error);
+  EXPECT_THROW(min_time - max_time, std::underflow_error);
+  EXPECT_NO_THROW(max_time - max_time);
+  EXPECT_NO_THROW(min_time - min_time);
+
+  // Cross zero in both directions
+  rclcpp::Time one_time(1);
+  EXPECT_NO_THROW(one_time - two);
+
+  rclcpp::Time minus_one_time(-1);
+  EXPECT_NO_THROW(minus_one_time + two);
+
+  EXPECT_NO_THROW(one_time - minus_one_time);
+  EXPECT_NO_THROW(minus_one_time - one_time);
+
+  rclcpp::Time two_time(2);
+  EXPECT_NO_THROW(one_time - two_time);
 }

rclcpp/test/test_time_source.cpp

@@ -116,7 +116,7 @@ TEST_F(TestTimeSource, clock) {
 
   auto t_out = ros_clock->now();
 
-  EXPECT_NE(0UL, t_out.nanoseconds());
+  EXPECT_NE(0L, t_out.nanoseconds());
   EXPECT_LT(t_low.nanoseconds(), t_out.nanoseconds());
   EXPECT_GT(t_high.nanoseconds(), t_out.nanoseconds());
 }
@@ -190,7 +190,7 @@ TEST_F(TestTimeSource, callbacks) {
 
   auto t_out = ros_clock->now();
 
-  EXPECT_NE(0UL, t_out.nanoseconds());
+  EXPECT_NE(0L, t_out.nanoseconds());
   EXPECT_LT(t_low.nanoseconds(), t_out.nanoseconds());
   EXPECT_GT(t_high.nanoseconds(), t_out.nanoseconds());
 
@@ -222,7 +222,7 @@ TEST_F(TestTimeSource, callbacks) {
 
   t_out = ros_clock->now();
 
-  EXPECT_NE(0UL, t_out.nanoseconds());
+  EXPECT_NE(0L, t_out.nanoseconds());
   EXPECT_LT(t_low.nanoseconds(), t_out.nanoseconds());
   EXPECT_GT(t_high.nanoseconds(), t_out.nanoseconds());
 }
@@ -296,7 +296,7 @@ TEST_F(TestTimeSource, callback_handler_erasure) {
 
   auto t_out = ros_clock->now();
 
-  EXPECT_NE(0UL, t_out.nanoseconds());
+  EXPECT_NE(0L, t_out.nanoseconds());
   EXPECT_LT(t_low.nanoseconds(), t_out.nanoseconds());
   EXPECT_GT(t_high.nanoseconds(), t_out.nanoseconds());
 
@@ -320,7 +320,7 @@ TEST_F(TestTimeSource, callback_handler_erasure) {
 
   t_out = ros_clock->now();
 
-  EXPECT_NE(0UL, t_out.nanoseconds());
+  EXPECT_NE(0L, t_out.nanoseconds());
   EXPECT_LT(t_low.nanoseconds(), t_out.nanoseconds());
   EXPECT_GT(t_high.nanoseconds(), t_out.nanoseconds());
 }