BoundedVector::data() is unusable

[thomasmoore-torc]

Bug report

Required Info:

• Operating System:
  • Ubuntu 20.04
• Installation type:
  • Source
• Version or commit hash:
  • 4.0.1
• DDS implementation:
  • Fast-RTPS
• Client library (if applicable):
  • rclcpp

Steps to reproduce issue

Consider the following Chatter.msg with a bounded array:

uint8[<=3145728] data

As the BoundedVector::data() methods are templated on the return type, deduction/substitution is not possible when attempting to store the returned pointer in a variable with auto storage:

chatter_interface::msg::Chatter chatter;
auto data = chatter.data.data();

This results in the the following errors:

chatter/talker.cc:33:34: error: no matching function for call to 'rosidl_runtime_cpp::BoundedVector<unsigned char, 3145728, std::allocator<unsigned char> >::data()'
   33 |     auto data = chatter.data.data();
      |                 ~~~~~~~~~~~~~~~~~^~
In file included from bazel-out/k8-opt/bin/chatter/rosidl_generator_cpp_app/chatter_interface/msg/detail/chatter__struct.hpp:14,
                 from bazel-out/k8-opt/bin/chatter/rosidl_generator_cpp_app/chatter_interface/msg/chatter.hpp:7,
                 from chatter/talker.cc:18:
external/ros2_rosidl/rosidl_runtime_cpp/include/rosidl_runtime_cpp/bounded_vector.hpp:418:3: note: candidate: 'template<class T, typename std::enable_if<((! std::is_same<T, unsigned char>::value) && (! std::is_same<T, bool>::value)), void>::type* <anonymous> > T* rosidl_runtime_cpp::BoundedVector<Tp, UpperBound, Alloc>::data() [with typename std::enable_if<((! std::is_same<T, Tp>::value) && (! std::is_same<T, bool>::value))>::type* <anonymous> = T; Tp = unsigned char; long unsigned int UpperBound = 3145728; Alloc = std::allocator<unsigned char>]'
  418 |   data() noexcept
      |   ^~~~
external/ros2_rosidl/rosidl_runtime_cpp/include/rosidl_runtime_cpp/bounded_vector.hpp:418:3: note:   template argument deduction/substitution failed:
chatter/talker.cc:33:34: note:   couldn't deduce template parameter 'T'
   33 |     auto data = chatter.data.data();
      |                 ~~~~~~~~~~~~~~~~~^~
external/ros2_rosidl/rosidl_runtime_cpp/include/rosidl_runtime_cpp/bounded_vector.hpp:431:3: note: candidate: 'template<class T, typename std::enable_if<((! std::is_same<T, unsigned char>::value) && (! std::is_same<T, bool>::value)), void>::type* <anonymous> > const T* rosidl_runtime_cpp::BoundedVector<Tp, UpperBound, Alloc>::data() const [with typename std::enable_if<((! std::is_same<T, Tp>::value) && (! std::is_same<T, bool>::value))>::type* <anonymous> = T; Tp = unsigned char; long unsigned int UpperBound = 3145728; Alloc = std::allocator<unsigned char>]'
  431 |   data() const noexcept
      |   ^~~~
external/ros2_rosidl/rosidl_runtime_cpp/include/rosidl_runtime_cpp/bounded_vector.hpp:431:3: note:   template argument deduction/substitution failed:
chatter/talker.cc:33:34: note:   couldn't deduce template parameter 'T'
   33 |     auto data = chatter.data.data();

If we then specify the return type via the template parameter:

chatter_interface::msg::Chatter chatter;
auto data = chatter.data.data<uint8_t>();

Even though we've specified the same type as the data member in the Chatter.msg, the BoundedVector::data() method is still not generated:

chatter/talker.cc:33:43: error: no matching function for call to 'rosidl_runtime_cpp::BoundedVector<unsigned char, 3145728, std::allocator<unsigned char> >::data<uint8_t>()'
   33 |     auto data = chatter.data.data<uint8_t>();
      |                 ~~~~~~~~~~~~~~~~~~~~~~~~~~^~
In file included from bazel-out/k8-opt/bin/chatter/rosidl_generator_cpp_app/chatter_interface/msg/detail/chatter__struct.hpp:14,
                 from bazel-out/k8-opt/bin/chatter/rosidl_generator_cpp_app/chatter_interface/msg/chatter.hpp:7,
                 from chatter/talker.cc:18:
external/ros2_rosidl/rosidl_runtime_cpp/include/rosidl_runtime_cpp/bounded_vector.hpp:418:3: note: candidate: 'template<class T, typename std::enable_if<((! std::is_same<T, unsigned char>::value) && (! std::is_same<T, bool>::value)), void>::type* <anonymous> > T* rosidl_runtime_cpp::BoundedVector<Tp, UpperBound, Alloc>::data() [with typename std::enable_if<((! std::is_same<T, Tp>::value) && (! std::is_same<T, bool>::value))>::type* <anonymous> = T; Tp = unsigned char; long unsigned int UpperBound = 3145728; Alloc = std::allocator<unsigned char>]'
  418 |   data() noexcept
      |   ^~~~
external/ros2_rosidl/rosidl_runtime_cpp/include/rosidl_runtime_cpp/bounded_vector.hpp:418:3: note:   template argument deduction/substitution failed:
external/ros2_rosidl/rosidl_runtime_cpp/include/rosidl_runtime_cpp/bounded_vector.hpp:415:17: error: no type named 'type' in 'struct std::enable_if<false, void>'
  415 |     >::type * = nullptr
      |                 ^~~~~~~
external/ros2_rosidl/rosidl_runtime_cpp/include/rosidl_runtime_cpp/bounded_vector.hpp:431:3: note: candidate: 'template<class T, typename std::enable_if<((! std::is_same<T, unsigned char>::value) && (! std::is_same<T, bool>::value)), void>::type* <anonymous> > const T* rosidl_runtime_cpp::BoundedVector<Tp, UpperBound, Alloc>::data() const [with typename std::enable_if<((! std::is_same<T, Tp>::value) && (! std::is_same<T, bool>::value))>::type* <anonymous> = T; Tp = unsigned char; long unsigned int UpperBound = 3145728; Alloc = std::allocator<unsigned char>]'
  431 |   data() const noexcept
      |   ^~~~
external/ros2_rosidl/rosidl_runtime_cpp/include/rosidl_runtime_cpp/bounded_vector.hpp:431:3: note:   template argument deduction/substitution failed:
external/ros2_rosidl/rosidl_runtime_cpp/include/rosidl_runtime_cpp/bounded_vector.hpp:428:17: error: no type named 'type' in 'struct std::enable_if<false, void>'
  428 |     >::type * = nullptr
      |                 ^~~~~~~

Interestingly, if we specify a different type for the template parameter:

chatter_interface::msg::Chatter chatter;
auto data = chatter.data.data<int8_t>();

The BoundedVector::data() method is generated, but results in another compilation error:

external/ros2_rosidl/rosidl_runtime_cpp/include/rosidl_runtime_cpp/bounded_vector.hpp:420:22: error: invalid conversion from 'unsigned char*' to 'signed char*' [-fpermissive]
  420 |     return Base::data();
      |            ~~~~~~~~~~^~
      |                      |
      |                      unsigned char*

This can be avoided by using void as the template parameter, but that's not particularly useful as it then requires the caller to cast the void pointer back to the actual type:

chatter_interface::msg::Chatter chatter;
uint8_t* data = static_cast<uint8_t*>(chatter.data.data<void>());

Expected behavior

According to section 6.12 of the OMG C++11 Language Mapping specification:

A bounded sequence is mapped to a distinct type to differentiate from an unbounded sequence. This distinct type must deliver std::vector semantics and support transparent conversion from bounded to unbounded and vice versa including support for move semantics.

In order to deliver std::vector semantics, then the data() member should be usable in the same manner as std::vector::data(), which is currently not the case.

Actual behavior

See the details above.

Additional information

The BoundedVector::data() methods are templatized on the return type:

rosidl/rosidl_runtime_cpp/include/rosidl_runtime_cpp/bounded_vector.hpp

Lines 410 to 434 in a8d9f8b

	template<
	typename T,
	typename std::enable_if<
	!std::is_same<T, Tp>::value &&
	!std::is_same<T, bool>::value
	>::type * = nullptr
	>
	T *
	data() noexcept
	{
	return Base::data();
	}
	template<
	typename T,
	typename std::enable_if<
	!std::is_same<T, Tp>::value &&
	!std::is_same<T, bool>::value
	>::type * = nullptr
	>
	const T *
	data() const noexcept
	{
	return Base::data();
	}

However, it's not clear why this is the case. One solution is to simplify the definitions of BoundedVector::data() to make them non-templatized and just return Base::value_type *:

-   template<
-     typename T,
-     typename std::enable_if<
-       !std::is_same<T, Tp>::value &&
-       !std::is_same<T, bool>::value
-     >::type * = nullptr
-   >
-   T *
+   Base::value_type *
    data() noexcept
    {
      return Base::data();
    }

-   template<
-     typename T,
-     typename std::enable_if<
-       !std::is_same<T, Tp>::value &&
-       !std::is_same<T, bool>::value
-     >::type * = nullptr
-   >
-   const T *
+   Base::value_type *
    data() const noexcept
    {
      return Base::data();
    }

Tagging @dirk-thomas as the original author of BoundedVector to hopefully be able to provide some context on this matter.