StringName: disable p_is_static, as codegen usages aren't in static variables

Author: Bromeon

godot-codegen/src/util.rs

@@ -41,7 +41,7 @@ pub fn c_str(string: &str) -> Literal {
 pub fn make_string_name(identifier: &str) -> TokenStream {
     let lit = c_str(identifier);
 
-    quote! { StringName::__static_cstr(#lit) }
+    quote! { StringName::__cstr(#lit) }
 }
 
 pub fn make_sname_ptr(identifier: &str) -> TokenStream {

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

@@ -8,7 +8,6 @@
 use std::fmt;
 
 use godot_ffi as sys;
-use godot_ffi::interface_fn;
 use sys::{ffi_methods, ExtVariantType, GodotFfi};
 
 use crate::builtin::{inner, Encoding, GString, NodePath, Variant};
@@ -84,7 +83,7 @@ impl StringName {
             let is_static = sys::conv::SYS_FALSE;
             let s = unsafe {
                 Self::new_with_string_uninit(|string_ptr| {
-                    let ctor = interface_fn!(string_name_new_with_latin1_chars);
+                    let ctor = sys::interface_fn!(string_name_new_with_latin1_chars);
                     ctor(
                         string_ptr,
                         cstr.as_ptr() as *const std::ffi::c_char,
@@ -245,9 +244,20 @@ impl StringName {
         inner::InnerStringName::from_outer(self)
     }
 
+    #[doc(hidden)] // Private for now. Needs API discussion, also regarding overlap with try_from_cstr().
+    pub fn __cstr(c_str: &'static std::ffi::CStr) -> Self {
+        // This used to be set to true, but `p_is_static` parameter in Godot should only be enabled if the result is indeed stored
+        // in a static. See discussion in https://github.com/godot-rust/gdext/pull/1316. We may unify this into a regular constructor,
+        // or provide a dedicated StringName cache (similar to ClassId cache) in the future, which would be freed on shutdown.
+        let is_static = false;
+
+        Self::__cstr_with_static(c_str, is_static)
+    }
+
     /// Creates a `StringName` from a static ASCII/Latin-1 `c"string"`.
     ///
-    /// This avoids unnecessary copies and allocations and directly uses the backing buffer. Useful for literals.
+    /// If `is_static` is true, avoids unnecessary copies and allocations and directly uses the backing buffer. However, this must
+    /// be stored in an actual `static` to not cause leaks/error messages with Godot. For literals, use `is_static=false`.
     ///
     /// Note that while Latin-1 encoding is the most common encoding for c-strings, it isn't a requirement. So if your c-string
     /// uses a different encoding (e.g. UTF-8), it is possible that some characters will not show up as expected.
@@ -260,17 +270,17 @@ impl StringName {
     /// use godot::builtin::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::__static_cstr(c"\xb1 Latin-1 string");
+    /// let sname = StringName::__cstr(c"\xb1 Latin-1 string");
     /// ```
     #[doc(hidden)] // Private for now. Needs API discussion, also regarding overlap with try_from_cstr().
-    pub fn __static_cstr(c_str: &'static std::ffi::CStr) -> Self {
+    pub fn __cstr_with_static(c_str: &'static std::ffi::CStr, is_static: bool) -> Self {
         // SAFETY: c_str is nul-terminated and remains valid for entire program duration.
         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::bool_to_sys(is_static),
                 )
             })
         }
@@ -488,6 +498,7 @@ mod serialize {
 }
 
 // TODO(v0.4.x): consider re-exposing in public API. Open questions: thread-safety, performance, memory leaks, global overhead.
+// Possibly in a more general StringName cache, similar to ClassId. See https://github.com/godot-rust/gdext/pull/1316.
 /// 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.
@@ -498,6 +509,6 @@ macro_rules! static_sname {
 
         let c_str: &'static std::ffi::CStr = $str;
         static SNAME: OnceLock<StringName> = OnceLock::new();
-        SNAME.get_or_init(|| StringName::__static_cstr(c_str))
+        SNAME.get_or_init(|| StringName::__cstr_with_static(c_str, true))
     }};
 }

godot-macros/src/class/data_models/inherent_impl.rs

@@ -459,7 +459,7 @@ fn add_virtual_script_call(
 
     let code = quote! {
         let object_ptr = #object_ptr;
-        let method_sname = ::godot::builtin::StringName::__static_cstr(#method_name_cstr);
+        let method_sname = ::godot::builtin::StringName::__cstr(#method_name_cstr);
         let method_sname_ptr = method_sname.string_sys();
         let has_virtual_override = unsafe { ::godot::private::has_virtual_script_method(object_ptr, method_sname_ptr) };
 

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

@@ -127,7 +127,7 @@ fn string_name_from_cstr() {
     ];
 
     for (bytes, string) in cases.into_iter() {
-        let a = StringName::__static_cstr(bytes);
+        let a = StringName::__cstr(bytes);
         let b = StringName::from(string);
 
         assert_eq!(a, b);