Refer to #[func] names in type-safe ways (not just strings)

[kang-sw]

I haven't been using this library for long, but it seems common in Rust code to connect symbols from GodotClass to signals of other script classes. For this, I end up creating StringName FFI instances to specify the identifiers for these symbols. To make this process more convenient, I wrote a macro.

#[macro_export]
macro_rules! string_name {
    ($base:ty, $ident:ident) => {{
        // Just ensure ident exist
        let str = $crate::string_name!(#[str], $ident);
        let _ = <$base>::$ident; // Generates compile-time error

        $crate::string_name!(#[set], str)
    }};

    ($name:literal) => {{
        $crate::string_name!(#[set], concat!($name, "\0").as_bytes())
    }};

    (#[str], $id:ident) => {
        concat!(stringify!($ident), "\0").as_bytes()
    };

    (#[set], $str:expr) => {
        // XXX: Optimization
        // - Is it cheap enough to create everytime?
        // - Otherwise, is it safe to store in OnceLock?
        godot::prelude::StringName::from_latin1_with_nul($str)
    };
}

In particular, I often call registered methods declared within the #[godot_api] block of Rust's GodotClass. The following usage simplifies this process and helps prevent human errors such as typos.

        // BEFORE
        timer.connect(
            "timeout".into(),
            Callable::from_object_method(
                &self.to_gd(),
                "__internal_try_start_frame_inner".into(),
            ),
        );

        // THEN
        timer.connect(
            string_name!("timeout"), // Godot side signal name; no way to find compile-time symbol!
            Callable::from_object_method(
                &self.to_gd(),
                string_name!(Self, __internal_try_start_frame_inner),
                //           ^^^^ `$base:ty` takes this 
                // + IDE support, compile time error, ...
            ),
        );

I think it would be beneficial for this type of API to be added to the library, especially if the #[godot_api] generator could also create type tags for methods connected to scripts. This would prevent accidentally creating symbol names for non-script methods with the string_name macro.

Here's a simple example of a compile-time verification macro that is partially supported by proc-macro.

#[godot_api]
impl MyClass {
   #[func]
    fn script_method() {}
}

impl MyClass {
    fn non_script_method() {}
}

// -- generated --
#[doc(hidden)]
struct __ScriptSymbols_MyClass {
  script_method: &'static str,
}

impl MyClass {
  #[doc(hidden)]
  const __SYMBOLS: __ScriptSymbols_MyClass {
    script_method: "script_method", // Later, it'll be able to handle aliases
  };
}

// -- usage

// let name: StringName = symbol_name!(MyClass, non_script_method);
//                                              ^^^^^^^^^^^^^^^^^
//         Compile Error: `non_script_method` not found in `__ScriptSymbols_MyClass`

let name: StringName = symbol_name!(MyClass, script_method);

// -- expands to
let name: StringName = {
  // Check if symbol exist
  let __sym = <$type> :: __SYMBOLS . $ident;
  
  // Build identifier name.
  StringName::from_latin1_with_nul(__sym.as_bytes())
};

I believe this approach would allow us to handle Rust bindings more safely than simply referencing symbols through strings.

[kang-sw]

Actually I wanted to write a macro which accepts parameter as path::Type::ident, but couldn't retrieve the last token from path. 😢

[lilizoey]

i think this is related to #531
it'd probably be better to use c-string literals for this when it's stabilized (next rust release?).

is there a reason you dont just do this?

timer.connect(
    "timeout".into(),
    Callable::from_object_method(
        &self.to_gd(),
        "__internal_try_start_frame_inner".into()),
    ),
);

or alternatively

timer.connect(
    "timeout".into(),
    self.base().callable("__internal_try_start_frame_inner"),
);

for type safety: ideally we'd avoid needing to use StringNames directly in most cases, like for referencing methods and signals, and have some kind of abstraction that just lets us use them directly. like the above could maybe be rewritten as

timer.connect(
    self.timeout,
    Self::__internal_try_start_frame_inner
)

but im not sure how feasible that is

[kang-sw]

This is basically to avoid using string literals whenever possible. Since we're generating bindings from rust code, we can benefit from a lot of compile-time information, and the name of symbol, is of course one of them.

Putting any "linkage" behind a string literal throws a lot of advantages into darkness;

1. We can't even get noticed whether there's symbol name error or not, until the execution reaches that point and throws runtime error.
2. Even runtime error was thrown, it's tricky to track the exact source of error as runtime backtrace is lost during cross-language boundary. Of course we can attach to the process and debug it; however, it's pretty costly in productivity term.
3. Refactoring in rust is fairly powerful, at least, compared to manually finding-and-replacing all string literal occurences.

Even with the simple macro that I provided first, all above disadvantages can be avoided. And if with the proc-macro support, we can even determine that given rust symbol is bound to Godot script or not, in compile time.

---

timer.connect(
    "timeout".into(),
    self.base().callable("__internal_try_start_frame_inner"),
);

Fixed original content to use into() expression, I didn't know this was possible; thanks!

[Bromeon]

@lilizoey

like the above could maybe be rewritten as

timer.connect(
    self.timeout,
    Self::__internal_try_start_frame_inner
)

but im not sure how feasible that is

Even if the function type can somehow be made working, the problem is that it has no name information associated.

We could technically use a HashMap<EnumThatHoldsAllSignatureTypes, String> to map to name, but that would again mean error checking at runtime. Also, it needs extra memory and significant implementation complexity just for the syntax. Furthermore, we couldn't detect at compile time whether a passed function has an actual #[func] or not.

A macro is the way to go here (separator to be discussed):

func_ref!(method); // self.callable("method") + checks
func_ref!(obj.method); // obj.callable("method") + checks

---

@kang-sw Small request for the future, please don't edit your initial message in a way that makes the conversation flow hard to follow -- the "is there a reason you dont just do this?" question makes no sense once you replace your post with those very suggestions, and it's very unclear where the value of string_name!("timeout") would be 🙂

Are there other symbols that need to be referenced statically than #[func]? Signals don't have first-class support in gdext yet and the #[signal] API will undergo breaking changes, so I don't want to invest any effort on that side.

[kang-sw]

please don't edit your initial message in a way that makes the conversation flow hard to follow

Apologies for making confusion ... 😢, rolling back again would make another additional
confusion, maybe ..?

---

Are there other symbols that need to be referenced statically than #[func]? Signals don't have first-class support in gdext yet and the #[signal] API will undergo breaking changes, so I don't want to invest any effort on that side.

Well I think most of the use case of "binding something FROM rust TO godot" would occur with
Callable, therefore basically I believe that other than method name symbols don't need
to be considered that seriously.

Of course, as long as we can retrieve static symbols from anything we expose, I think that
discouraging use of string literal would be better way to go.

---

Anyways, yes, it seems I underestimated the complexity of making verification layer of
static symbols. For some excuse, this is some additional idea of I thought initially, just
take a look for fun:

1. Currently following code(a) generates something like this(b):

   // (a) user code
   #[godot_api]
   impl Foo {
       #[func]
       fn foo(&self) {}
   }
   
   // (b) generated
   impl ::godot::obj::cap::ImplementsGodotApi for Foo {
       fn __register_methods() {
           {
               ...
               let method_name = StringName::from("foo");
               ... 
           }
       }
   }

2. Instead of declaring string literal from detected symbol name foo, let's define a new
   type which represents a rust symbol that was exposed to godot

   // in godot::somewhere
   pub struct MethodSymbol {
       pub name: &'static str,
       // Maybe some additional metadata; e.g. alias?
   }
   
   pub struct ConstantSymbol { ... }
   pub struct FieldSymbol { ... }

3. And in #[godot_api], from user code(c), generate list of symbols, and define a
   struct(d) which gathers all symbols in one place. Then create a constant(e) which actually
   fills actual symbols within it.

   // (c) user code
   #[godot_api]
   impl Foo {
       #[func]
       fn foo(&self) {}
       
       #[func]
       fn bar(&self, param: i32) {}
   }
   
   // (d) generate a struct that gathers all symbols in one place
   
   #[doc(hidden)] // Don't need to be exposed to user
   struct __GeneratedSymbols_Foo {
       // For each exposed symbols, create corresponding struct field
       pub foo: MethodSymbol,
       
       // 
       pub bar: MethodSymbol,
   }
   
   // (e) Create a constant that holds actual symbol names
   
   impl Foo {
       #[doc(hidden)] // Don't need to be exposed to user
       const __SYMBOLS: __GeneratedSymbols_Foo {
           foo: MethodSymbol { name: "foo" },
           bar: MethodSymbol { name: "bar" },
       }
   }

4. Additionally, let the binding generator of section 1 (b) to use this symbol instead.

   impl ::godot::obj::cap::ImplementsGodotApi for Foo {
       fn __register_methods() {
           let symbols = &Foo::__SYMBOLS;
           {
               ...
               let method_name = StringName::from(symbols.foo.name);
               ... 
           }
       }
   }

5. With this setup, we can define following macro(similar way you mentioned):

    // Verbose version of macro; which simply let user specify type of method.
    let callable = func_ref!(
        obj, // Any expression results in instance of `ToGd`
        some::my::gd::Foo, // type name of obj
        foo, // Name of method symbol
    );

   This macro would be expanded like following code:

    let callable = /*expansion*/ {
        fn __symbol_name_error() {
            let _: &MethodSymbol = &some::my::gd::Foo.__SYMBOLS.foo;
            //                    ^^^^^^^^^^^^^^^^^ $class:ty ^^^ $method:ident
            //
            //  When user mistakes:
            //  Catch (1) Name `xxx` not found
            //  Catch (2) `xxx` is not `MethodSymbol`
        }
        
        Callable::from_object_method(
            &ToGd::to_gd(&obj),
            //            ^^^ $gd:expr
            StringName::from(some::my::gd::Foo.__SYMBOLS.foo.name)
            //               ^^^^^^^^^^^^^^^^^           ^^^
            //               $class:ty                   $method:ident
        )
    };

   With this macro, we can check if:

   • Did the user typed correct symbol name?
   • Is the symbol name is of method?

6. However, in this setup, we can't determine if the object and the method bound
   correctly; i.e. what if obj is not subclass of Foo?

   For this, let's add generic parameter on method symbol(a), and define a method which
   verifies given parameter is subclass of this method symbol(b).

   pub struct MethodSymbol<C: GodotClass> {...}
   //                      ^^^ (a)
   
   impl<C: GodotClass> MethodSymbol<C> {
       // (b)
       #[doc(hidden)] // No need for user
       pub fn __of<T:Inherits<C>>(_ref: &Gd<T>) {}
   }

   Now the above macro expansion of section 5, changes into this way:

   let callable = /*expansion*/ {
       let __sym: &MethodSymbol = &some::my::gd::Foo.__SYMBOLS.foo;
       //                    ^^^^^^^^^^^^^^^^^ $class:ty ^^^ $method:ident
       
       let __gd = ToGd::to_gd(&obj),
       //                      ^^^ $gd:expr
       
       __sym.__of(&__gd);
       //    ^^^ Now catch (3), `xxx` does not implmeent `Inherits<Foo>`
       
       Callable::from_object_method(
           &__gd,
           StringName::from(some::my::gd::Foo.__SYMBOLS.foo.name)
       )
   };

   Now we can catch if user bound the function symbol correctly,

   • in compile time
   • without runtime overhead!

7. Yet, this approach limited to userspace [godot_api]s. Perhaps we can extend the
   symbol generation for every builtin types, in similar manner?

   And additionally, making every generated symbol class __GeneratedSymbols_* to
   implement Deref (a) to its superclass will make this approach much more flexible;

   // Assume that the gdext library generated `Node.__SYMBOLS` for `Node`
   
   #[derive(GodotClass)]
   #[class(base = Node)]
   pub struct Bar {
        ...
   }
   
   #[godot_api]
   impl Bar {
      fn my_bar(&self) {}
   }
   
   // --- generates following code:
   
   impl __GeneratedSymbols_Bar {
      pub my_bar: MethodSymbol<Bar>,
   }
   
   // (a) Generate `deref` trait to its superclass; which makes above macro work with its superclass.
   impl std::ops::Deref for __GeneratedSymbols_Bar {
      type Target = __GeneratedSymbols_Node;
      
      fn deref(&self) -> &Self::Target {
          &Node.__SYMBOLS
      }
   }   

[Bromeon]

I'd say we follow YAGNI and not try to find the perfect bullet-proof system. After all, the title of this issue is

Adding simple macro for building StringName from rust symbols?

😁 Adding hundreds of LoC for a quality-of-life change may not be warranted; this code also needs to be maintained. Last, we need to keep in mind that generating code is not free: it takes time to generate, and takes again time to be parsed by the compiler on user side. So, the less symbols we have to generate, the better. This multiplies with every single user class, which may well be noticeable in larger projects.

Your detailed analysis is very valuable, thanks for taking the time! 🙂 Maybe we can agree on some of the points for an initial implementation.

---

If we can refer to user-defined #[func] in the same class via identifiers instead of strings, that's already a big win and likely enough for most use cases. So your suggestion in 3) sounds great:

3. And in #[godot_api], from user code(c), generate list of symbols, and define a
struct(d) which gathers all symbols in one place. Then create a constant(e) which actually
fills actual symbols within it.

Do we even need the MethodSymbol struct? Because we just want to verify that something exists, so the fields bar and foo could directly have &'static str type. Or even unit () -- as the string can also be obtained via stringify!() from the macro invocation.

If we ever need to add other metadata, we can still do that later...

---

4. Additionally, let the binding generator of section 1 (b) to use this symbol instead.

That code is not user-facing -- the generator knows the symbols. So indirection here would add more coupling without real benefits.

---

6. However, in this setup, we can't determine if the object and the method bound
correctly; i.e. what if obj is not subclass of Foo?
For this, let's add generic parameter on method symbol(a), and define a method which
verifies given parameter is subclass of this method symbol(b).

Conceptually, it seems wrong that MethodSymbol has a generic parameter -- all symbols in one class will have the same argument type, namely the one of the enclosing Self. If the user invokes

 let callable = func_ref!(
     obj, // Gd instance
     module::MyClass, // class type
     foo, // #[func] name
 );

, then the macro can just do something like

fn __type_check<T>(_obj: &Gd<T>) {}

__type_check::<$Class>(&$obj)

which would not compile if obj would point to a different type than module::MyClass.

---

On another note, I would probably not add a separate trait ToGd. Just expect the user to pass a Gd<T> or &Gd<T> instance, or assume self if nothing is passed.

let callable = func_ref!(foo);

let obj: Gd<Other>;
let callable = func_ref!(obj, foo; module::MyClass);

[kang-sw]

Thanks for detailed answer! Following are about initial intentions, and some corrections of myself:

---

Adding hundreds of LoC for a quality-of-life change may not be warranted

Agree. I noticed that things are not going 'simple' anymore .. Just let's reinterpret the term 'simplicity' as for user side; that frees user from worrying about hidden typo 😁.

---

Do we even need the MethodSymbol struct?

Initially that was to differentiate method names from other kind of symbols within __SYMBOLS struct, however, as long as we access to the symbols via macros; that knows what they're referring to(e.g. func_ref only need to find method name), I agree that there's no additional type bound required; Something like below would be sufficient without MethodSymbol

// Generate code
impl MyClass {
    // Only stores method reference names, it's enough to prevent user 
    // from binding field or constant, or static method.
    const __METHOD_SYMBOLS: __GeneratedMethodSymbols_MyClass = ..;
    const __FIELD_SYMBOLS: __GeneratedMethodSymbols_MyClass = ..;
}

---

Conceptually, it seems wrong that MethodSymbol has a generic parameter

...

fn __type_check<T>(_obj: &Gd<T>) {}

__type_check::<$Class>(&$obj)

Ah yes, this is a shard that I missed during cluttered brainstorming 😄. I agree that above macro usage already provides every information to verify type safety. Thanks for pointing out!

---

That code is not user-facing -- the generator knows the symbols.

plus

I would probably not add a separate trait ToGd.

For both, I just didn't have any concrete idea what the internal implementation / exposed interface should look like. Thanks for detailed answer again, it's actually very helpful for me either learning these kind of insight!

[Bromeon]

Thanks as well for taking the time to look into the current codegen implementation! I know it's not easy to understand 😅

But your timing is really good, last month I did two massive refactors (1, 2) to simplify those parts. So could have been worse 😬