From 68d42f59ecece016c2289c38b795d45686b58376 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Kijewski?= Date: Tue, 9 Jan 2024 15:52:48 +0100 Subject: [PATCH] Add `const_new()`; remove `new_inline()` and `from_static_str()` (#336) * Rename `from_static_str()` into `const_new()` * Remove `new_inline()` --- compact_str/src/lib.rs | 50 +++++++++----------------- compact_str/src/repr/mod.rs | 59 +++++++++++++++--------------- compact_str/src/repr/smallvec.rs | 2 +- compact_str/src/repr/traits.rs | 17 +++++++-- compact_str/src/tests.rs | 61 ++++++++++++++------------------ compact_str/src/traits.rs | 6 ++-- examples/diesel/src/lib.rs | 2 +- examples/sqlx/src/lib.rs | 2 +- fuzz/src/creation.rs | 4 +-- 9 files changed, 95 insertions(+), 108 deletions(-) diff --git a/compact_str/src/lib.rs b/compact_str/src/lib.rs index 846f9a3cc..f2c4b2da7 100644 --- a/compact_str/src/lib.rs +++ b/compact_str/src/lib.rs @@ -157,6 +157,8 @@ impl CompactString { /// Creates a new [`CompactString`] from any type that implements `AsRef`. /// If the string is short enough, then it will be inlined on the stack! /// + /// In a `static` or `const` context you can use the method [`CompactString::const_new()`]. + /// /// # Examples /// /// ### Inlined @@ -210,51 +212,33 @@ impl CompactString { CompactString(Repr::new(text.as_ref()).unwrap_with_msg()) } - /// Creates a new inline [`CompactString`] at compile time. - /// - /// For most use cases you should use the method [`CompactString::from_static_str()`], - /// which will inline static strings, too, if they are short enough. - /// - /// # Examples - /// ``` - /// use compact_str::CompactString; - /// - /// const DEFAULT_NAME: CompactString = CompactString::new_inline("untitled"); - /// ``` - /// - /// Note: Trying to create a long string that can't be inlined, will fail to build. - /// ```compile_fail - /// # use compact_str::CompactString; - /// const LONG: CompactString = CompactString::new_inline("this is a long string that can't be stored on the stack"); - /// ``` - #[inline] - pub const fn new_inline(text: &str) -> Self { - CompactString(Repr::new_inline(text)) - } - /// Creates a new inline [`CompactString`] from `&'static str` at compile time. - /// /// Complexity: O(1). As an optimization, short strings get inlined. /// + /// In a dynamic context you can use the method [`CompactString::new()`]. + /// /// # Examples /// ``` /// use compact_str::CompactString; /// - /// const DEFAULT_NAME: CompactString = CompactString::from_static_str("untitled"); + /// const DEFAULT_NAME: CompactString = CompactString::const_new("untitled"); /// ``` #[inline] - pub const fn from_static_str(text: &'static str) -> Self { - CompactString(Repr::from_static_str(text)) + pub const fn const_new(text: &'static str) -> Self { + CompactString(Repr::const_new(text)) } - /// Get back the `&'static str` constructed by [`CompactString::from_static_str`]. + /// Get back the `&'static str` constructed by [`CompactString::const_new`]. + /// + /// If the string was short enough that it could be inlined, then it was inline, and + /// this method will return `None`. /// /// # Examples /// ``` /// use compact_str::CompactString; /// /// const DEFAULT_NAME: CompactString = - /// CompactString::from_static_str("That is not dead which can eternal lie."); + /// CompactString::const_new("That is not dead which can eternal lie."); /// assert_eq!( /// DEFAULT_NAME.as_static_str().unwrap(), /// "That is not dead which can eternal lie.", @@ -958,7 +942,7 @@ impl CompactString { #[must_use] pub fn repeat(&self, n: usize) -> Self { if n == 0 || self.is_empty() { - Self::new_inline("") + Self::const_new("") } else if n == 1 { self.clone() } else { @@ -1098,7 +1082,7 @@ impl CompactString { /// /// ``` /// # use compact_str::CompactString; - /// let mut s = CompactString::from_static_str("Hello, world!"); + /// let mut s = CompactString::const_new("Hello, world!"); /// let w = s.split_off(5); /// /// assert_eq!(w, ", world!"); @@ -1106,7 +1090,7 @@ impl CompactString { /// ``` pub fn split_off(&mut self, at: usize) -> Self { if let Some(s) = self.as_static_str() { - let result = Self::from_static_str(&s[at..]); + let result = Self::const_new(&s[at..]); // SAFETY: the previous line `self[at...]` would have panicked if `at` was invalid unsafe { self.set_len(at) }; result @@ -2397,9 +2381,9 @@ impl fmt::Write for CompactString { if self.is_empty() && !self.is_heap_allocated() { // Since self is currently an empty inline variant or // an empty `StaticStr` variant, constructing a new one - // with `Self::from_static_str` is more efficient since + // with `Self::const_new` is more efficient since // it is guaranteed to be O(1). - *self = Self::from_static_str(s); + *self = Self::const_new(s); } else { self.push_str(s); } diff --git a/compact_str/src/repr/mod.rs b/compact_str/src/repr/mod.rs index 40edac5e9..e0e460221 100644 --- a/compact_str/src/repr/mod.rs +++ b/compact_str/src/repr/mod.rs @@ -44,7 +44,7 @@ pub const STATIC_STR_MASK: u8 = LastUtf8Char::Static as u8; /// by `LENGTH_MASK` pub const LENGTH_MASK: u8 = 0b11000000; -const EMPTY: Repr = Repr::new_inline(""); +const EMPTY: Repr = Repr::const_new(""); #[repr(C)] pub struct Repr( @@ -81,26 +81,13 @@ impl Repr { } #[inline] - pub const fn new_inline(text: &str) -> Self { - let len = text.len(); - - if len <= MAX_SIZE { - let inline = InlineBuffer::new_const(text); - Repr::from_inline(inline) - } else { - panic!("Inline string was too long, max length is `core::mem::size_of::()` bytes"); - } - } - - #[inline] - pub const fn from_static_str(text: &'static str) -> Self { + pub const fn const_new(text: &'static str) -> Self { if text.len() <= MAX_SIZE { let inline = InlineBuffer::new_const(text); - Self::from_inline(inline) + Repr::from_inline(inline) } else { let repr = StaticStr::new(text); - // SAFETY: A `StaticStr` and `Repr` have the same size - unsafe { core::mem::transmute(repr) } + Repr::from_static(repr) } } @@ -604,6 +591,18 @@ impl Repr { unsafe { core::mem::transmute(heap) } } + /// Reinterprets a [`StaticStr`] into a [`Repr`] + /// + /// Note: This is safe because [`StaticStr`] and [`Repr`] are the same size. We used to define + /// [`Repr`] as a `union` which implicitly transmuted between the two types, but that prevented + /// us from defining a "niche" value to make `Option` the same size as just + /// `CompactString` + #[inline(always)] + const fn from_static(heap: StaticStr) -> Self { + // SAFETY: A `StaticStr` and `Repr` have the same size + unsafe { core::mem::transmute(heap) } + } + /// Reinterprets a [`Repr`] as a [`HeapBuffer`] /// /// # SAFETY @@ -839,7 +838,7 @@ mod tests { assert_eq!(repr.len(), s.len()); // test StaticStr variant - let repr = Repr::from_static_str(s); + let repr = Repr::const_new(s); assert_eq!(repr.as_str(), s); assert_eq!(repr.len(), s.len()); } @@ -949,7 +948,7 @@ mod tests { assert_eq!(control, s.as_str()); // test StaticStr variant - let r = Repr::from_static_str(control); + let r = Repr::const_new(control); let s = r.into_string(); assert_eq!(control.len(), s.len()); @@ -983,7 +982,7 @@ mod tests { assert_eq!(r.is_heap_allocated(), is_heap); // test StaticStr variant - let mut r = Repr::from_static_str(control); + let mut r = Repr::const_new(control); let mut c = String::from(control); r.push_str(append); @@ -1047,7 +1046,7 @@ mod tests { assert_eq!(r.is_heap_allocated(), is_heap); // Test static_str variant - let mut r = Repr::from_static_str(initial); + let mut r = Repr::const_new(initial); r.reserve(additional).unwrap(); assert!(r.capacity() >= initial.len() + additional); @@ -1078,7 +1077,7 @@ mod tests { assert_eq!(r_a.is_heap_allocated(), r_b.is_heap_allocated()); // test StaticStr variant - let r_a = Repr::from_static_str(initial); + let r_a = Repr::const_new(initial); let r_b = r_a.clone(); assert_eq!(r_a.as_str(), initial); @@ -1090,14 +1089,14 @@ mod tests { assert_eq!(r_a.is_heap_allocated(), r_b.is_heap_allocated()); } - #[test_case(Repr::from_static_str(""), Repr::from_static_str(""); "empty clone from static")] - #[test_case(Repr::new_inline("abc"), Repr::from_static_str("efg"); "short clone from static")] - #[test_case(Repr::new("i am a longer string that will be on the heap").unwrap(), Repr::from_static_str(EIGHTEEN_MB_STR); "long clone from static")] - #[test_case(Repr::from_static_str(""), Repr::new_inline(""); "empty clone from inline")] - #[test_case(Repr::new_inline("abc"), Repr::new_inline("efg"); "short clone from inline")] - #[test_case(Repr::new("i am a longer string that will be on the heap").unwrap(), Repr::new_inline("small"); "long clone from inline")] - #[test_case(Repr::from_static_str(""), Repr::new(EIGHTEEN_MB_STR).unwrap(); "empty clone from heap")] - #[test_case(Repr::new_inline("abc"), Repr::new(EIGHTEEN_MB_STR).unwrap(); "short clone from heap")] + #[test_case(Repr::const_new(""), Repr::const_new(""); "empty clone from static")] + #[test_case(Repr::const_new("abc"), Repr::const_new("efg"); "short clone from static")] + #[test_case(Repr::new("i am a longer string that will be on the heap").unwrap(), Repr::const_new(EIGHTEEN_MB_STR); "long clone from static")] + #[test_case(Repr::const_new(""), Repr::const_new(""); "empty clone from inline")] + #[test_case(Repr::const_new("abc"), Repr::const_new("efg"); "short clone from inline")] + #[test_case(Repr::new("i am a longer string that will be on the heap").unwrap(), Repr::const_new("small"); "long clone from inline")] + #[test_case(Repr::const_new(""), Repr::new(EIGHTEEN_MB_STR).unwrap(); "empty clone from heap")] + #[test_case(Repr::const_new("abc"), Repr::new(EIGHTEEN_MB_STR).unwrap(); "short clone from heap")] #[test_case(Repr::new("i am a longer string that will be on the heap").unwrap(), Repr::new(EIGHTEEN_MB_STR).unwrap(); "long clone from heap")] fn test_clone_from(mut initial: Repr, source: Repr) { initial.clone_from(&source); diff --git a/compact_str/src/repr/smallvec.rs b/compact_str/src/repr/smallvec.rs index 5f42641d3..45b583ea8 100644 --- a/compact_str/src/repr/smallvec.rs +++ b/compact_str/src/repr/smallvec.rs @@ -45,7 +45,7 @@ mod tests { assert_eq!(ex_compact, s); // test `StaticStr` variant - let og_compact = CompactString::from_static_str(s); + let og_compact = CompactString::const_new(s); assert_eq!(og_compact, s); let bytes = og_compact.into_bytes(); diff --git a/compact_str/src/repr/traits.rs b/compact_str/src/repr/traits.rs index d4dd356f5..c530d73fa 100644 --- a/compact_str/src/repr/traits.rs +++ b/compact_str/src/repr/traits.rs @@ -1,8 +1,10 @@ +use core::hint::unreachable_unchecked; + use super::Repr; use crate::ToCompactStringError; -const FALSE: Repr = Repr::new_inline("false"); -const TRUE: Repr = Repr::new_inline("true"); +const FALSE: Repr = Repr::const_new("false"); +const TRUE: Repr = Repr::const_new("true"); /// Defines how to _efficiently_ create a [`Repr`] from `self` pub(crate) trait IntoRepr { @@ -42,7 +44,16 @@ impl IntoRepr for char { #[inline] fn into_repr(self) -> Result { let mut buf = [0_u8; 4]; - Ok(Repr::new_inline(self.encode_utf8(&mut buf))) + let s = self.encode_utf8(&mut buf); + + // This match is just a hint for the compiler. + match s.len() { + 1..=4 => (), + // SAFETY: a UTF-8 character is 1 to 4 bytes. + _ => unsafe { unreachable_unchecked() }, + } + + Ok(Repr::new(s)?) } } diff --git a/compact_str/src/tests.rs b/compact_str/src/tests.rs index edfdc9a6a..bccd39f0e 100644 --- a/compact_str/src/tests.rs +++ b/compact_str/src/tests.rs @@ -380,20 +380,19 @@ fn proptest_to_lowercase(#[strategy(rand_unicode())] control: String) { #[test] fn test_const_creation() { - const EMPTY: CompactString = CompactString::new_inline(""); - const SHORT: CompactString = CompactString::new_inline("rust"); + const EMPTY: CompactString = CompactString::const_new(""); + const SHORT: CompactString = CompactString::const_new("rust"); - const EMPTY_STATIC_STR: CompactString = CompactString::from_static_str(""); - const SHORT_STATIC_STR: CompactString = CompactString::from_static_str("rust"); + const EMPTY_STATIC_STR: CompactString = CompactString::const_new(""); + const SHORT_STATIC_STR: CompactString = CompactString::const_new("rust"); #[cfg(target_pointer_width = "64")] - const PACKED: CompactString = CompactString::new_inline("i am 24 characters long!"); + const PACKED: CompactString = CompactString::const_new("i am 24 characters long!"); #[cfg(target_pointer_width = "32")] - const PACKED: CompactString = CompactString::new_inline("i am 12 char"); + const PACKED: CompactString = CompactString::const_new("i am 12 char"); - const PACKED_STATIC_STR0: CompactString = - CompactString::from_static_str("i am 24 characters long!"); - const PACKED_STATIC_STR1: CompactString = CompactString::from_static_str("i am 12 char"); + const PACKED_STATIC_STR0: CompactString = CompactString::const_new("i am 24 characters long!"); + const PACKED_STATIC_STR1: CompactString = CompactString::const_new("i am 12 char"); assert_eq!(EMPTY, CompactString::new("")); assert_eq!(SHORT, CompactString::new("rust")); @@ -522,7 +521,7 @@ fn test_extend_packed_from_empty() { } #[test_case(CompactString::from(""); "inline")] -#[test_case(CompactString::from_static_str(""); "static_str")] +#[test_case(CompactString::const_new(""); "static_str")] fn test_pop_empty(mut compact: CompactString) { let num_pops = 256; (0..num_pops).for_each(|_| { @@ -553,7 +552,7 @@ fn test_compact_str_is_send_and_sync() { } #[test_case(CompactString::default(); "inline")] -#[test_case(CompactString::from_static_str(""); "static_str")] +#[test_case(CompactString::const_new(""); "static_str")] fn test_fmt_write(mut compact: CompactString) { use core::fmt::Write; @@ -601,32 +600,26 @@ fn test_plus_operator() { fn test_plus_operator_static_str() { // + &CompactString assert_eq!( - CompactString::from_static_str("a") + &CompactString::from_static_str("b"), + CompactString::const_new("a") + &CompactString::const_new("b"), "ab" ); // + &str - assert_eq!(CompactString::from_static_str("a") + "b", "ab"); + assert_eq!(CompactString::const_new("a") + "b", "ab"); // + &String - assert_eq!( - CompactString::from_static_str("a") + &String::from("b"), - "ab" - ); + assert_eq!(CompactString::const_new("a") + &String::from("b"), "ab"); // + &Box let box_str = String::from("b").into_boxed_str(); - assert_eq!(CompactString::from_static_str("a") + &box_str, "ab"); + assert_eq!(CompactString::const_new("a") + &box_str, "ab"); // + &Cow<'a, str> let cow = Cow::from("b"); - assert_eq!(CompactString::from_static_str("a") + &cow, "ab"); + assert_eq!(CompactString::const_new("a") + &cow, "ab"); // Implementing `Add for String` can break adding &String or other types to String, so we // explicitly don't do this. See https://github.com/rust-lang/rust/issues/77143 for more details. // Below we assert adding types to String still compiles // String + &CompactString - assert_eq!( - String::from("a") + &CompactString::from_static_str("b"), - "ab" - ); + assert_eq!(String::from("a") + &CompactString::const_new("b"), "ab"); // String + &String assert_eq!(String::from("a") + &("b".to_string()), "ab"); // String + &str @@ -642,7 +635,7 @@ fn test_plus_equals_operator() { #[test] fn test_plus_equals_operator_static_str() { - let mut m = CompactString::from_static_str("a"); + let mut m = CompactString::const_new("a"); m += "b"; assert_eq!(m, "ab"); } @@ -1154,7 +1147,7 @@ fn test_into_string_small_static_str() { let str_addr = data.as_ptr(); let str_len = data.len(); - let compact = CompactString::from_static_str(data); + let compact = CompactString::const_new(data); let new_string = String::from(compact); let new_str_addr = new_string.as_ptr(); let new_str_len = new_string.len(); @@ -1188,7 +1181,7 @@ fn test_into_string_long_static_str() { let str_addr = data.as_ptr(); let str_len = data.len(); - let compact = CompactString::from_static_str(data); + let compact = CompactString::const_new(data); let new_string = String::from(compact); let new_str_addr = new_string.as_ptr(); let new_str_len = new_string.len(); @@ -1220,7 +1213,7 @@ fn test_into_string_empty_static_str() { let data = ""; let str_len = data.len(); - let compact = CompactString::from_static_str(data); + let compact = CompactString::const_new(data); let new_string = String::from(compact); let new_str_addr = new_string.as_ptr(); let new_str_len = new_string.len(); @@ -1250,14 +1243,14 @@ fn test_truncate_noops_if_new_len_greater_than_current() { #[test] fn test_truncate_noops_if_new_len_greater_than_current_static_str() { - let mut short = CompactString::from_static_str("short"); + let mut short = CompactString::const_new("short"); short.truncate(100); assert_eq!(short.len(), 5); assert_eq!(short.capacity(), MAX_SIZE); let mut long = - CompactString::from_static_str("i am a long string that will be allocated on the heap"); + CompactString::const_new("i am a long string that will be allocated on the heap"); long.truncate(500); assert_eq!(long.len(), 53); @@ -1273,7 +1266,7 @@ fn test_truncate_panics_on_non_char_boundary() { } #[test_case(CompactString::from; "inline")] -#[test_case(CompactString::from_static_str; "static_str")] +#[test_case(CompactString::const_new; "static_str")] fn test_insert(to_compact: fn(&'static str) -> CompactString) { // insert into empty string let mut one_byte = to_compact(""); @@ -1381,7 +1374,7 @@ fn test_remove_empty_string() { #[test] #[should_panic(expected = "cannot remove a char from the end of a string")] fn test_remove_empty_string_static() { - let mut compact = CompactString::from_static_str(""); + let mut compact = CompactString::const_new(""); compact.remove(0); } @@ -1493,7 +1486,7 @@ fn test_reserve_shrink_roundtrip_static() { // longer than 24 bytes, so the string does not get inlined const TEXT: &str = "Hello, world! How are you today?"; - let mut s = CompactString::from_static_str(TEXT); + let mut s = CompactString::const_new(TEXT); assert!(!s.is_heap_allocated()); assert_eq!(s.capacity(), TEXT.len()); assert_eq!(s, TEXT); @@ -1539,7 +1532,7 @@ fn test_reserve_shrink_roundtrip_static_inline() { // shorter than 12 bytes, so the string gets inlined const TEXT: &str = "Hello."; - let mut s = CompactString::from_static_str(TEXT); + let mut s = CompactString::const_new(TEXT); assert!(!s.is_heap_allocated()); assert_eq!(s.capacity(), MAX_SIZE); assert_eq!(s, TEXT); @@ -1954,7 +1947,7 @@ fn test_is_empty() { ]; assert!(CompactString::new("").is_empty()); - assert!(CompactString::from_static_str("").is_empty()); + assert!(CompactString::const_new("").is_empty()); for (len, s) in ZEROS.iter().copied().enumerate() { let mut a = CompactString::new(s); diff --git a/compact_str/src/traits.rs b/compact_str/src/traits.rs index 401a85a7c..ff00bca9d 100644 --- a/compact_str/src/traits.rs +++ b/compact_str/src/traits.rs @@ -127,7 +127,7 @@ impl ToCompactString for T { &num::NonZeroU128 as s => s.into_repr()?, &num::NonZeroI128 as s => s.into_repr()?, s => { - let mut c = CompactString::new_inline(""); + let mut c = CompactString::const_new(""); write!(c, "{}", s)?; return Ok(c); } @@ -194,14 +194,14 @@ where { fn concat_compact(&self) -> CompactString { self.into_iter() - .fold(CompactString::new_inline(""), |mut s, item| { + .fold(CompactString::const_new(""), |mut s, item| { s.push_str(item.as_ref()); s }) } fn join_compact>(&self, separator: S) -> CompactString { - let mut compact_string = CompactString::new_inline(""); + let mut compact_string = CompactString::const_new(""); let mut iter = self.into_iter().peekable(); let sep = separator.as_ref(); diff --git a/examples/diesel/src/lib.rs b/examples/diesel/src/lib.rs index 049442053..9df000337 100644 --- a/examples/diesel/src/lib.rs +++ b/examples/diesel/src/lib.rs @@ -18,7 +18,7 @@ use crate::models::{ use crate::schema::todos; use crate::schema::todos::dsl::*; -const TITLE: CompactString = CompactString::from_static_str("Say hello!"); +const TITLE: CompactString = CompactString::const_new("Say hello!"); #[test] fn diesel_roundtrip() -> Result<(), Box> { diff --git a/examples/sqlx/src/lib.rs b/examples/sqlx/src/lib.rs index 28c567f24..1c02e8065 100644 --- a/examples/sqlx/src/lib.rs +++ b/examples/sqlx/src/lib.rs @@ -17,7 +17,7 @@ use sqlx::{ }; use tempfile::tempdir; -const TITLE: CompactString = CompactString::from_static_str("Say hello!"); +const TITLE: CompactString = CompactString::const_new("Say hello!"); macro_rules! test_body { ($($test_name:ident($DbPool:path, $DbArguments:path, $compile_only:literal);)*) => {$( diff --git a/fuzz/src/creation.rs b/fuzz/src/creation.rs index a951bda30..8d4e2cbd2 100644 --- a/fuzz/src/creation.rs +++ b/fuzz/src/creation.rs @@ -95,7 +95,7 @@ pub enum Creation<'a> { CollectBoxStr(Vec>), /// Create using [`std::default::Default`] Default, - /// Create using [`CompactString::from_static_str`], using a collection of interesting strings. + /// Create using [`CompactString::const_new`], using a collection of interesting strings. FromStaticStr(u8), } @@ -769,7 +769,7 @@ impl Creation<'_> { 241..=255 => HUGE_STATIC_STR, }; - let compact = CompactString::from_static_str(s); + let compact = CompactString::const_new(s); let std_str = s.to_string(); assert_eq!(compact, std_str);