Skip to content
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

Unicode handling in String and WString #362

Merged
merged 1 commit into from
Apr 26, 2024

Conversation

nwn
Copy link
Contributor

@nwn nwn commented Feb 4, 2024

According to https://design.ros2.org/articles/wide_strings.html, the string and wstring types must use the UTF-8 and UTF-16 encodings (not necessarily enforced), cannot contain null bytes or words (enforced), and, when bounded, are measured in terms of bytes or words.

Moreover, though the rosidl_runtime_c U16String type uses uint_least16_t, Rust guarantees the existence of a precise u16 type, so we should use that instead of ushort, which isn't guaranteed to be the same as uint_least16_t anyways. (Rust doesn't support platforms where uint_least16_t != uint16_t.)

According to https://design.ros2.org/articles/wide_strings.html, the `string` and `wstring` types must use the UTF-8 and UTF-16 encodings (not necessarily enforced), cannot contain null bytes or words (enforced), and, when bounded, are measured in terms of bytes or words.

Moreover, though the rosidl_runtime_c `U16String` type uses `uint_least16_t`, Rust guarantees the existence of a precise `u16` type, so we should use that instead of `ushort`, which isn't guaranteed to be the same as `uint_least16_t` either. (Rust doesn't support platforms where `uint_least16_t != uint16_t`.)
@nwn nwn marked this pull request as ready for review February 9, 2024 22:59
@maspe36 maspe36 self-requested a review April 13, 2024 15:54
Copy link
Collaborator

@maspe36 maspe36 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey sorry for the long delay @nwn, these changes LGTM.

Rust doesn't support platforms where uint_least16_t != uint16_t.

Out of curiosity, I was trying to track down this claim, but I'm coming up empty. Do you have a source for this?

@nwn
Copy link
Contributor Author

nwn commented Apr 15, 2024

No worries @maspe36, thanks for taking a look.

Rust doesn't support platforms where uint_least16_t != uint16_t.

Out of curiosity, I was trying to track down this claim, but I'm coming up empty. Do you have a source for this?

I'm actually failing to find a direct source for that claim as well, though I thought I had one. I think it can still be inferred from the definitions of uint16_t and uint_least16_t. If a platform defines uint16_t, then by definition uint_least16_t must be the same size. So by providing an exact u16, Rust would require this.

@maspe36 maspe36 merged commit 875bda3 into ros2-rust:main Apr 26, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants