Skip to content

Commit e8ca64a

Browse files
committed
Use rust str instead of cstr in ClassIdSource
1 parent a335a52 commit e8ca64a

File tree

5 files changed

+19
-62
lines changed

5 files changed

+19
-62
lines changed

godot-codegen/src/generator/classes.rs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,6 @@ fn make_class(class: &Class, ctx: &mut Context, view: &ApiView) -> GeneratedClas
9191

9292
// Strings
9393
let godot_class_str = &class_name.godot_ty;
94-
let class_name_cstr = util::c_str(godot_class_str);
9594

9695
// Idents and tokens
9796
let (base_ty, base_ident_opt) = match class.base_class.as_ref() {
@@ -248,7 +247,7 @@ fn make_class(class: &Class, ctx: &mut Context, view: &ApiView) -> GeneratedClas
248247
// Optimization note: instead of lazy init, could use separate static which is manually initialized during registration.
249248
static CLASS_ID: std::sync::OnceLock<ClassId> = std::sync::OnceLock::new();
250249

251-
let name: &'static ClassId = CLASS_ID.get_or_init(|| ClassId::__alloc_next_ascii(#class_name_cstr));
250+
let name: &'static ClassId = CLASS_ID.get_or_init(|| ClassId::__alloc_next_unicode(#godot_class_str));
252251
*name
253252
}
254253

godot-core/src/meta/class_id.rs

Lines changed: 14 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ use std::any::TypeId;
99
use std::borrow::Cow;
1010
use std::cell::OnceCell;
1111
use std::collections::HashMap;
12-
use std::ffi::CStr;
1312
use std::fmt;
1413
use std::hash::Hash;
1514

@@ -95,7 +94,7 @@ impl ClassId {
9594
"In Godot < 4.4, class name must be ASCII: '{name}'"
9695
);
9796

98-
cache.insert_class_id(ClassIdSource::Owned(name), Some(type_id), false)
97+
cache.insert_class_id(ClassIdSource(Cow::Owned(name)), Some(type_id), false)
9998
}
10099

101100
/// Create a `ClassId` from a runtime string (for dynamic class names).
@@ -105,7 +104,7 @@ impl ClassId {
105104
pub(crate) fn new_dynamic(class_name: String) -> Self {
106105
let mut cache = CLASS_ID_CACHE.lock();
107106

108-
cache.insert_class_id(ClassIdSource::Owned(class_name), None, false)
107+
cache.insert_class_id(ClassIdSource(Cow::Owned(class_name)), None, false)
109108
}
110109

111110
// Test-only APIs.
@@ -127,37 +126,16 @@ impl ClassId {
127126
Self { global_index: 0 }
128127
}
129128

130-
/// Create a new ASCII; expect to be unique. Internal, reserved for macros.
131-
#[doc(hidden)]
132-
pub fn __alloc_next_ascii(class_name_cstr: &'static CStr) -> Self {
133-
let utf8 = class_name_cstr
134-
.to_str()
135-
.expect("class name is invalid UTF-8");
136-
137-
assert!(
138-
utf8.is_ascii(),
139-
"ClassId::alloc_next_ascii() with non-ASCII Unicode string '{utf8}'"
140-
);
141-
142-
let source = ClassIdSource::Borrowed(class_name_cstr);
143-
let mut cache = CLASS_ID_CACHE.lock();
144-
cache.insert_class_id(source, None, true)
145-
}
146-
147129
/// Create a new Unicode entry; expect to be unique. Internal, reserved for macros.
148130
#[doc(hidden)]
149131
pub fn __alloc_next_unicode(class_name_str: &'static str) -> Self {
132+
#[cfg(before_api = "4.4")]
150133
assert!(
151-
cfg!(since_api = "4.4"),
134+
class_name_str.is_ascii(),
152135
"Before Godot 4.4, class names must be ASCII, but '{class_name_str}' is not.\nSee https://github.com/godotengine/godot/pull/96501."
153136
);
154137

155-
assert!(
156-
!class_name_str.is_ascii(),
157-
"ClassId::__alloc_next_unicode() with ASCII string '{class_name_str}'"
158-
);
159-
160-
let source = ClassIdSource::Owned(class_name_str.to_owned());
138+
let source = ClassIdSource(Cow::Borrowed(class_name_str));
161139
let mut cache = CLASS_ID_CACHE.lock();
162140
cache.insert_class_id(source, None, true)
163141
}
@@ -206,15 +184,13 @@ impl ClassId {
206184

207185
impl fmt::Display for ClassId {
208186
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
209-
self.with_string_name(|s| s.fmt(f))
187+
self.to_cow_str().fmt(f)
210188
}
211189
}
212190

213191
impl fmt::Debug for ClassId {
214192
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
215-
let cache = CLASS_ID_CACHE.lock();
216-
let entry = cache.get_entry(self.global_index as usize);
217-
let name = entry.rust_str.as_cow_str();
193+
let name = self.to_cow_str();
218194

219195
if name.is_empty() {
220196
write!(f, "ClassId(none)")
@@ -224,10 +200,6 @@ impl fmt::Debug for ClassId {
224200
}
225201
}
226202

227-
fn ascii_cstr_to_str(cstr: &CStr) -> &str {
228-
cstr.to_str().expect("should be validated ASCII")
229-
}
230-
231203
// ----------------------------------------------------------------------------------------------------------------------------------------------
232204

233205
/// Entry in the class name cache.
@@ -247,31 +219,25 @@ impl ClassIdEntry {
247219
}
248220

249221
fn none() -> Self {
250-
Self::new(ClassIdSource::Borrowed(c""))
222+
Self::new(ClassIdSource(Cow::Borrowed("")))
251223
}
252224
}
253225

254226
// ----------------------------------------------------------------------------------------------------------------------------------------------
255227

256-
/// `Cow`-like enum for class names, but with C strings as the borrowed variant.
257-
enum ClassIdSource {
258-
Owned(String),
259-
Borrowed(&'static CStr),
260-
}
228+
/// `Cow` for class names, with static strings as the borrowed variant.
229+
struct ClassIdSource(Cow<'static, str>);
261230

262231
impl ClassIdSource {
263232
fn to_string_name(&self) -> StringName {
264-
match self {
265-
ClassIdSource::Owned(s) => StringName::from(s),
266-
ClassIdSource::Borrowed(cstr) => StringName::__cstr(cstr),
233+
match &self.0 {
234+
Cow::Owned(s) => StringName::from(s),
235+
Cow::Borrowed(str) => StringName::from(*str),
267236
}
268237
}
269238

270239
fn as_cow_str(&self) -> Cow<'static, str> {
271-
match self {
272-
ClassIdSource::Owned(s) => Cow::Owned(s.clone()),
273-
ClassIdSource::Borrowed(cstr) => Cow::Borrowed(ascii_cstr_to_str(cstr)),
274-
}
240+
self.0.clone()
275241
}
276242
}
277243
// ----------------------------------------------------------------------------------------------------------------------------------------------

godot-core/src/meta/signature.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -470,7 +470,7 @@ impl<'a> CallContext<'a> {
470470
}
471471

472472
/// Call from Godot into a custom Callable.
473-
pub fn custom_callable(function_name: &'a str) -> Self {
473+
pub const fn custom_callable(function_name: &'a str) -> Self {
474474
Self {
475475
class_name: Cow::Borrowed("<Callable>"),
476476
function_name,

godot-macros/src/class/derive_godot_class.rs

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -55,15 +55,7 @@ pub fn derive_godot_class(item: venial::Item) -> ParseResult<TokenStream> {
5555
.map_or_else(|| class.name.clone(), |rename| rename)
5656
.to_string();
5757

58-
// Determine if we can use ASCII for the class name (in most cases).
59-
// Crate godot-macros does not have knowledge of `api-*` features (and neither does user crate where macros are generated),
60-
// so we can't cause a compile error if Unicode is used before Godot 4.4. However, this causes a runtime error at startup.
61-
let class_name_allocation = if class_name_str.is_ascii() {
62-
let c_str = util::c_str(&class_name_str);
63-
quote! { ClassId::__alloc_next_ascii(#c_str) }
64-
} else {
65-
quote! { ClassId::__alloc_next_unicode(#class_name_str) }
66-
};
58+
let class_name_allocation = quote! { ClassId::__alloc_next_unicode(#class_name_str) };
6759

6860
if struct_cfg.is_internal {
6961
modifiers.push(quote! { with_internal })

itest/rust/src/object_tests/class_name_test.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -158,10 +158,10 @@ fn class_name_debug() {
158158
fn class_name_alloc_panic() {
159159
// ASCII.
160160
{
161-
let _1st = ClassId::__alloc_next_ascii(c"DuplicateTestClass");
161+
let _1st = ClassId::__alloc_next_unicode("DuplicateTestClass");
162162

163163
expect_panic("2nd allocation with same ASCII string fails", || {
164-
let _2nd = ClassId::__alloc_next_ascii(c"DuplicateTestClass");
164+
let _2nd = ClassId::__alloc_next_unicode("DuplicateTestClass");
165165
});
166166
}
167167

0 commit comments

Comments
 (0)