From 88a342db294c370b103b7328e353e84f08f27584 Mon Sep 17 00:00:00 2001 From: monidzik <51805940+monidzik@users.noreply.github.com> Date: Fri, 22 Nov 2019 19:23:55 +0100 Subject: [PATCH] Type conversions fixes (#901) * Fix type conversions Signed-off-by: Monika Idzik * Add static_casts Signed-off-by: Monika Idzik * Address PR comments Signed-off-by: Monika Idzik * Remove one time use variable Signed-off-by: Monika Idzik --- rclcpp/src/rclcpp/duration.cpp | 32 +++++++++++-------- rclcpp/src/rclcpp/parameter_client.cpp | 2 +- rclcpp/src/rclcpp/parameter_value.cpp | 2 +- rclcpp/src/rclcpp/utilities.cpp | 4 +-- rclcpp/test/test_duration.cpp | 6 ++-- rclcpp/test/test_time.cpp | 2 +- .../src/lifecycle_node_interface_impl.hpp | 18 +++++------ rclcpp_lifecycle/src/state.cpp | 2 +- rclcpp_lifecycle/src/transition.cpp | 2 +- 9 files changed, 37 insertions(+), 33 deletions(-) diff --git a/rclcpp/src/rclcpp/duration.cpp b/rclcpp/src/rclcpp/duration.cpp index 35db0165f5..bdf9bdace7 100644 --- a/rclcpp/src/rclcpp/duration.cpp +++ b/rclcpp/src/rclcpp/duration.cpp @@ -55,7 +55,8 @@ Duration::Duration(const Duration & rhs) Duration::Duration( const builtin_interfaces::msg::Duration & duration_msg) { - rcl_duration_.nanoseconds = RCL_S_TO_NS(static_cast(duration_msg.sec)); + rcl_duration_.nanoseconds = + static_cast(RCL_S_TO_NS(duration_msg.sec)); rcl_duration_.nanoseconds += duration_msg.nanosec; } @@ -122,15 +123,15 @@ Duration::operator>(const rclcpp::Duration & rhs) const void bounds_check_duration_sum(int64_t lhsns, int64_t rhsns, uint64_t max) { - auto abs_lhs = (uint64_t)std::abs(lhsns); - auto abs_rhs = (uint64_t)std::abs(rhsns); + auto abs_lhs = static_cast(std::abs(lhsns)); + auto abs_rhs = static_cast(std::abs(rhsns)); if (lhsns > 0 && rhsns > 0) { - if (abs_lhs + abs_rhs > (uint64_t) max) { + if (abs_lhs + abs_rhs > max) { throw std::overflow_error("addition leads to int64_t overflow"); } } else if (lhsns < 0 && rhsns < 0) { - if (abs_lhs + abs_rhs > (uint64_t) max) { + if (abs_lhs + abs_rhs > max) { throw std::underflow_error("addition leads to int64_t underflow"); } } @@ -150,15 +151,15 @@ Duration::operator+(const rclcpp::Duration & rhs) const void bounds_check_duration_difference(int64_t lhsns, int64_t rhsns, uint64_t max) { - auto abs_lhs = (uint64_t)std::abs(lhsns); - auto abs_rhs = (uint64_t)std::abs(rhsns); + auto abs_lhs = static_cast(std::abs(lhsns)); + auto abs_rhs = static_cast(std::abs(rhsns)); if (lhsns > 0 && rhsns < 0) { - if (abs_lhs + abs_rhs > (uint64_t) max) { + if (abs_lhs + abs_rhs > max) { throw std::overflow_error("duration subtraction leads to int64_t overflow"); } } else if (lhsns < 0 && rhsns > 0) { - if (abs_lhs + abs_rhs > (uint64_t) max) { + if (abs_lhs + abs_rhs > max) { throw std::underflow_error("duration subtraction leads to int64_t underflow"); } } @@ -181,8 +182,9 @@ bounds_check_duration_scale(int64_t dns, double scale, uint64_t max) { auto abs_dns = static_cast(std::abs(dns)); auto abs_scale = std::abs(scale); - - if (abs_scale > 1.0 && abs_dns > static_cast(max / abs_scale)) { + if (abs_scale > 1.0 && + abs_dns > static_cast(static_cast(max) / static_cast(abs_scale))) + { if ((dns > 0 && scale > 0) || (dns < 0 && scale < 0)) { throw std::overflow_error("duration scaling leads to int64_t overflow"); } else { @@ -201,7 +203,9 @@ Duration::operator*(double scale) const this->rcl_duration_.nanoseconds, scale, std::numeric_limits::max()); - return Duration(static_cast(rcl_duration_.nanoseconds * scale)); + long double scale_ld = static_cast(scale); + return Duration(static_cast( + static_cast(rcl_duration_.nanoseconds) * scale_ld)); } rcl_duration_value_t @@ -228,8 +232,8 @@ Duration::to_rmw_time() const // reuse conversion logic from msg creation builtin_interfaces::msg::Duration msg = *this; rmw_time_t result; - result.sec = msg.sec; - result.nsec = msg.nanosec; + result.sec = static_cast(msg.sec); + result.nsec = static_cast(msg.nanosec); return result; } diff --git a/rclcpp/src/rclcpp/parameter_client.cpp b/rclcpp/src/rclcpp/parameter_client.cpp index 8afbf43726..4b0372301a 100644 --- a/rclcpp/src/rclcpp/parameter_client.cpp +++ b/rclcpp/src/rclcpp/parameter_client.cpp @@ -152,7 +152,7 @@ AsyncParametersClient::get_parameters( auto & pvalues = cb_f.get()->values; for (auto & pvalue : pvalues) { - auto i = &pvalue - &pvalues[0]; + auto i = static_cast(&pvalue - &pvalues[0]); rcl_interfaces::msg::Parameter parameter; parameter.name = request->names[i]; parameter.value = pvalue; diff --git a/rclcpp/src/rclcpp/parameter_value.cpp b/rclcpp/src/rclcpp/parameter_value.cpp index 79c1e13e74..7825eb417c 100644 --- a/rclcpp/src/rclcpp/parameter_value.cpp +++ b/rclcpp/src/rclcpp/parameter_value.cpp @@ -154,7 +154,7 @@ ParameterValue::ParameterValue(const int64_t int_value) ParameterValue::ParameterValue(const float double_value) { - value_.double_value = double_value; + value_.double_value = static_cast(double_value); value_.type = rcl_interfaces::msg::ParameterType::PARAMETER_DOUBLE; } diff --git a/rclcpp/src/rclcpp/utilities.cpp b/rclcpp/src/rclcpp/utilities.cpp index 83008b4e53..4cbfd27aaa 100644 --- a/rclcpp/src/rclcpp/utilities.cpp +++ b/rclcpp/src/rclcpp/utilities.cpp @@ -103,9 +103,9 @@ remove_ros_arguments(int argc, char const * const argv[]) throw exceptions::RCLError(base_exc, ""); } - std::vector return_arguments(nonros_argc); + std::vector return_arguments(static_cast(nonros_argc)); - for (int ii = 0; ii < nonros_argc; ++ii) { + for (size_t ii = 0; ii < static_cast(nonros_argc); ++ii) { return_arguments[ii] = std::string(nonros_argv[ii]); } diff --git a/rclcpp/test/test_duration.cpp b/rclcpp/test/test_duration.cpp index b77f0467d4..b69a790d28 100644 --- a/rclcpp/test/test_duration.cpp +++ b/rclcpp/test/test_duration.cpp @@ -47,15 +47,15 @@ TEST(TestDuration, operators) { EXPECT_FALSE(young == old); rclcpp::Duration add = old + young; - EXPECT_EQ(add.nanoseconds(), (rcl_duration_value_t)(old.nanoseconds() + young.nanoseconds())); + EXPECT_EQ(add.nanoseconds(), old.nanoseconds() + young.nanoseconds()); EXPECT_EQ(add, old + young); rclcpp::Duration sub = young - old; - EXPECT_EQ(sub.nanoseconds(), (rcl_duration_value_t)(young.nanoseconds() - old.nanoseconds())); + EXPECT_EQ(sub.nanoseconds(), young.nanoseconds() - old.nanoseconds()); EXPECT_EQ(sub, young - old); rclcpp::Duration scale = old * 3; - EXPECT_EQ(scale.nanoseconds(), (rcl_duration_value_t)(old.nanoseconds() * 3)); + EXPECT_EQ(scale.nanoseconds(), old.nanoseconds() * 3); rclcpp::Duration time = rclcpp::Duration(0, 0); rclcpp::Duration copy_constructor_duration(time); diff --git a/rclcpp/test/test_time.cpp b/rclcpp/test/test_time.cpp index cc5cc328c5..773819ab88 100644 --- a/rclcpp/test/test_time.cpp +++ b/rclcpp/test/test_time.cpp @@ -123,7 +123,7 @@ TEST(TestTime, operators) { EXPECT_TRUE(young != old); rclcpp::Duration sub = young - old; - EXPECT_EQ(sub.nanoseconds(), (rcl_duration_value_t)(young.nanoseconds() - old.nanoseconds())); + EXPECT_EQ(sub.nanoseconds(), (young.nanoseconds() - old.nanoseconds())); EXPECT_EQ(sub, young - old); rclcpp::Time young_changed(young); diff --git a/rclcpp_lifecycle/src/lifecycle_node_interface_impl.hpp b/rclcpp_lifecycle/src/lifecycle_node_interface_impl.hpp index 5dc81e9629..9fc21e5171 100644 --- a/rclcpp_lifecycle/src/lifecycle_node_interface_impl.hpp +++ b/rclcpp_lifecycle/src/lifecycle_node_interface_impl.hpp @@ -224,7 +224,7 @@ class LifecycleNode::LifecycleNodeInterfaceImpl resp->success = false; return; } - transition_id = rcl_transition->id; + transition_id = static_cast(rcl_transition->id); } node_interfaces::LifecycleNodeInterface::CallbackReturn cb_return_code; @@ -289,11 +289,11 @@ class LifecycleNode::LifecycleNodeInterfaceImpl for (uint8_t i = 0; i < state_machine_.current_state->valid_transition_size; ++i) { auto rcl_transition = state_machine_.current_state->valid_transitions[i]; lifecycle_msgs::msg::TransitionDescription trans_desc; - trans_desc.transition.id = rcl_transition.id; + trans_desc.transition.id = static_cast(rcl_transition.id); trans_desc.transition.label = rcl_transition.label; - trans_desc.start_state.id = rcl_transition.start->id; + trans_desc.start_state.id = static_cast(rcl_transition.start->id); trans_desc.start_state.label = rcl_transition.start->label; - trans_desc.goal_state.id = rcl_transition.goal->id; + trans_desc.goal_state.id = static_cast(rcl_transition.goal->id); trans_desc.goal_state.label = rcl_transition.goal->label; resp->available_transitions.push_back(trans_desc); } @@ -315,11 +315,11 @@ class LifecycleNode::LifecycleNodeInterfaceImpl for (uint8_t i = 0; i < state_machine_.transition_map.transitions_size; ++i) { auto rcl_transition = state_machine_.transition_map.transitions[i]; lifecycle_msgs::msg::TransitionDescription trans_desc; - trans_desc.transition.id = rcl_transition.id; + trans_desc.transition.id = static_cast(rcl_transition.id); trans_desc.transition.label = rcl_transition.label; - trans_desc.start_state.id = rcl_transition.start->id; + trans_desc.start_state.id = static_cast(rcl_transition.start->id); trans_desc.start_state.label = rcl_transition.start->label; - trans_desc.goal_state.id = rcl_transition.goal->id; + trans_desc.goal_state.id = static_cast(rcl_transition.goal->id); trans_desc.goal_state.label = rcl_transition.goal->label; resp->available_transitions.push_back(trans_desc); } @@ -434,7 +434,7 @@ class LifecycleNode::LifecycleNodeInterfaceImpl // in case no callback was attached, we forward directly auto cb_success = node_interfaces::LifecycleNodeInterface::CallbackReturn::SUCCESS; - auto it = cb_map_.find(cb_id); + auto it = cb_map_.find(static_cast(cb_id)); if (it != cb_map_.end()) { auto callback = it->second; try { @@ -460,7 +460,7 @@ class LifecycleNode::LifecycleNodeInterfaceImpl auto transition = rcl_lifecycle_get_transition_by_label(state_machine_.current_state, transition_label); if (transition) { - change_state(transition->id, cb_return_code); + change_state(static_cast(transition->id), cb_return_code); } return get_current_state(); } diff --git a/rclcpp_lifecycle/src/state.cpp b/rclcpp_lifecycle/src/state.cpp index a84f1d301f..7024a82f88 100644 --- a/rclcpp_lifecycle/src/state.cpp +++ b/rclcpp_lifecycle/src/state.cpp @@ -130,7 +130,7 @@ State::id() const if (!state_handle_) { throw std::runtime_error("Error in state! Internal state_handle is NULL."); } - return state_handle_->id; + return static_cast(state_handle_->id); } std::string diff --git a/rclcpp_lifecycle/src/transition.cpp b/rclcpp_lifecycle/src/transition.cpp index d7bae6c280..cb7d471052 100644 --- a/rclcpp_lifecycle/src/transition.cpp +++ b/rclcpp_lifecycle/src/transition.cpp @@ -213,7 +213,7 @@ Transition::id() const if (!transition_handle_) { throw std::runtime_error("internal transition_handle is null"); } - return transition_handle_->id; + return static_cast(transition_handle_->id); } std::string