Repository navigation
Tracking Issue for utf8_chunks #99543
Description
Feature gate: #![feature(utf8_chunks)]
This is a tracking issue for an improved API for str::from_utf8.
Public API
// core::str
pub struct Utf8Chunks<'a> { ... }
impl<'a> Utf8Chunks<'a> {
pub fn new(bytes: &'a [u8]) -> Self;
}
impl<'a> Iterator for Utf8Chunks<'a> {
type Item = Utf8Chunk<'a>;
}
impl<'a> Clone for Utf8Chunks<'a>;
impl<'a> Debug for Utf8Chunks<'a>;
impl<'a> FusedIterator for Utf8Chunks<'a>;
pub struct Utf8Chunk<'a> { ... }
impl<'a> Utf8Chunk<'a> {
pub fn valid(&self) -> &'a str;
pub fn invalid(&self) -> &'a [u8];
}
impl<'a> Clone for Utf8Chunk<'a>;
impl<'a> Debug for Utf8Chunk<'a>;
impl<'a> PartialEq for Utf8Chunk<'a>;
impl<'a> Eq for Utf8Chunk<'a>;Steps / History
- Proposal: Expose
Utf8LossyChunksIterlibs-team#54 - Implementation: Expose
Utf8LossyasUtf8Chunks#99544 - Final comment period (FCP)1
- Stabilization PR
Unresolved Questions
- Should the constructor be
Utf8Chunks::newor<[u8]>::utf8_chunks? - Should
Utf8Chunks::debugor a similar method be exposed?
Activity
Rollup merge of rust-lang#99544 - dylni:expose-utf8lossy, r=Mark-Simu…
dtolnay commentedon Nov 24, 2023
I'd be interested in using this to implement Display and Debug for
CxxStringin the cxx crate. Here is the current implementation without Utf8Chunks:Here is what it looks like using Utf8Chunks as currently exists in nightly:
Are there other known use cases so far that we could look at before an FCP? One thing I am interested in is how the current Utf8Chunks API compares with this alternative one, not based on Iterator, with just 1 type:
dylni commentedon Dec 16, 2023
@dtolnay I am currently waiting on this ACP for stabilization.
I am aware of the following use cases.
String::from_utf8_lossy)Debugformatting (as you mentioned)I was originally going to use this feature in
os_str_bytesforDebugformatting, butinvalidreturning individual "sequences" made this usage cumbersome.OsStrcannot be assumed to have the same invalid sequences. However, the individual sequences are required for lossy conversion to work withUtf8Chunksin its current form within libstd.My concern is that the alternate API is easier to misuse (e.g., calling
next_validtwice for two valid chunks). It also requires parsing each invalid sequence twice.Dylan-DPC commentedon Mar 6, 2024
@dylni generally, you don't need an ACP for this to stabilise (unless the team explicitly asked for it which I don't think happened in this case).
The next step is an FCP. In which case, you can submit a stabilisation pr for it linking this issue and preferably putting the report you shared here in that pr and then the team will run an fcp either in the pr or the issue.
dylni commentedon Mar 9, 2024
@Dylan-DPC Right, but the problem is that the ACP would change the API. Stabilizing at this point would prevent the API change from landing.
dtolnay commentedon Apr 11, 2024
@rust-lang/libs-api:
@rfcbot fcp merge
I propose stabilizing
core::str::Utf8Chunksandcore::str::Utf8Chunk❗with the minor modification described in rust-lang/libs-team#190 ❗.I recently wanted this API for C++ string Debug impls in cxx as described in #99543 (comment), and also in libproc_macro for synthesizing C-string literals in #123769.
rfcbot commentedon Apr 11, 2024
Team member @dtolnay has proposed to merge this. The next step is review by the rest of the tagged team members:
No concerns currently listed.
Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!
See this document for info about what commands tagged team members can give me.
rfcbot commentedon Apr 11, 2024
🔔 This is now entering its final comment period, as per the review above. 🔔
BurntSushi commentedon Apr 11, 2024
Note that this same API, under the same name, is also in
bstr. See alsoUtf8ChunksandUtf8Chunk. I can only identify two differences:bstrdoesn't have aUtf8Chunks::newconstructor. This seems okay.Utf8Chunkin std lacks bstr'sUtf8Chunk::incompletemethod. This seems okay and something we can add later if the use cases arise.With that said, does this mean we also have an
itertoolssituation here?bstrworks by defining methods on[u8]via an extension trait.4 remaining items
BurntSushi commentedon Apr 15, 2024
I don't have a specific example to point to unfortunately. But it's similar in concept at least to
Utf8Error::error_len. My guess is that if you can find a real world use case forUtf8Error::error_len, then it would also serve as a use case forUtf8Chunk::incomplete. That is, it's useful in a streaming context where a single codepoint might straddle chunk boundaries. @m-ou-se actually added it tobstrlong ago: BurntSushi/bstr#68m-ou-se commentedon Apr 15, 2024
I don't remember exactly what I needed that function for, but I'm sure I sent that PR because I actually needed it. I can't find any uses of it in any of the code I currently have checked out, but it looks like at that time I was working on two relevant (never finished) projects where I might have needed this: 1. a terminal emulator, and 2. some embedded project with serial communication in chunks.
There is some separate discussion on use cases in this issue: BurntSushi/bstr#42
Auto merge of rust-lang#123909 - dtolnay:utf8chunks, r=<try>
coolreader18 commentedon Apr 18, 2024
Unless
Utf8Chunks::newis intended as a temporary solution until there can be autf8_chunksmethod on[u8](à laarray::IntoIter::new) I feel like I would expect the construction function to becore::str::utf8_chunks(&[u8]) -> Utf8Chunks<'_>. This is a different type of operation, but for example,std::fs::read_dir()returnsstd::fs::ReadDir, you don't callstd::fs::ReadDir::new().newimplies an operation, just like any other function, and it semantically makes sense to me to create a newVecorCellorio::Error, but not to create a newUtf8Chunks. It's, like... a behavior type, not a data type.dtolnay commentedon Apr 19, 2024
GitHub search for "chunk.incomplete()" (guessing a likely variable name) shows 0 hits. GitHub search for ".incomplete()" shows 123 hits, of which I looked through all of them and 0 are related to utf8 chunks. In #123909, crater uncovered 0 uses of
incompleteacross crates.io and GitHub.As such, I recommend that we do not import bstr's
incompleteinto core until such time as it is better justified.Crater showed a single occurrence of the second type of expected break in #99543 (comment). The affected crate is actively developed (within the past 5 days) and is not depended on by any other crate on crates.io. This breakage is allowed by the standard library's API evolution policy (https://rust-lang.github.io/rfcs/1105-api-evolution.html#minor-change-adding-any-inherent-items).
I recommend that we move forward with #123909 as written (when the FCP completes in 2 days, of course).
rfcbot commentedon Apr 21, 2024
The final comment period, with a disposition to merge, as per the review above, is now complete.
As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.
This will be merged soon.
Auto merge of rust-lang#123909 - dtolnay:utf8chunks, r=joboet
Auto merge of #123909 - dtolnay:utf8chunks, r=joboet
Auto merge of #123909 - dtolnay:utf8chunks, r=joboet
Auto merge of #123909 - dtolnay:utf8chunks, r=joboet