Avoid redundant WTF-8 checks in PathBuf

[thaliaarchi]

Eliminate checks for WTF-8 boundaries in PathBuf::set_extension and add_extension, where joining WTF-8 surrogate halves is impossible. Don't convert the str to OsStr, because OsString::push specializes to skip the joining when given strings.

To assist in this, mark the internal methods OsString::truncate and extend_from_slice as unsafe to communicate their safety invariants better than with module privacy.

Similar to #137777.

cc @joboet @ChrisDenton

[rustbot]

r? @workingjubilee

rustbot has assigned @workingjubilee.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

[workingjubilee]

oh dear.

[thaliaarchi]

@ChrisDenton wanna review this? They've reviewed my other WTF-8 stuff, so I probably should have r?'d them.

[rustbot]

Failed to set assignee to 'd: invalid assignee

Note: Only org members with at least the repository "read" role, users with write permissions, or people who have commented on the PR may be assigned.

[workingjubilee]

"must not require encoding-dependent surrogate joining" is deferring the safety requirement to instead be "you must know exactly how the implementation works", so I think this is not a significant improvement on messaging for the (ahem) modularity of reasoning.

unless we can concisely describe what requires that, in which case "don't put that in" should be the requirement instead.

and I am pretty sure we can?

[workingjubilee]

specifically "the last bytes of self must not encode a leading/high surrogate for a surrogate pair, and the first bytes of other must not encode a trailing/low surrogate of a surrogate pair". is there another case?

[workingjubilee]

yeah, it feels like "must not require joining surrogates" kinda wants to just say what the bad input that you shouldn't give it is but is being coy instead.

[workingjubilee]

I am having increasingly negative opinions about having an internal type named Slice but it is not your responsibility for that, so

[thaliaarchi]

Then what would a better name be? I might submit a PR for that. But maybe not.

[workingjubilee]

I am not sure, honestly! it just is starting to bug me because this looked like a typo at first.

[workingjubilee]

"well, my child, when a high surrogate and a low surrogate get together, they blow your code up stop doing that"

[thaliaarchi]

I've updated the safety comments. In addition to rewording based on your feedback, I also mentioned that the slice must be valid for the given encoding like from_encoded_bytes_unchecked.

[workingjubilee]

cool, that looks good to me. should be able to r+ it in the morning or thereabouts.

[workingjubilee]

Thanks!

[workingjubilee]

Thank you to @thaliaarchi for helping explain to/remind me that the reason this is sufficient as a safety condition is that joined surrogate pairs get distinct encodings that are not identical to "a lone surrogate followed by a lone surrogate" because that's why UTF-8 is, er, UTF-8 instead of... yeah.

To reiterate for my own benefit, because I have an annoying "I understand this from the bottom up or it doesn't click" problem, this required understanding:

• UTF-16 evolved from UCS-2, an encoding for the "basic multilingual plane"
• UTF-16 encodes "astral" code points beyond the BMP using "surrogate pairs", unassigned values from UCS-2 that served as a "niche" in the encoding, to fit in each single u16 the de facto enum:

enum Utf16 {
    Single(Ucs2),
    Paired(Surrogate)
}

• Nominally, "UTF-16" encoded data should reject the Utf16::Paired variant when it isn't actually paired, but a lot of "WTF-16" encodings actually allow lone surrogates in because everyone was still tacitly assuming handling things code-unit-by-code-unit would work fine and that's not how any multi-code-unit encoding works.
• WTF-8 always encodes values in UTF-8 when values can be validly encoded in UTF-8.
• WTF-8 has an encoding for lone surrogates so it can handle invalid "WTF-16", taken out of UTF-8's own encoding "niche": it is not valid UTF-8.
• This means we have to handle "this surrogate should not exist" and "this surrogate is paired with another surrogate so they're a real boy Unicode code point". Thus, putting two surrogates "one after the other" is not always "simple" appending for WTF-8, when it can actually mean decoding the pair of code units into a Unicode code point and then encoding that into UTF-8.

( There's a detail for "trailing and leading" surrogates about the actual encoding for surrogates but I am deliberately eliding it because everyone spends five hours of explanation on exactly how surrogates are encoded which doesn't matter when thinking about the design at a higher level, such that everything that actually matters oozes out of my brain before we get to the conclusion. )

( I guess that's me not being completely bottom-up but whatever. )

[workingjubilee]

...this is almost completely irrelevant to this actual review obviously I'm just trying to write this down again to try to get it to stick to my brain this time, as I have tried to learn/understand this at least 3 times.

[workingjubilee]

@bors r+

[bors]

📌 Commit 0f0c0d8 has been approved by workingjubilee

It is now in the queue for this repository.