Cogitri commented 3 years ago

When I have a struct like:

    #[derive(Debug, CompositeTemplate)]
    #[template(resource = "/dev/Cogitri/test/ui/my_window.ui")]
    pub struct MyWindow {
        #[template_child]
        pub add_data_button: TemplateChild<gtk::Button>,
    }

And init it like so:

        fn new() -> Self {
            Self {
                add_data_button: TemplateChild::default(),
            }
       }

I don't get any error message during compilation or when creating the object. However, once I access add_data_button, the app crashes:

thread 'main' panicked at 'assertion failed: !self.ptr.is_null()', /var/home/rasmus/Projects/Jinsei/build/target/cargo-home/git/checkouts/gtk4-rs-e74ad56283dfeb5e/64d0e1c/gtk4/src/subclass/widget.rs:1000:13
stack backtrace:
   0: std::panicking::begin_panic
             at /var/home/rasmus/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/panicking.rs:505
   1: <gtk4::subclass::widget::TemplateChild<T> as core::ops::deref::Deref>::deref
             at ./build/target/cargo-home/git/checkouts/gtk4-rs-e74ad56283dfeb5e/64d0e1c/gtk4/src/subclass/widget.rs:1000
   2: health::windows::window::imp::HealthWindow::connect_handlers
             at ./src/windows/window.rs:158
   3: <health::windows::window::imp::HealthWindow as glib::subclass::object::ObjectImpl>::constructed
             at ./src/windows/window.rs:129
   4: glib::subclass::object::constructed
             at ./build/target/cargo-home/git/checkouts/gtk-rs-48ef14c1f17c79fb/c58fab4/glib/src/subclass/object.rs:109
   5: <unknown>
   6: g_object_newv
   7: glib::object::Object::new_internal
             at ./build/target/cargo-home/git/checkouts/gtk-rs-48ef14c1f17c79fb/c58fab4/glib/src/object.rs:1165
   8: glib::object::Object::with_type
             at ./build/target/cargo-home/git/checkouts/gtk-rs-48ef14c1f17c79fb/c58fab4/glib/src/object.rs:1105
   9: glib::object::Object::new
             at ./build/target/cargo-home/git/checkouts/gtk-rs-48ef14c1f17c79fb/c58fab4/glib/src/object.rs:1074
  10: health::windows::window::HealthWindow::new
             at ./src/windows/window.rs:213
  11: <health::core::application::imp::HealthApplication as gio::subclass::application::ApplicationImpl>::activate
             at ./src/core/application.rs:57
  12: gio::subclass::application::application_activate
             at ./build/target/cargo-home/git/checkouts/gtk-rs-48ef14c1f17c79fb/c58fab4/gio/src/subclass/application.rs:368
  13: g_signal_emit_valist
  14: g_signal_emit
  15: <unknown>
  16: <T as gio::subclass::application::ApplicationImplExt>::parent_local_command_line
             at ./build/target/cargo-home/git/checkouts/gtk-rs-48ef14c1f17c79fb/c58fab4/gio/src/subclass/application.rs:232
  17: gio::subclass::application::ApplicationImpl::local_command_line
             at ./build/target/cargo-home/git/checkouts/gtk-rs-48ef14c1f17c79fb/c58fab4/gio/src/subclass/application.rs:99
  18: gio::subclass::application::application_local_command_line
             at ./build/target/cargo-home/git/checkouts/gtk-rs-48ef14c1f17c79fb/c58fab4/gio/src/subclass/application.rs:411
  19: g_application_run
  20: <O as gio::application::ApplicationExtManual>::run
             at ./build/target/cargo-home/git/checkouts/gtk-rs-48ef14c1f17c79fb/c58fab4/gio/src/application.rs:23
  21: health::main
             at ./src/main.rs:27
  22: core::ops::function::FnOnce::call_once
             at /var/home/rasmus/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ops/function.rs:

Which makes sense, but it'd be nice if it either crashed during construction with a more helpful error message (or maybe even when compiling).

Tested w/ e0cbea57

ids1024 commented 3 years ago

(or maybe even when compiling)

I suppose since the struct is being processed by the CompositeTemplate proc macro, this would indeed be possible.

It would also be possible to make it less verbose by either having #[template_child] automatically wrap a member with TemplateChild, or removing #[template_child] and auto-detecting TemplateChild members. The former seems too magical and unclear, the latter might be alright.

bilelmoussaoui commented 2 years ago

The issue with the second suggestion is that you can no longer set a different id for the type or mark it as internal-child.

jf2048 commented 2 years ago

The first suggestion cannot be done without making it an attribute macro. At bare minimum we probably want to use the better error message from TemplateChild::get inside that Deref impl.

The other things we do can are:

Try to parse the XML inside the macro and check for the existence of the ids. This should work but cannot be done with resource templates. We can only do null checking here, not type checking.
Add another trait method for some null + type checking during init_template. This should work in all cases.

jf2048 commented 2 years ago

943 improves the runtime error message. For compile time checks, I think the best we could do is try a string match on the type `TemplateChild` and throw a warning

gtk-rs / gtk4-rs

Forgetting to add #[template_child] to TemplateChild members leads to runtime crashes when accessing the member #179

943 improves the runtime error message. For compile time checks, I think the best we could do is try a string match on the type `TemplateChild` and throw a warning

gtk-rs / gtk4-rs

Forgetting to add #[template_child] to TemplateChild members leads to runtime crashes when accessing the member #179

943 improves the runtime error message. For compile time checks, I think the best we could do is try a string match on the type TemplateChild and throw a warning

943 improves the runtime error message. For compile time checks, I think the best we could do is try a string match on the type `TemplateChild` and throw a warning