From 9f65b9dd59cd9e1fc28fd1afa89504b5e23dc95f Mon Sep 17 00:00:00 2001 From: Esteve Fernandez Date: Mon, 12 Oct 2015 11:55:33 -0700 Subject: [PATCH 1/4] Added tests for rclcpp::function_traits and work around expression SFINAE --- rclcpp/CMakeLists.txt | 1 + .../rclcpp/any_subscription_callback.hpp | 52 +-- rclcpp/include/rclcpp/function_traits.hpp | 6 + rclcpp/include/rclcpp/node.hpp | 62 ++-- rclcpp/test/test_function_traits.cpp | 328 ++++++++++++++++++ 5 files changed, 390 insertions(+), 59 deletions(-) create mode 100644 rclcpp/test/test_function_traits.cpp diff --git a/rclcpp/CMakeLists.txt b/rclcpp/CMakeLists.txt index 14ea23d..b6fec7d 100644 --- a/rclcpp/CMakeLists.txt +++ b/rclcpp/CMakeLists.txt @@ -22,6 +22,7 @@ if(AMENT_ENABLE_TESTING) include_directories(include) + ament_add_gtest(test_function_traits test/test_function_traits.cpp) ament_add_gtest(test_mapped_ring_buffer test/test_mapped_ring_buffer.cpp) ament_add_gtest(test_intra_process_manager test/test_intra_process_manager.cpp) if(TARGET test_intra_process_manager) diff --git a/rclcpp/include/rclcpp/any_subscription_callback.hpp b/rclcpp/include/rclcpp/any_subscription_callback.hpp index 507d3ac..c54a8b8 100644 --- a/rclcpp/include/rclcpp/any_subscription_callback.hpp +++ b/rclcpp/include/rclcpp/any_subscription_callback.hpp @@ -57,34 +57,42 @@ struct AnySubscriptionCallback AnySubscriptionCallback(const AnySubscriptionCallback &) = default; - template::arity == 1 - >::type * = nullptr, - typename std::enable_if< - std::is_same< - typename function_traits::template argument_type<0>, - typename std::shared_ptr - >::value - >::type * = nullptr + template< + typename CallbackT, + std::size_t Arity = 1 > - void set(CallbackT callback) + typename std::enable_if::value, void>::type + set( + CallbackT callback, + typename std::enable_if< + std::is_same< + typename function_traits::template argument_type<0>, + typename std::shared_ptr + >::value + >::type * = nullptr) { shared_ptr_callback = callback; } - template::arity == 2 - >::type * = nullptr, - typename std::enable_if< - std::is_same< - typename function_traits::template argument_type<0>, - typename std::shared_ptr - >::value - >::type * = nullptr + template< + typename CallbackT, + std::size_t Arity = 2 > - void set(CallbackT callback) + typename std::enable_if::value, void>::type + set( + CallbackT callback, + typename std::enable_if< + std::is_same< + typename function_traits::template argument_type<0>, + typename std::shared_ptr + >::value + >::type * = nullptr, + typename std::enable_if< + std::is_same< + typename function_traits::template argument_type<1>, + rmw_message_info_t + >::value + >::type * = nullptr) { static_assert(std::is_same< typename function_traits::template argument_type<1>, diff --git a/rclcpp/include/rclcpp/function_traits.hpp b/rclcpp/include/rclcpp/function_traits.hpp index 0a82783..d0b96be 100644 --- a/rclcpp/include/rclcpp/function_traits.hpp +++ b/rclcpp/include/rclcpp/function_traits.hpp @@ -58,6 +58,12 @@ struct function_traits : public function_traits {}; +template +struct arity_comparator +{ + static constexpr bool value = (Arity == function_traits::arity); +}; + } /* namespace rclcpp */ #endif /* RCLCPP_RCLCPP_FUNCTION_TRAITS_HPP_ */ diff --git a/rclcpp/include/rclcpp/node.hpp b/rclcpp/include/rclcpp/node.hpp index ed38731..58bd8e7 100644 --- a/rclcpp/include/rclcpp/node.hpp +++ b/rclcpp/include/rclcpp/node.hpp @@ -250,9 +250,17 @@ private: template< typename ServiceT, typename FunctorT, - typename std::enable_if< - function_traits::arity == 2 - >::type * = nullptr, + std::size_t Arity = 2 + > + typename std::enable_if< + rclcpp::arity_comparator::value, + typename rclcpp::service::Service::SharedPtr + >::type + create_service_internal( + std::shared_ptr node_handle, + rmw_service_t * service_handle, + const std::string & service_name, + FunctorT callback, typename std::enable_if< std::is_same< typename function_traits::template argument_type<0>, @@ -264,14 +272,7 @@ private: typename function_traits::template argument_type<1>, typename std::shared_ptr >::value - >::type * = nullptr - > - typename rclcpp::service::Service::SharedPtr - create_service_internal( - std::shared_ptr node_handle, - rmw_service_t * service_handle, - const std::string & service_name, - FunctorT callback) + >::type * = nullptr) { typename rclcpp::service::Service::CallbackType callback_without_header = callback; @@ -282,9 +283,17 @@ private: template< typename ServiceT, typename FunctorT, - typename std::enable_if< - function_traits::arity == 3 - >::type * = nullptr, + std::size_t Arity = 3 + > + typename std::enable_if< + arity_comparator::value, + typename rclcpp::service::Service::SharedPtr + >::type + create_service_internal( + std::shared_ptr node_handle, + rmw_service_t * service_handle, + const std::string & service_name, + FunctorT callback, typename std::enable_if< std::is_same< typename function_traits::template argument_type<0>, @@ -296,35 +305,14 @@ private: typename function_traits::template argument_type<1>, typename std::shared_ptr >::value - >::type * = nullptr -/* - TODO(esteve): reenable this block of code when VS2015 gets better support - for SFINAE and remove the static_assert from the body of this method. For - more info about the current support for SFINAE in VS2015 RC: - - http://blogs.msdn.com/b/vcblog/archive/2015/04/29/c-11-14-17-features-in-vs-2015-rc.aspx - , + >::type * = nullptr, typename std::enable_if< std::is_same< typename function_traits::template argument_type<2>, typename std::shared_ptr >::value - >::type * = nullptr - */ - > - typename rclcpp::service::Service::SharedPtr - create_service_internal( - std::shared_ptr node_handle, - rmw_service_t * service_handle, - const std::string & service_name, - FunctorT callback) + >::type * = nullptr) { - static_assert( - std::is_same< - typename function_traits::template argument_type<2>, - typename std::shared_ptr - >::value, "Third argument must be of type std::shared_ptr"); - typename rclcpp::service::Service::CallbackWithHeaderType callback_with_header = callback; return service::Service::make_shared( diff --git a/rclcpp/test/test_function_traits.cpp b/rclcpp/test/test_function_traits.cpp new file mode 100644 index 0000000..57eb631 --- /dev/null +++ b/rclcpp/test/test_function_traits.cpp @@ -0,0 +1,328 @@ +// Copyright 2015 Open Source Robotics Foundation, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include + +#include + +int func_no_args() +{ + return 0; +} + +int func_one_int(int) +{ + return 1; +} + +int func_two_ints(int, int) +{ + return 2; +} + +int func_one_int_one_char(int, char) +{ + return 3; +} + +struct FunctionObjectNoArgs +{ + int operator()() const + { + return 0; + } +}; + +struct FunctionObjectOneInt +{ + int operator()(int) const + { + return 1; + } +}; + +struct FunctionObjectTwoInts +{ + int operator()(int, int) const + { + return 2; + } +}; + +struct FunctionObjectOneIntOneChar +{ + int operator()(int, char) const + { + return 3; + } +}; + +template< + typename FunctorT, + std::size_t Arity = 1 +> +typename std::enable_if::value, int>::type +func_accept_callback(FunctorT callback, + typename std::enable_if< + std::is_same< + int, + typename rclcpp::function_traits::template argument_type<0> + >::value + >::type * = nullptr +) +{ + int a = 4; + return callback(a); +} + +template< + typename FunctorT, + std::size_t Arity = 2 +> +typename std::enable_if::value, int>::type +func_accept_callback(FunctorT callback, + typename std::enable_if< + std::is_same< + int, + typename rclcpp::function_traits::template argument_type<0> + >::value + >::type * = nullptr, + typename std::enable_if< + std::is_same< + int, + typename rclcpp::function_traits::template argument_type<1> + >::value + >::type * = nullptr) +{ + int a = 5; + int b = 6; + return callback(a, b); +} + +template< + typename FunctorT, + std::size_t Arity = 2 +> +typename std::enable_if::value, int>::type +func_accept_callback(FunctorT callback, + typename std::enable_if< + std::is_same< + int, + typename rclcpp::function_traits::template argument_type<0> + >::value + >::type * = nullptr, + typename std::enable_if< + std::is_same< + char, + typename rclcpp::function_traits::template argument_type<1> + >::value + >::type * = nullptr) +{ + int a = 7; + char b = 8; + return callback(a, b); +} + +/* + Tests that funcion_traits calculates arity of several functors. + */ +TEST(TestFunctionTraits, arity) { + // Test regular functions + static_assert( + rclcpp::function_traits::arity == 0, + "Functor does not accept arguments"); + + static_assert( + rclcpp::function_traits::arity == 1, + "Functor only accepts one argument"); + + static_assert( + rclcpp::function_traits::arity == 2, + "Functor only accepts two arguments"); + + static_assert( + rclcpp::function_traits::arity == 2, + "Functor only accepts two arguments"); + + // Test lambdas + auto lambda_no_args = []() { + return 0; + }; + + auto lambda_one_int = [](int) { + return 1; + }; + + auto lambda_two_ints = [](int, int) { + return 2; + }; + + auto lambda_one_int_one_char = [](int, char) { + return 3; + }; + + static_assert( + rclcpp::function_traits::arity == 0, + "Functor does not accept arguments"); + + static_assert( + rclcpp::function_traits::arity == 1, + "Functor only accepts one argument"); + + static_assert( + rclcpp::function_traits::arity == 2, + "Functor only accepts two arguments"); + + static_assert( + rclcpp::function_traits::arity == 2, + "Functor only accepts two arguments"); + + // Test objects that have a call operator + static_assert( + rclcpp::function_traits::arity == 0, + "Functor does not accept arguments"); + + static_assert( + rclcpp::function_traits::arity == 1, + "Functor only accepts one argument"); + + static_assert( + rclcpp::function_traits::arity == 2, + "Functor only accepts two arguments"); + + static_assert( + rclcpp::function_traits::arity == 2, + "Functor only accepts two arguments"); +} + +/* + Tests that funcion_traits deducts the type of the arguments of several functors. + */ +TEST(TestFunctionTraits, argument_types) { + // Test regular functions + static_assert( + std::is_same< + int, + rclcpp::function_traits::template argument_type<0> + >::value, "Functor accepts an int as first argument"); + + static_assert( + std::is_same< + int, + rclcpp::function_traits::template argument_type<0> + >::value, "Functor accepts an int as first argument"); + + static_assert( + std::is_same< + int, + rclcpp::function_traits::template argument_type<1> + >::value, "Functor accepts an int as second argument"); + + static_assert( + std::is_same< + int, + rclcpp::function_traits::template argument_type<0> + >::value, "Functor accepts an int as first argument"); + + static_assert( + std::is_same< + char, + rclcpp::function_traits::template argument_type<1> + >::value, "Functor accepts a char as second argument"); + + // Test lambdas + auto lambda_one_int = [](int) { + return 1; + }; + + auto lambda_two_ints = [](int, int) { + return 2; + }; + + auto lambda_one_int_one_char = [](int, char) { + return 3; + }; + + static_assert( + std::is_same< + int, + rclcpp::function_traits::template argument_type<0> + >::value, "Functor accepts an int as first argument"); + + static_assert( + std::is_same< + int, + rclcpp::function_traits::template argument_type<0> + >::value, "Functor accepts an int as first argument"); + + static_assert( + std::is_same< + int, + rclcpp::function_traits::template argument_type<1> + >::value, "Functor accepts an int as second argument"); + + static_assert( + std::is_same< + int, + rclcpp::function_traits::template argument_type<0> + >::value, "Functor accepts an int as first argument"); + + static_assert( + std::is_same< + char, + rclcpp::function_traits::template argument_type<1> + >::value, "Functor accepts a char as second argument"); + + // Test objects that have a call operator + static_assert( + std::is_same< + int, + rclcpp::function_traits::template argument_type<0> + >::value, "Functor accepts an int as first argument"); + + static_assert( + std::is_same< + int, + rclcpp::function_traits::template argument_type<0> + >::value, "Functor accepts an int as first argument"); + + static_assert( + std::is_same< + int, + rclcpp::function_traits::template argument_type<1> + >::value, "Functor accepts an int as second argument"); + + static_assert( + std::is_same< + int, + rclcpp::function_traits::template argument_type<0> + >::value, "Functor accepts an int as first argument"); + + static_assert( + std::is_same< + char, + rclcpp::function_traits::template argument_type<1> + >::value, "Functor accepts a char as second argument"); +} + +/* + Tests that functions are matched via SFINAE. + */ +TEST(TestFunctionTraits, sfinae_match) { + EXPECT_EQ(1, func_accept_callback(func_one_int)); + + EXPECT_EQ(2, func_accept_callback(func_two_ints)); + + EXPECT_EQ(3, func_accept_callback(func_one_int_one_char)); +} From c5455becebf48d691839a273400b026045d7afe9 Mon Sep 17 00:00:00 2001 From: Esteve Fernandez Date: Mon, 12 Oct 2015 17:31:48 -0700 Subject: [PATCH 2/4] Update callback signature --- rclcpp/include/rclcpp/any_subscription_callback.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rclcpp/include/rclcpp/any_subscription_callback.hpp b/rclcpp/include/rclcpp/any_subscription_callback.hpp index c54a8b8..a205d2f 100644 --- a/rclcpp/include/rclcpp/any_subscription_callback.hpp +++ b/rclcpp/include/rclcpp/any_subscription_callback.hpp @@ -90,7 +90,7 @@ struct AnySubscriptionCallback typename std::enable_if< std::is_same< typename function_traits::template argument_type<1>, - rmw_message_info_t + const rmw_message_info_t & >::value >::type * = nullptr) { From 3294098602904b2efd650bce0cf1c4894555d111 Mon Sep 17 00:00:00 2001 From: Esteve Fernandez Date: Tue, 13 Oct 2015 10:49:15 -0700 Subject: [PATCH 3/4] Added comments about the SFINAE voodoo --- rclcpp/include/rclcpp/function_traits.hpp | 5 +++++ rclcpp/include/rclcpp/node.hpp | 10 ++++++++++ 2 files changed, 15 insertions(+) diff --git a/rclcpp/include/rclcpp/function_traits.hpp b/rclcpp/include/rclcpp/function_traits.hpp index d0b96be..7c64787 100644 --- a/rclcpp/include/rclcpp/function_traits.hpp +++ b/rclcpp/include/rclcpp/function_traits.hpp @@ -25,6 +25,7 @@ namespace rclcpp * but unfortunately std::function's constructor on VS2015 is too greedy, * so we need a mechanism for checking the arity and the type of each argument * in a callback function. + * See http://blogs.msdn.com/b/vcblog/archive/2015/06/19/c-11-14-17-features-in-vs-2015-rtm.aspx */ @@ -58,6 +59,10 @@ struct function_traits : public function_traits {}; +/* NOTE(esteve): + * VS2015 does not support expression SFINAE, so we're using this template to evaluate + * the arity of a function. + */ template struct arity_comparator { diff --git a/rclcpp/include/rclcpp/node.hpp b/rclcpp/include/rclcpp/node.hpp index 58bd8e7..b90631f 100644 --- a/rclcpp/include/rclcpp/node.hpp +++ b/rclcpp/include/rclcpp/node.hpp @@ -247,6 +247,16 @@ private: bool ignore_local_publications, typename message_memory_strategy::MessageMemoryStrategy::SharedPtr msg_mem_strat); +/* NOTE(esteve): + * The following template machinery works around VS2015's lack of support for expression SFINAE: + * - We first declare the arity we want to match, i.e. 2 or 3. + * - Then we use the arity_comparator template to SFINAE on the arity of the passed functor. + * - Lastly, we SFINAE on the types of the arguments of the functor. + * These steps happen in different parts of the function signature because we want to stagger + * instantation of the templates because VS2015 can't conditionally enable templates that depend + * on another template. + * See test_function_traits.cpp for streamlined examples of how to use this pattern. + */ template< typename ServiceT, typename FunctorT, From ea9d3306d7ae1a3cde2b4838b3af73abc9cf7d60 Mon Sep 17 00:00:00 2001 From: Esteve Fernandez Date: Tue, 13 Oct 2015 11:57:05 -0700 Subject: [PATCH 4/4] Update const callbacks --- .../rclcpp/any_subscription_callback.hpp | 60 +++++++++---------- 1 file changed, 30 insertions(+), 30 deletions(-) diff --git a/rclcpp/include/rclcpp/any_subscription_callback.hpp b/rclcpp/include/rclcpp/any_subscription_callback.hpp index a205d2f..6949b7e 100644 --- a/rclcpp/include/rclcpp/any_subscription_callback.hpp +++ b/rclcpp/include/rclcpp/any_subscription_callback.hpp @@ -94,46 +94,46 @@ struct AnySubscriptionCallback >::value >::type * = nullptr) { - static_assert(std::is_same< - typename function_traits::template argument_type<1>, - const rmw_message_info_t &>::value, - "Passed incorrect argument type to callback, should be rmw_message_info_t"); shared_ptr_with_info_callback = callback; } - template::arity == 1 - >::type * = nullptr, - typename std::enable_if< - std::is_same< - typename function_traits::template argument_type<0>, - typename std::shared_ptr - >::value - >::type * = nullptr + template< + typename CallbackT, + std::size_t Arity = 1 > - void set(CallbackT callback) + typename std::enable_if::value, void>::type + set( + CallbackT callback, + typename std::enable_if< + std::is_same< + typename function_traits::template argument_type<0>, + typename std::shared_ptr + >::value + >::type * = nullptr) { const_shared_ptr_callback = callback; } - template::arity == 2 - >::type * = nullptr, - typename std::enable_if< - std::is_same< - typename function_traits::template argument_type<0>, - typename std::shared_ptr - >::value - >::type * = nullptr + template< + typename CallbackT, + std::size_t Arity = 2 > - void set(CallbackT callback) - { - static_assert(std::is_same< + typename std::enable_if::value, void>::type + set( + CallbackT callback, + typename std::enable_if< + std::is_same< + typename function_traits::template argument_type<0>, + typename std::shared_ptr + >::value + >::type * = nullptr, + typename std::enable_if< + std::is_same< typename function_traits::template argument_type<1>, - const rmw_message_info_t &>::value, - "Passed incorrect argument type to callback, should be rmw_message_info_t"); + const rmw_message_info_t & + >::value + >::type * = nullptr) + { const_shared_ptr_with_info_callback = callback; } /*