Conversation
|
Pulls: #937 |
Signed-off-by: Michael Carlstrom <rmc@carlstrom.com>
Signed-off-by: Michael Carlstrom <rmc@carlstrom.com>
Signed-off-by: Michael Carlstrom <rmc@carlstrom.com>
|
Pulls: #937 |
Signed-off-by: Michael Carlstrom <rmc@carlstrom.com>
|
Pulls: #937 |
Signed-off-by: Michael Carlstrom <rmc@carlstrom.com>
|
Pulls: #937 |
|
Pulls: #937, ros2/rosidl_typesupport_fastrtps#143 |
|
Pulls: #937, ros2/rosidl_typesupport_fastrtps#143 |
| using Base::Base; | ||
|
|
||
| operator std::vector<std::byte> &() { | ||
| return *reinterpret_cast<std::vector<std::byte> *>(this); |
There was a problem hiding this comment.
This looked suspect, so I verified, and I'm pretty sure this is undefined behavior. You can reinterpret cast the elements of the vector, but not the entire vector structure itself because C++.
If we move forward to C++20, this would be a good use of span.
There was a problem hiding this comment.
Yeah. Its super cooked on C++17. Also without template specialization coercion there is no way to make this a complete no breaking change. Not sure how much of a problem this is in the practical sense but did notice it which is why type support had to be updated. Maybe the proper method is just add a second byte field then deprecate the old one? But also maybe it's not worth the hassle to be technically correct.
|
Pulls: #937, ros2/demos#777 |
|
I think the easiest way would be to use #945. And just add a new_byte type. And use deprecation to transition users. Rather than do this fancy stuff with coerecion. |

Description
Fixes TODO: To migrate to
std::byteIs this user-facing behavior change?
Yes this will emits lots of deprecation warnings but should be compile compatible.
Did you use Generative AI?
Additional Information