Merge pull request #1307 from beicause/string-name-cstr-non-static

Remove static lifetime in `StringName::from(&CStr)`

Author: Bromeon

godot-core/src/builtin/mod.rs

@@ -41,7 +41,7 @@ pub mod __prelude_reexport {
     pub use rect2i::*;
     pub use rid::*;
     pub use signal::*;
-    pub use string::{Encoding, GString, NodePath, StringName};
+    pub use string::{static_name, Encoding, GString, NodePath, StringName};
     pub use transform2d::*;
     pub use transform3d::*;
     pub use variant::*;

godot-core/src/builtin/string/mod.rs

@@ -22,6 +22,7 @@ pub use string_name::*;
 use crate::meta;
 use crate::meta::error::ConvertError;
 use crate::meta::{FromGodot, GodotConvert, ToGodot};
+pub use crate::static_name;
 
 impl GodotConvert for &str {
     type Via = GString;

godot-core/src/builtin/string/string_name.rs

@@ -35,9 +35,8 @@ use crate::{impl_shared_string_api, meta};
 ///
 /// # Performance
 ///
-/// The fastest way to create string names is by using null-terminated C-string literals such as `c"MyClass"`. These have `'static` lifetime and
-/// can be used directly by Godot, without allocation or conversion. The encoding is limited to Latin-1, however. See the corresponding
-/// [`From<&'static CStr>` impl](#impl-From<%26CStr>-for-StringName).
+/// The fastest way to create string names is using [`static_name!`][crate::builtin::static_name], which creates a static cached `StringName` from null-terminated C-string literals such as `c"MyClass"`. These can be used directly by Godot without conversion. The encoding is limited to Latin-1, however. See the corresponding
+/// [`From<&CStr>` impl](#impl-From<%26CStr>-for-StringName).
 ///
 /// # All string types
 ///
@@ -249,11 +248,6 @@ impl StringName {
     pub fn as_inner(&self) -> inner::InnerStringName<'_> {
         inner::InnerStringName::from_outer(self)
     }
-
-    /// Increment ref-count. This may leak memory if used wrongly.
-    fn inc_ref(&self) {
-        std::mem::forget(self.clone());
-    }
 }
 
 // SAFETY:
@@ -360,8 +354,8 @@ impl From<&NodePath> for StringName {
     }
 }
 
-impl From<&'static std::ffi::CStr> for StringName {
-    /// Creates a `StringName` from a static ASCII/Latin-1 `c"string"`.
+impl From<&std::ffi::CStr> for StringName {
+    /// Creates a `StringName` from a ASCII/Latin-1 `c"string"`.
     ///
     /// This avoids unnecessary copies and allocations and directly uses the backing buffer. Useful for literals.
     ///
@@ -375,23 +369,17 @@ impl From<&'static std::ffi::CStr> for StringName {
     /// // '±' is a Latin-1 character with codepoint 0xB1. Note that this is not UTF-8, where it would need two bytes.
     /// let sname = StringName::from(c"\xb1 Latin-1 string");
     /// ```
-    fn from(c_str: &'static std::ffi::CStr) -> Self {
+    fn from(c_str: &std::ffi::CStr) -> Self {
         // SAFETY: c_str is nul-terminated and remains valid for entire program duration.
-        let result = unsafe {
+        unsafe {
             Self::new_with_string_uninit(|ptr| {
                 sys::interface_fn!(string_name_new_with_latin1_chars)(
                     ptr,
                     c_str.as_ptr(),
-                    sys::conv::SYS_TRUE, // p_is_static
+                    sys::conv::SYS_FALSE, // p_is_static
                 )
             })
-        };
-
-        // StringName expects that the destructor is not invoked on static instances (or only at global exit; see SNAME(..) macro in Godot).
-        // According to testing with godot4 --verbose, there is no mention of "Orphan StringName" at shutdown when incrementing the ref-count,
-        // so this should not leak memory.
-        result.inc_ref();
-        result
+        }
     }
 }
 
@@ -500,3 +488,30 @@ mod serialize {
         }
     }
 }
+
+/// Creates and gets a reference to a static `StringName` from a ASCII/Latin-1 `c"string"`.
+///
+/// This is the fastest way to create a StringName repeatedly, with the result being cached and never released, like `SNAME` in Godot source code. Suitable for scenarios where high performance is required.
+#[macro_export]
+macro_rules! static_name {
+    ($str:literal) => {{
+        use std::sync::OnceLock;
+
+        use godot::sys;
+
+        let c_str: &'static std::ffi::CStr = $str;
+        static SNAME: OnceLock<StringName> = OnceLock::new();
+        SNAME.get_or_init(|| {
+            // SAFETY: c_str is nul-terminated and remains valid for entire program duration.
+            unsafe {
+                StringName::new_with_string_uninit(|ptr| {
+                    sys::interface_fn!(string_name_new_with_latin1_chars)(
+                        ptr,
+                        c_str.as_ptr(),
+                        sys::conv::SYS_TRUE, // p_is_static
+                    )
+                })
+            }
+        })
+    }};
+}

itest/rust/src/builtin_tests/string/string_name_test.rs

@@ -7,7 +7,7 @@
 
 use std::collections::HashSet;
 
-use godot::builtin::{Encoding, GString, NodePath, StringName};
+use godot::builtin::{static_name, Encoding, GString, NodePath, StringName};
 
 use crate::framework::{assert_eq_self, itest};
 
@@ -134,6 +134,30 @@ fn string_name_from_cstr() {
     }
 }
 
+#[itest]
+fn string_name_static_name() {
+    let a = static_name!(c"pure ASCII\t[~]").clone();
+    let b = StringName::from("pure ASCII\t[~]");
+
+    assert_eq!(a, b);
+
+    let a1 = a.clone();
+    let a2 = static_name!(c"pure ASCII\t[~]").clone();
+
+    assert_eq!(a, a1);
+    assert_eq!(a1, a2);
+
+    let a = static_name!(c"\xB1").clone();
+    let b = StringName::from("±");
+
+    assert_eq!(a, b);
+
+    let a = static_name!(c"Latin-1 \xA3 \xB1 text \xBE").clone();
+    let b = StringName::from("Latin-1 £ ± text ¾");
+
+    assert_eq!(a, b);
+}
+
 #[itest]
 fn string_name_with_null() {
     // Godot always ignores bytes after a null byte.