Skip to content

Commit 7c015c1

Browse files
committed
Drop generation C++->RCL allocator conversion.
As explained in #1254, there's conceptually no way to implement RCL allocators in terms of C++ allocators. Since allocator_common can't be fixed, delete it and rely on implementers to provide a suitable specialization. Since there's other blocking bugs for allocator use (#1061), this shouldn't break existing users.
1 parent 2531787 commit 7c015c1

File tree

5 files changed

+21
-24
lines changed

5 files changed

+21
-24
lines changed

rclcpp/include/rclcpp/allocator/allocator_common.hpp

+12-19
Original file line numberDiff line numberDiff line change
@@ -39,61 +39,54 @@ void * retyped_allocate(size_t size, void * untyped_allocator)
3939
return std::allocator_traits<Alloc>::allocate(*typed_allocator, size);
4040
}
4141

42-
template<typename T, typename Alloc>
42+
template<typename Alloc>
4343
void retyped_deallocate(void * untyped_pointer, void * untyped_allocator)
4444
{
4545
auto typed_allocator = static_cast<Alloc *>(untyped_allocator);
4646
if (!typed_allocator) {
4747
throw std::runtime_error("Received incorrect allocator type");
4848
}
49-
auto typed_ptr = static_cast<T *>(untyped_pointer);
49+
auto typed_ptr = static_cast<typename Alloc::value_type *>(untyped_pointer);
5050
std::allocator_traits<Alloc>::deallocate(*typed_allocator, typed_ptr, 1);
5151
}
5252

53-
template<typename T, typename Alloc>
53+
template<typename Alloc>
5454
void * retyped_reallocate(void * untyped_pointer, size_t size, void * untyped_allocator)
5555
{
5656
auto typed_allocator = static_cast<Alloc *>(untyped_allocator);
5757
if (!typed_allocator) {
5858
throw std::runtime_error("Received incorrect allocator type");
5959
}
60-
auto typed_ptr = static_cast<T *>(untyped_pointer);
60+
auto typed_ptr = static_cast<typename Alloc::value_type *>(untyped_pointer);
6161
std::allocator_traits<Alloc>::deallocate(*typed_allocator, typed_ptr, 1);
6262
return std::allocator_traits<Alloc>::allocate(*typed_allocator, size);
6363
}
6464

65+
} // namespace allocator
6566

6667
// Convert a std::allocator_traits-formatted Allocator into an rcl allocator
67-
template<
68-
typename T,
69-
typename Alloc,
70-
typename std::enable_if<!std::is_same<Alloc, std::allocator<void>>::value>::type * = nullptr>
68+
template<typename Alloc>
7169
rcl_allocator_t get_rcl_allocator(Alloc & allocator)
7270
{
7371
rcl_allocator_t rcl_allocator = rcl_get_default_allocator();
7472
#ifndef _WIN32
75-
rcl_allocator.allocate = &retyped_allocate<Alloc>;
76-
rcl_allocator.deallocate = &retyped_deallocate<T, Alloc>;
77-
rcl_allocator.reallocate = &retyped_reallocate<T, Alloc>;
73+
rcl_allocator.allocate = &allocator::retyped_allocate<Alloc>;
74+
rcl_allocator.deallocate = &allocator::retyped_deallocate<Alloc>;
75+
rcl_allocator.reallocate = &allocator::retyped_reallocate<Alloc>;
7876
rcl_allocator.state = &allocator;
7977
#else
8078
(void)allocator; // Remove warning
8179
#endif
8280
return rcl_allocator;
8381
}
8482

85-
// TODO(jacquelinekay) Workaround for an incomplete implementation of std::allocator<void>
86-
template<
87-
typename T,
88-
typename Alloc,
89-
typename std::enable_if<std::is_same<Alloc, std::allocator<void>>::value>::type * = nullptr>
90-
rcl_allocator_t get_rcl_allocator(Alloc & allocator)
83+
template <typename T>
84+
rcl_allocator_t get_rcl_allocator(std::allocator<T>& allocator)
9185
{
92-
(void)allocator;
86+
(void)allocator; // Remove warning
9387
return rcl_get_default_allocator();
9488
}
9589

96-
} // namespace allocator
9790
} // namespace rclcpp
9891

9992
#endif // RCLCPP__ALLOCATOR__ALLOCATOR_COMMON_HPP_

rclcpp/include/rclcpp/message_memory_strategy.hpp

+2-2
Original file line numberDiff line numberDiff line change
@@ -61,15 +61,15 @@ class MessageMemoryStrategy
6161
message_allocator_ = std::make_shared<MessageAlloc>();
6262
serialized_message_allocator_ = std::make_shared<SerializedMessageAlloc>();
6363
buffer_allocator_ = std::make_shared<BufferAlloc>();
64-
rcutils_allocator_ = allocator::get_rcl_allocator<char, BufferAlloc>(*buffer_allocator_.get());
64+
rcutils_allocator_ = get_rcl_allocator(*buffer_allocator_.get());
6565
}
6666

6767
explicit MessageMemoryStrategy(std::shared_ptr<Alloc> allocator)
6868
{
6969
message_allocator_ = std::make_shared<MessageAlloc>(*allocator.get());
7070
serialized_message_allocator_ = std::make_shared<SerializedMessageAlloc>(*allocator.get());
7171
buffer_allocator_ = std::make_shared<BufferAlloc>(*allocator.get());
72-
rcutils_allocator_ = allocator::get_rcl_allocator<char, BufferAlloc>(*buffer_allocator_.get());
72+
rcutils_allocator_ = get_rcl_allocator(*buffer_allocator_.get());
7373
}
7474

7575
virtual ~MessageMemoryStrategy() = default;

rclcpp/include/rclcpp/publisher_options.hpp

+3-1
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,9 @@ struct PublisherOptionsWithAllocator : public PublisherOptionsBase
7575
using AllocatorTraits = std::allocator_traits<Allocator>;
7676
using MessageAllocatorT = typename AllocatorTraits::template rebind_alloc<MessageT>;
7777
auto message_alloc = std::make_shared<MessageAllocatorT>(*this->get_allocator().get());
78-
result.allocator = rclcpp::allocator::get_rcl_allocator<MessageT>(*message_alloc);
78+
// TODO: This is likely to invoke undefined behavior - message_alloc goes out of scope
79+
// at the end of this function, but the allocator doesn't.
80+
result.allocator = get_rcl_allocator(*message_alloc);
7981
result.qos = qos.get_rmw_qos_profile();
8082

8183
// Apply payload to rcl_publisher_options if necessary.

rclcpp/include/rclcpp/strategies/allocator_memory_strategy.hpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -437,7 +437,7 @@ class AllocatorMemoryStrategy : public memory_strategy::MemoryStrategy
437437

438438
rcl_allocator_t get_allocator() override
439439
{
440-
return rclcpp::allocator::get_rcl_allocator<void *, VoidAlloc>(*allocator_.get());
440+
return get_rcl_allocator(*allocator_.get());
441441
}
442442

443443
size_t number_of_ready_subscriptions() const override

rclcpp/include/rclcpp/subscription_options.hpp

+3-1
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,9 @@ struct SubscriptionOptionsWithAllocator : public SubscriptionOptionsBase
102102
using AllocatorTraits = std::allocator_traits<Allocator>;
103103
using MessageAllocatorT = typename AllocatorTraits::template rebind_alloc<MessageT>;
104104
auto message_alloc = std::make_shared<MessageAllocatorT>(*allocator.get());
105-
result.allocator = allocator::get_rcl_allocator<MessageT>(*message_alloc);
105+
// TODO: This is likely to invoke undefined behavior - message_alloc goes out of scope
106+
// at the end of this function, but the allocator doesn't.
107+
result.allocator = get_rcl_allocator(*message_alloc);
106108
result.qos = qos.get_rmw_qos_profile();
107109
result.rmw_subscription_options.ignore_local_publications = this->ignore_local_publications;
108110

0 commit comments

Comments
 (0)