Skip to content

Commit b4f9fac

Browse files
committed
perf: smol_str — avoid redundant work in eq
feat: add missing APIs to smol_str docs: Improve safety documentation fix: borsh deserialization off-by-one for inline threshold
1 parent 80af448 commit b4f9fac

2 files changed

Lines changed: 103 additions & 37 deletions

File tree

src/tools/rust-analyzer/lib/smol_str/src/borsh.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ impl BorshDeserialize for SmolStr {
1616
#[inline]
1717
fn deserialize_reader<R: Read>(reader: &mut R) -> borsh::io::Result<Self> {
1818
let len = u32::deserialize_reader(reader)?;
19-
if (len as usize) < INLINE_CAP {
19+
if (len as usize) <= INLINE_CAP {
2020
let mut buf = [0u8; INLINE_CAP];
2121
reader.read_exact(&mut buf[..len as usize])?;
2222
_ = core::str::from_utf8(&buf[..len as usize]).map_err(|err| {

src/tools/rust-analyzer/lib/smol_str/src/lib.rs

Lines changed: 102 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,24 @@ impl SmolStr {
100100
pub const fn is_heap_allocated(&self) -> bool {
101101
matches!(self.0, Repr::Heap(..))
102102
}
103+
104+
/// Constructs a `SmolStr` from a byte slice, returning an error if the slice is not valid
105+
/// UTF-8.
106+
#[inline]
107+
pub fn from_utf8(bytes: &[u8]) -> Result<SmolStr, core::str::Utf8Error> {
108+
core::str::from_utf8(bytes).map(SmolStr::new)
109+
}
110+
111+
/// Constructs a `SmolStr` from a byte slice without checking that the bytes are valid UTF-8.
112+
///
113+
/// # Safety
114+
///
115+
/// `bytes` must be valid UTF-8.
116+
#[inline]
117+
pub unsafe fn from_utf8_unchecked(bytes: &[u8]) -> SmolStr {
118+
// SAFETY: caller guarantees bytes are valid UTF-8
119+
SmolStr::new(unsafe { core::str::from_utf8_unchecked(bytes) })
120+
}
103121
}
104122

105123
impl Clone for SmolStr {
@@ -116,7 +134,10 @@ impl Clone for SmolStr {
116134
return cold_clone(self);
117135
}
118136

119-
// SAFETY: We verified that the payload of `Repr` is a POD
137+
// SAFETY: The non-heap variants (`Repr::Inline` and `Repr::Static`) contain only
138+
// `Copy` data (a `[u8; 23]` + `InlineSize` enum, or a `&'static str` fat pointer)
139+
// and carry no drop glue, so a raw `ptr::read` bitwise copy is sound.
140+
// The heap variant (`Repr::Heap`) is excluded above.
120141
unsafe { core::ptr::read(self as *const SmolStr) }
121142
}
122143
}
@@ -142,7 +163,12 @@ impl ops::Deref for SmolStr {
142163
impl Eq for SmolStr {}
143164
impl PartialEq<SmolStr> for SmolStr {
144165
fn eq(&self, other: &SmolStr) -> bool {
145-
self.0.ptr_eq(&other.0) || self.as_str() == other.as_str()
166+
match (&self.0, &other.0) {
167+
(Repr::Inline { len: l_len, buf: l_buf }, Repr::Inline { len: r_len, buf: r_buf }) => {
168+
l_len == r_len && l_buf == r_buf
169+
}
170+
_ => self.as_str() == other.as_str(),
171+
}
146172
}
147173
}
148174

@@ -483,11 +509,15 @@ enum InlineSize {
483509
}
484510

485511
impl InlineSize {
486-
/// SAFETY: `value` must be less than or equal to [`INLINE_CAP`]
512+
/// # Safety
513+
///
514+
/// `value` must be in the range `0..=23` (i.e. a valid `InlineSize` discriminant).
515+
/// Values outside this range would produce an invalid enum discriminant, which is UB.
487516
#[inline(always)]
488517
const unsafe fn transmute_from_u8(value: u8) -> Self {
489518
debug_assert!(value <= InlineSize::_V23 as u8);
490-
// SAFETY: The caller is responsible to uphold this invariant
519+
// SAFETY: The caller guarantees `value` is a valid discriminant for this
520+
// `#[repr(u8)]` enum (0..=23), so the transmute produces a valid `InlineSize`.
491521
unsafe { mem::transmute::<u8, Self>(value) }
492522
}
493523
}
@@ -563,24 +593,15 @@ impl Repr {
563593
Repr::Static(data) => data,
564594
Repr::Inline { len, buf } => {
565595
let len = *len as usize;
566-
// SAFETY: len is guaranteed to be <= INLINE_CAP
596+
// SAFETY: `len` is an `InlineSize` discriminant (0..=23) which is always
597+
// <= INLINE_CAP (23), so `..len` is always in bounds of `buf: [u8; 23]`.
567598
let buf = unsafe { buf.get_unchecked(..len) };
568-
// SAFETY: buf is guaranteed to be valid utf8 for ..len bytes
599+
// SAFETY: All constructors that produce `Repr::Inline` copy from valid
600+
// UTF-8 sources (`&str` or char encoding), so `buf[..len]` is valid UTF-8.
569601
unsafe { ::core::str::from_utf8_unchecked(buf) }
570602
}
571603
}
572604
}
573-
574-
fn ptr_eq(&self, other: &Self) -> bool {
575-
match (self, other) {
576-
(Self::Heap(l0), Self::Heap(r0)) => Arc::ptr_eq(l0, r0),
577-
(Self::Static(l0), Self::Static(r0)) => core::ptr::eq(l0, r0),
578-
(Self::Inline { len: l_len, buf: l_buf }, Self::Inline { len: r_len, buf: r_buf }) => {
579-
l_len == r_len && l_buf == r_buf
580-
}
581-
_ => false,
582-
}
583-
}
584605
}
585606

586607
/// Convert value to [`SmolStr`] using [`fmt::Display`], potentially without allocating.
@@ -666,7 +687,7 @@ impl StrExt for str {
666687
buf[..len].copy_from_slice(self.as_bytes());
667688
buf[..len].make_ascii_lowercase();
668689
SmolStr(Repr::Inline {
669-
// SAFETY: `len` is in bounds
690+
// SAFETY: `len` is guarded to be <= INLINE_CAP (23), a valid `InlineSize` discriminant.
670691
len: unsafe { InlineSize::transmute_from_u8(len as u8) },
671692
buf,
672693
})
@@ -683,7 +704,7 @@ impl StrExt for str {
683704
buf[..len].copy_from_slice(self.as_bytes());
684705
buf[..len].make_ascii_uppercase();
685706
SmolStr(Repr::Inline {
686-
// SAFETY: `len` is in bounds
707+
// SAFETY: `len` is guarded to be <= INLINE_CAP (23), a valid `InlineSize` discriminant.
687708
len: unsafe { InlineSize::transmute_from_u8(len as u8) },
688709
buf,
689710
})
@@ -703,8 +724,11 @@ impl StrExt for str {
703724
if let [from_u8] = from.as_bytes()
704725
&& let [to_u8] = to.as_bytes()
705726
{
727+
// SAFETY: `from` and `to` are single-byte `&str`s. In valid UTF-8, a single-byte
728+
// code unit is always in the range 0x00..=0x7F (i.e. ASCII). The closure only
729+
// replaces the matching ASCII byte with another ASCII byte, and returns all
730+
// other bytes unchanged, so UTF-8 validity is preserved.
706731
return if self.len() <= count {
707-
// SAFETY: `from_u8` & `to_u8` are ascii
708732
unsafe { replacen_1_ascii(self, |b| if b == from_u8 { *to_u8 } else { *b }) }
709733
} else {
710734
unsafe {
@@ -736,7 +760,11 @@ impl StrExt for str {
736760
}
737761
}
738762

739-
/// SAFETY: `map` fn must only replace ascii with ascii or return unchanged bytes.
763+
/// # Safety
764+
///
765+
/// `map` must satisfy: for every byte `b` in `src`, if `b <= 0x7F` (ASCII) then `map(b)` must
766+
/// also be `<= 0x7F` (ASCII). If `b > 0x7F` (part of a multi-byte UTF-8 sequence), `map` must
767+
/// return `b` unchanged. This ensures the output is valid UTF-8 whenever the input is.
740768
#[inline]
741769
unsafe fn replacen_1_ascii(src: &str, mut map: impl FnMut(&u8) -> u8) -> SmolStr {
742770
if src.len() <= INLINE_CAP {
@@ -745,13 +773,16 @@ unsafe fn replacen_1_ascii(src: &str, mut map: impl FnMut(&u8) -> u8) -> SmolStr
745773
buf[idx] = map(b);
746774
}
747775
SmolStr(Repr::Inline {
748-
// SAFETY: `len` is in bounds
776+
// SAFETY: `src` is a `&str` so `src.len()` <= INLINE_CAP <= 23, which is a
777+
// valid `InlineSize` discriminant.
749778
len: unsafe { InlineSize::transmute_from_u8(src.len() as u8) },
750779
buf,
751780
})
752781
} else {
753782
let out = src.as_bytes().iter().map(map).collect();
754-
// SAFETY: We replaced ascii with ascii on valid utf8 strings.
783+
// SAFETY: The caller guarantees `map` only substitutes ASCII bytes with ASCII
784+
// bytes and leaves multi-byte UTF-8 continuation bytes untouched, so the
785+
// output byte sequence is valid UTF-8.
755786
unsafe { String::from_utf8_unchecked(out).into() }
756787
}
757788
}
@@ -773,9 +804,11 @@ fn inline_convert_while_ascii(s: &str, convert: fn(&u8) -> u8) -> ([u8; INLINE_C
773804
let mut is_ascii = [false; N];
774805

775806
while slice.len() >= N {
776-
// SAFETY: checked in loop condition
807+
// SAFETY: The loop condition guarantees `slice.len() >= N`, so `..N` is in bounds.
777808
let chunk = unsafe { slice.get_unchecked(..N) };
778-
// SAFETY: out_slice has at least same length as input slice and gets sliced with the same offsets
809+
// SAFETY: `out_slice` starts with the same length as `slice` (both derived from
810+
// `s.len()`) and both are advanced by the same offset `N` each iteration, so
811+
// `out_slice.len() >= N` holds whenever `slice.len() >= N`.
779812
let out_chunk = unsafe { out_slice.get_unchecked_mut(..N) };
780813

781814
for j in 0..N {
@@ -794,6 +827,7 @@ fn inline_convert_while_ascii(s: &str, convert: fn(&u8) -> u8) -> ([u8; INLINE_C
794827
out_chunk[j] = convert(&chunk[j]);
795828
}
796829

830+
// SAFETY: Same reasoning as above — both slices have len >= N at this point.
797831
slice = unsafe { slice.get_unchecked(N..) };
798832
out_slice = unsafe { out_slice.get_unchecked_mut(N..) };
799833
}
@@ -804,7 +838,9 @@ fn inline_convert_while_ascii(s: &str, convert: fn(&u8) -> u8) -> ([u8; INLINE_C
804838
if byte > 127 {
805839
break;
806840
}
807-
// SAFETY: out_slice has at least same length as input slice
841+
// SAFETY: `out_slice` is always the same length as `slice` (both start equal and
842+
// are advanced by 1 together), and `slice` is non-empty per the loop condition,
843+
// so index 0 and `1..` are in bounds for both.
808844
unsafe {
809845
*out_slice.get_unchecked_mut(0) = convert(&byte);
810846
}
@@ -813,8 +849,10 @@ fn inline_convert_while_ascii(s: &str, convert: fn(&u8) -> u8) -> ([u8; INLINE_C
813849
}
814850

815851
unsafe {
816-
// SAFETY: we know this is a valid char boundary
817-
// since we only skipped over leading ascii bytes
852+
// SAFETY: We only advanced past bytes that satisfy `b <= 127`, i.e. ASCII bytes.
853+
// In UTF-8, ASCII bytes (0x00..=0x7F) are always single-byte code points and
854+
// never appear as continuation bytes, so the remaining `slice` starts at a valid
855+
// UTF-8 char boundary.
818856
let rest = core::str::from_utf8_unchecked(slice);
819857
(out, rest)
820858
}
@@ -875,17 +913,17 @@ impl SmolStrBuilder {
875913

876914
/// Builds a [`SmolStr`] from `self`.
877915
#[must_use]
878-
pub fn finish(&self) -> SmolStr {
879-
SmolStr(match &self.0 {
880-
&SmolStrBuilderRepr::Inline { len, buf } => {
916+
pub fn finish(self) -> SmolStr {
917+
SmolStr(match self.0 {
918+
SmolStrBuilderRepr::Inline { len, buf } => {
881919
debug_assert!(len <= INLINE_CAP);
882920
Repr::Inline {
883921
// SAFETY: We know that `value.len` is less than or equal to the maximum value of `InlineSize`
884922
len: unsafe { InlineSize::transmute_from_u8(len as u8) },
885923
buf,
886924
}
887925
}
888-
SmolStrBuilderRepr::Heap(heap) => Repr::new(heap),
926+
SmolStrBuilderRepr::Heap(heap) => Repr::new(&heap),
889927
})
890928
}
891929

@@ -900,8 +938,10 @@ impl SmolStrBuilder {
900938
*len += char_len;
901939
} else {
902940
let mut heap = String::with_capacity(new_len);
903-
// copy existing inline bytes over to the heap
904-
// SAFETY: inline data is guaranteed to be valid utf8 for `old_len` bytes
941+
// SAFETY: `buf[..*len]` was built by prior `push`/`push_str` calls
942+
// that only wrote valid UTF-8 (from `char::encode_utf8` or `&str`
943+
// byte copies), so extending the Vec with these bytes preserves the
944+
// String's UTF-8 invariant.
905945
unsafe { heap.as_mut_vec().extend_from_slice(&buf[..*len]) };
906946
heap.push(c);
907947
self.0 = SmolStrBuilderRepr::Heap(heap);
@@ -926,8 +966,10 @@ impl SmolStrBuilder {
926966

927967
let mut heap = String::with_capacity(*len);
928968

929-
// copy existing inline bytes over to the heap
930-
// SAFETY: inline data is guaranteed to be valid utf8 for `old_len` bytes
969+
// SAFETY: `buf[..old_len]` was built by prior `push`/`push_str` calls
970+
// that only wrote valid UTF-8 (from `char::encode_utf8` or `&str` byte
971+
// copies), so extending the Vec with these bytes preserves the String's
972+
// UTF-8 invariant.
931973
unsafe { heap.as_mut_vec().extend_from_slice(&buf[..old_len]) };
932974
heap.push_str(s);
933975
self.0 = SmolStrBuilderRepr::Heap(heap);
@@ -945,6 +987,30 @@ impl fmt::Write for SmolStrBuilder {
945987
}
946988
}
947989

990+
impl iter::Extend<char> for SmolStrBuilder {
991+
fn extend<I: iter::IntoIterator<Item = char>>(&mut self, iter: I) {
992+
for c in iter {
993+
self.push(c);
994+
}
995+
}
996+
}
997+
998+
impl<'a> iter::Extend<&'a str> for SmolStrBuilder {
999+
fn extend<I: iter::IntoIterator<Item = &'a str>>(&mut self, iter: I) {
1000+
for s in iter {
1001+
self.push_str(s);
1002+
}
1003+
}
1004+
}
1005+
1006+
impl<'a> iter::Extend<&'a String> for SmolStrBuilder {
1007+
fn extend<I: iter::IntoIterator<Item = &'a String>>(&mut self, iter: I) {
1008+
for s in iter {
1009+
self.push_str(s);
1010+
}
1011+
}
1012+
}
1013+
9481014
impl From<SmolStrBuilder> for SmolStr {
9491015
fn from(value: SmolStrBuilder) -> Self {
9501016
value.finish()

0 commit comments

Comments
 (0)