cstr16: add method to get the underlying bytes

[phip1611]

This fixes #787 by adding to_bytes_with_nul and to_bytes. The operations can also be called on CString16, as CString16 implements Deref<CStr16>.

Checklist

• Sensible git history (for example, squash "typo" or "fix" commits). See the Rewriting History guide for help.
• Update the changelog (if necessary)

[nicholasbishop]

I think it would be better to leave out the AsRef/Borrow impls, as it would not be as clear when reading the code as an explicit call to to_bytes_with_nul.

[phip1611]

I'd much rather drop self.to_bytes() and rename self.to_bytes_with_nul() to self.to_bytes(). Then we can keep AsRef and Borrow. What do you think?

[nicholasbishop]

Yeah that sounds reasonable. Also, I didn't think of this before, but it should probably be as_bytes instead of to_bytes (per the conversion naming guidelines).

For Borrow and AsRef, do you have an example of how they'd get used in practice? Like <CStr16 as AsRef<[u8]>>::as_ref(string) is pretty verbose so I assume a user would usually prefer to write string.as_bytes(). But perhaps there's some generic code where the former would be more usable? An example would help me clear up my doubts since I always get a bit confused with those two very-similar-seeming traits.

[phip1611]

Sure. For example FileSystem::write() consumes content: impl AsRef<[u8]> as second parameter. This is equivalent to the standard lib's implementation.

Hence, you can just write a CStr16 to a file.

I only implemented both, AsRef and Borrow, as the standard library also does this for a lot of types (for example for std::path::Path). In my past years I've come accross a lot of code that used AsRef-types as parameters but Borrow only once or twice - Probably good to have both, tho.

[phip1611]

Update Hm, now I'm confused.. https://doc.rust-lang.org/core/ffi/struct.CStr.html provides to_bytes() and to_bytes_with_nul() methods, but those method implementations are cheap (other than the naming convention implies).

But I do not see a strong reason why we should be consistent with https://doc.rust-lang.org/core/ffi/struct.CStr.html .. but..on the other hand.. we can also stay consistent with std::ffi::CStr and provide to_bytes(), to_bytes_until_nul(), and drop AsRef and Borrow, as it might be confusing then.

[nicholasbishop]

Interesting. I would guess the reason is this:

Note: This method is currently implemented as a constant-time cast, but it is planned to alter its definition in the future to perform the length calculation whenever this method is called.

Several CStr methods make note that the implementation of the type may change in the future to make it more FFI friendly. Not sure how likely that is to ever actually happen.

I'm realizing I don't have a very strong opinion here :)

How about: just provide the as_bytes method (so no separate with/without null methods), and also include the AsRef and Borrow impls?

[phip1611]

Done