Make get_rcl_allocator a function family

As explained in #1254, there's conceptually no way to implement RCL
allocators in terms of C++ allocators. In order to fix this behavior,
we have to delete the generic version of get_rcl_allocator. Since some
ROS code depends on this, we need to allow users to write their own
version of get_rcl_allocator for allocators that support the C-style
interface (most do).

So this CL changes get_rcl_allocator from a template function
into a family of (potentially templated) functions, which allows
users to add their own overloads and rely on the "most specialized"
mechanism for function specialization to select the right one. See
http://www.gotw.ca/publications/mill17.htm for details. This also
allows us to return get_rcl_default_allocator for all specializations
of std::allocator (previously, only for std::allocator), which will
already fix #1254 for pretty much all clients. I'll continue to work
on deleting the generic version, though, to make sure that nobody is
accidentally bitten by it.

I've tried to test this by doing a full ROS compilation following the
Dockerfile of the source Docker image, and all packages compile.

Signed-off-by: Steve Wolter <swolter@google.com>

Author: stevewolter

rclcpp/include/rclcpp/allocator/allocator_common.hpp

@@ -39,61 +39,54 @@ void * retyped_allocate(size_t size, void * untyped_allocator)
   return std::allocator_traits<Alloc>::allocate(*typed_allocator, size);
 }
 
-template<typename T, typename Alloc>
+template<typename Alloc>
 void retyped_deallocate(void * untyped_pointer, void * untyped_allocator)
 {
   auto typed_allocator = static_cast<Alloc *>(untyped_allocator);
   if (!typed_allocator) {
     throw std::runtime_error("Received incorrect allocator type");
   }
-  auto typed_ptr = static_cast<T *>(untyped_pointer);
+  auto typed_ptr = static_cast<typename Alloc::value_type *>(untyped_pointer);
   std::allocator_traits<Alloc>::deallocate(*typed_allocator, typed_ptr, 1);
 }
 
-template<typename T, typename Alloc>
+template<typename Alloc>
 void * retyped_reallocate(void * untyped_pointer, size_t size, void * untyped_allocator)
 {
   auto typed_allocator = static_cast<Alloc *>(untyped_allocator);
   if (!typed_allocator) {
     throw std::runtime_error("Received incorrect allocator type");
   }
-  auto typed_ptr = static_cast<T *>(untyped_pointer);
+  auto typed_ptr = static_cast<typename Alloc::value_type *>(untyped_pointer);
   std::allocator_traits<Alloc>::deallocate(*typed_allocator, typed_ptr, 1);
   return std::allocator_traits<Alloc>::allocate(*typed_allocator, size);
 }
 
+}  // namespace allocator
 
 // Convert a std::allocator_traits-formatted Allocator into an rcl allocator
-template<
-  typename T,
-  typename Alloc,
-  typename std::enable_if<!std::is_same<Alloc, std::allocator<void>>::value>::type * = nullptr>
+template<typename Alloc>
 rcl_allocator_t get_rcl_allocator(Alloc & allocator)
 {
   rcl_allocator_t rcl_allocator = rcl_get_default_allocator();
 #ifndef _WIN32
-  rcl_allocator.allocate = &retyped_allocate<Alloc>;
-  rcl_allocator.deallocate = &retyped_deallocate<T, Alloc>;
-  rcl_allocator.reallocate = &retyped_reallocate<T, Alloc>;
+  rcl_allocator.allocate = &allocator::retyped_allocate<Alloc>;
+  rcl_allocator.deallocate = &allocator::retyped_deallocate<Alloc>;
+  rcl_allocator.reallocate = &allocator::retyped_reallocate<Alloc>;
   rcl_allocator.state = &allocator;
 #else
   (void)allocator;  // Remove warning
 #endif
   return rcl_allocator;
 }
 
-// TODO(jacquelinekay) Workaround for an incomplete implementation of std::allocator<void>
-template<
-  typename T,
-  typename Alloc,
-  typename std::enable_if<std::is_same<Alloc, std::allocator<void>>::value>::type * = nullptr>
-rcl_allocator_t get_rcl_allocator(Alloc & allocator)
+template <typename T>
+rcl_allocator_t get_rcl_allocator(std::allocator<T>& allocator)
 {
-  (void)allocator;
+  (void)allocator;  // Remove warning
   return rcl_get_default_allocator();
 }
 
-}  // namespace allocator
 }  // namespace rclcpp
 
 #endif  // RCLCPP__ALLOCATOR__ALLOCATOR_COMMON_HPP_

rclcpp/include/rclcpp/message_memory_strategy.hpp

@@ -61,15 +61,15 @@ class MessageMemoryStrategy
     message_allocator_ = std::make_shared<MessageAlloc>();
     serialized_message_allocator_ = std::make_shared<SerializedMessageAlloc>();
     buffer_allocator_ = std::make_shared<BufferAlloc>();
-    rcutils_allocator_ = allocator::get_rcl_allocator<char, BufferAlloc>(*buffer_allocator_.get());
+    rcutils_allocator_ = get_rcl_allocator(*buffer_allocator_.get());
   }
 
   explicit MessageMemoryStrategy(std::shared_ptr<Alloc> allocator)
   {
     message_allocator_ = std::make_shared<MessageAlloc>(*allocator.get());
     serialized_message_allocator_ = std::make_shared<SerializedMessageAlloc>(*allocator.get());
     buffer_allocator_ = std::make_shared<BufferAlloc>(*allocator.get());
-    rcutils_allocator_ = allocator::get_rcl_allocator<char, BufferAlloc>(*buffer_allocator_.get());
+    rcutils_allocator_ = get_rcl_allocator(*buffer_allocator_.get());
   }
 
   virtual ~MessageMemoryStrategy() = default;

rclcpp/include/rclcpp/publisher_options.hpp

@@ -75,7 +75,9 @@ struct PublisherOptionsWithAllocator : public PublisherOptionsBase
     using AllocatorTraits = std::allocator_traits<Allocator>;
     using MessageAllocatorT = typename AllocatorTraits::template rebind_alloc<MessageT>;
     auto message_alloc = std::make_shared<MessageAllocatorT>(*this->get_allocator().get());
-    result.allocator = rclcpp::allocator::get_rcl_allocator<MessageT>(*message_alloc);
+    // TODO: This is likely to invoke undefined behavior - message_alloc goes out of scope
+    // at the end of this function, but the allocator doesn't.
+    result.allocator = get_rcl_allocator(*message_alloc);
     result.qos = qos.get_rmw_qos_profile();
 
     // Apply payload to rcl_publisher_options if necessary.

rclcpp/include/rclcpp/strategies/allocator_memory_strategy.hpp

@@ -437,7 +437,7 @@ class AllocatorMemoryStrategy : public memory_strategy::MemoryStrategy
 
   rcl_allocator_t get_allocator() override
   {
-    return rclcpp::allocator::get_rcl_allocator<void *, VoidAlloc>(*allocator_.get());
+    return get_rcl_allocator(*allocator_.get());
   }
 
   size_t number_of_ready_subscriptions() const override

rclcpp/include/rclcpp/subscription_options.hpp

@@ -102,7 +102,9 @@ struct SubscriptionOptionsWithAllocator : public SubscriptionOptionsBase
     using AllocatorTraits = std::allocator_traits<Allocator>;
     using MessageAllocatorT = typename AllocatorTraits::template rebind_alloc<MessageT>;
     auto message_alloc = std::make_shared<MessageAllocatorT>(*allocator.get());
-    result.allocator = allocator::get_rcl_allocator<MessageT>(*message_alloc);
+    // TODO: This is likely to invoke undefined behavior - message_alloc goes out of scope
+    // at the end of this function, but the allocator doesn't.
+    result.allocator = get_rcl_allocator(*message_alloc);
     result.qos = qos.get_rmw_qos_profile();
     result.rmw_subscription_options.ignore_local_publications = this->ignore_local_publications;