Remove AsRef and instead introduce Cow-returning canonicalize methods on locale/langid #5727
Conversation
… on locale/langid
Manishearth
force-pushed
the
locale-langid-canonicalize
branch
from
ea8ce7b to
1ab3fde
Compare
components/locale_core/src/langid.rs
Outdated
| if let Ok(s) = core::str::from_utf8(input) { | ||
| Ok(s.into()) | ||
| } else { | ||
| Ok(cow.into_owned().into()) |
There was a problem hiding this comment.
this is unreachable. try_from_utf8 succeeding should allow you to justify from_utf8_unchecked
There was a problem hiding this comment.
It would be extremely bad unsafe code hygeine to rely on the behavior of a complicated parse function spread into multiple files across the crate. We should have that justification only if we are willing to document try_from_utf8 as guaranteed to fail for invalid UTF8, and add safety comments throughout it.
The justification of the bytes being equal is an easier one IMO. I'm just not sure if the unsafe is worth it.
I've gone ahead and added unsafe justified by byte equality.
components/locale_core/src/langid.rs
Outdated
| if let Ok(s) = core::str::from_utf8(input) { | ||
| Ok(s.into()) | ||
| } else { | ||
| Ok(cow.into_owned().into()) |
There was a problem hiding this comment.
It would be extremely bad unsafe code hygeine to rely on the behavior of a complicated parse function spread into multiple files across the crate. We should have that justification only if we are willing to document try_from_utf8 as guaranteed to fail for invalid UTF8, and add safety comments throughout it.
The justification of the bytes being equal is an easier one IMO. I'm just not sure if the unsafe is worth it.
I've gone ahead and added unsafe justified by byte equality.
Fixes #2748