Document that `OsString` and `OsStr` are bytes; provide conversions to bytes #95290

joshtriplett · 2022-03-24T22:49:21Z

The OsString and OsStr types are wrappers around bytes, and are required by existing stable APIs to be byte-based encodings that are supersets of UTF-8 (since a &str can become an &OsStr without allocation). However, both types are opaque, and do not provide access to raw bytes, primarily because on Windows they use the WTF-8 encoding, the standard for which recommends against exposing it.

Nonetheless, many crates do rely on OsStr and OsString being bytes. We already provide the OsStrExt and OsStringExt extension traits, which get used reasonably widely despite being non-portable. And the os_str_bytes crate has millions of downloads, and tens of thousands of new downloads everyday.

This PR is an effort to seek consensus for making the contents of an OsStr or OsString available as bytes, inspired by recent discussions within the libs-api team.

This is a minimal PR to acheive that goal. This PR consists almost entirely of documentation, updating the module documentation in std::ffi and the type documentation on OsString. This PR only adds two methods: OsStr::as_bytes and OsString::into_vec, and plumbs those methods down to the existing implementations for both UNIX and Windows. Both methods are unstable (and I'll file a tracking issue for them as soon as I take this PR out of draft status).

In particular, this PR does not add any new From or TryFrom or Into or TryInto impls, nor does it add methods to construct OsStr or OsString from bytes (which would require error handling), nor does it attempt to do any further unification of implementations between Windows and UNIX, nor does it extend these types to have more of the functionality of bstr. All of those things could happen in potential future changes, and I'd be happy to make some of those changes myself, but I'd like to focus on the initial consensus for making the change before further work that depends on the change.

As one further note of the implications of this change: there has been a proposal to change OsStr/OsString on Windows from WTF-8 to a more complex encoding that can potentially speed up matching for use with the Pattern API. Since that proposal, we've decided to seal the Pattern API and avoid stabilizing it. I would propose that we not switch the encoding of OsStr/OsString, and proceed with exposing bytes using the current encoding. Another option would be to expose bytes but not specify the exact encoding beyond "superset of UTF-8".

r? @ghost

Mark-Simulacrum · 2022-03-25T02:55:10Z

are required by existing stable APIs to be byte-based encodings that are supersets of UTF-8 (since a &str can become an &OsStr without allocation).

It seems like this only requires that there is some variant of OsStr that is byte-based encoded as a superset of UTF-8, right? It would be possible for OsStr to wrap something like enum OsStrInternal { Utf8(&str), Other(???) }, presuming we used some kind of pointer tricks or whatever that are compatible with &str's representation to encode the state -- i.e., not actually an enum. (For example, Other might be encoded by moving the length of the str +isize::MAX, since such lengths aren't valid &str).

I mostly ask/note this because I want to make sure that this kind of option is explicitly mentioned in the writeup (and potentially rejected, either for complexity or infeasibility if (for example) we can't think of an encoding that will work), so that we can point back to it when thinking about this in the future.

joshtriplett · 2022-03-25T05:18:10Z

are required by existing stable APIs to be byte-based encodings that are supersets of UTF-8 (since a &str can become an &OsStr without allocation).

It seems like this only requires that there is some variant of OsStr that is byte-based encoded as a superset of UTF-8, right? It would be possible for OsStr to wrap something like enum OsStrInternal { Utf8(&str), Other(???) }, presuming we used some kind of pointer tricks or whatever that are compatible with &str's representation to encode the state -- i.e., not actually an enum. (For example, Other might be encoded by moving the length of the str +isize::MAX, since such lengths aren't valid &str).

I mostly ask/note this because I want to make sure that this kind of option is explicitly mentioned in the writeup (and potentially rejected, either for complexity or infeasibility if (for example) we can't think of an encoding that will work), so that we can point back to it when thinking about this in the future.

Fair enough; strictly speaking, the requirement would be that OsStr must be a superset of a UTF-8 str, which is not quite the same as saying that the bytes must be a superset of UTF-8. I think in practice the documentation should not actually make that distinction as it would be confusing to the reader, but libs-api should absolutely allow for that possibility when considering options.

We could, for example, use some encoding that has a second variant for a native [u16] on Windows. I don't think we should do that here, even though we might be able to.

…o bytes This commit consists almost entirely of documentation, updating the module documentation in `std::ffi` and the type documentation on `OsString`. This only adds two methods: `OsStr::as_bytes` and `OsString::into_vec`, and plumbs those methods down to the existing implementations for both UNIX and Windows.

dylni · 2022-03-26T13:51:51Z

And the os_str_bytes crate has millions of downloads, and tens of thousands of new downloads everyday.

Note that this crate doesn't rely on the internal representation at all. It decodes the encoded u16s from OsStrExt::encode_wide. So, this crate would not make sense to be a reason that the internal encoding could not change. However, the current libstd API does only allow limited changes.

Also, the preferred way to use the crate is to use RawOsStr and RawOsString, which make it rare to need the raw bytes. For example, clap only uses those structs.

I think this PR would still be a good idea, since many people rely on the representation anyway. However, I wanted to clarify those points.

on Windows they use the WTF-8 encoding, the standard for which recommends against exposing it.

One option would be to do the same as os_str_bytes and document limited invariants about the encoding. This would allow many of the same changes that could be made now to the internal representation.

dylni · 2022-03-26T14:05:06Z

There is one problem if this PR gets merged and starts getting used. Without a wrapper, it becomes very easy to mix WTF-8 and UTF-8 bytes. For searching and concatenation, that would be a problem, since both need special handling of WTF-8 bytes.

Also, if storage is permitted, the encoding could never change.

joshtriplett · 2022-03-30T18:38:20Z

One option would be to do the same as os_str_bytes and document limited invariants about the encoding.

I have no objections to documenting a lesser guarantee and making the encoding less specified, if that's needed in order to get consensus to make this change.

rustdoc: Early doc link resolution fixes and refactorings A subset of rust-lang#94857 that shouldn't cause perf regressions, but should fix some issues like https://rust-lang.zulipchat.com/#narrow/stream/266220-rustdoc/topic/ICE.20in.20collect_intra_doc_links.2Ers rust-lang#95290 and improve performance in cases like rust-lang#95694.

petrochenkov · 2022-04-07T23:59:51Z

#95706 has landed, so af7b7da should no longer be necessary.

joshtriplett added T-libs-api I-libs-api-nominated labels Mar 24, 2022

joshtriplett force-pushed the os-str-is-bytes branch from f1086bb to ccd297b Compare Mar 24, 2022