-
Notifications
You must be signed in to change notification settings - Fork 141
Use std::byte
in cpp files for bytes
#881
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: rolling
Are you sure you want to change the base?
Use std::byte
in cpp files for bytes
#881
Conversation
Signed-off-by: Michael Carlstrom <[email protected]>
rosidl_generator_tests/test/rosidl_generator_cpp/test_array_generator.hpp
Show resolved
Hide resolved
rosidl_generator_tests/test/rosidl_generator_cpp/test_array_generator.hpp
Show resolved
Hide resolved
rosidl_generator_tests/test/rosidl_generator_cpp/test_array_generator.hpp
Show resolved
Hide resolved
rosidl_generator_tests/test/rosidl_generator_cpp/test_interfaces.cpp
Outdated
Show resolved
Hide resolved
rosidl_generator_tests/test/rosidl_generator_cpp/test_msg_initialization.cpp
Outdated
Show resolved
Hide resolved
rosidl_generator_tests/test/rosidl_generator_cpp/test_srv_initialization.cpp
Outdated
Show resolved
Hide resolved
rosidl_typesupport_introspection_tests/test/introspection_libraries_under_test.hpp
Show resolved
Hide resolved
Signed-off-by: Michael Carlstrom <[email protected]>
Signed-off-by: Michael Carlstrom <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm with green CI. i would like to have another review.
Note that this is a breaking change. |
While working on #882 Found a big use for std::byte simplifying to msg__traits.hpp. Will probably split it out of #882. Edit: My cool simplification will need to wait for C++ 20 and the addition of |
If/When this gets merged I can help fix downstream bugs. |
Pulls: #881 |
Unsurprisingly this causes errors in |
We usually avoid breaking changes and try to deprecate something first in one release and then remove it in the next release, see https://docs.ros.org/en/rolling/The-ROS2-Project/Contributing/Developer-Guide.html#deprecation-strategy. That's what I really meant with my "breaking change" comment above; sorry I wasn't clear. So yeah, we should try to support setting/getting an |
The following works for everything except compound assignments e.j. EDIT: Updated so size matches byte size #include <cstddef>
#include <iostream>
struct DeprecatedHelperByte {
std::byte storage_{};
[[deprecated("Assign std::byte instead")]]
operator unsigned char() const noexcept {
std::cout << "getter called char\n";
return static_cast<unsigned char>(storage_);
}
operator std::byte() const noexcept {
std::cout << "getter called byte\n";
return storage_;
}
DeprecatedHelperByte& operator=(std::byte b) noexcept {
std::cout << "setter called byte\n";
storage_ = b;
return *this;
}
[[deprecated("Assign std::byte instead")]]
DeprecatedHelperByte& operator=(unsigned char b) noexcept {
std::cout << "setter called char\n";
storage_ = std::byte{b};
return *this;
}
};
struct Foo {
DeprecatedHelperByte data{};
};
int main() {
Foo f;
f.data = std::byte{1};
unsigned char b = f.data;
f.data = 1;
std::byte c = f.data;
std::cout << "\nSize of Foo: " << sizeof(Foo) << '\n';
} |
Signed-off-by: Michael Carlstrom <[email protected]>
Hmm while this works great it would need a specialization for all the containers (vector, array, bounded_sequence) ROS uses. Will take into a look into just adding conversion methods. |
Signed-off-by: Michael Carlstrom <[email protected]>
Signed-off-by: Michael Carlstrom <[email protected]>
Signed-off-by: Michael Carlstrom <[email protected]>
Signed-off-by: Michael Carlstrom <[email protected]>
Signed-off-by: Michael Carlstrom <[email protected]>
Signed-off-by: Michael Carlstrom <[email protected]>
Signed-off-by: Michael Carlstrom <[email protected]>
Signed-off-by: Michael Carlstrom <[email protected]>
Signed-off-by: Michael Carlstrom <[email protected]>
Signed-off-by: Michael Carlstrom <[email protected]>
Description
Resolves TODO to use std::byte. Downstream users will need to update if they use IDL bytes. The message generation documentation would also need to be updated. Will also likely need a ROS Discourse post.
Is this user-facing behavior change?
Yes users will need to switch to using std::byte rather than the existing conversion like int.
Did you use Generative AI?
No
Additional Information