Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add glib::signals! macro #1577

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
+525 −8
Draft

Add glib::signals! macro #1577

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 11 additions & 5 deletions examples/object_subclass/author.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,13 @@
use glib::prelude::*;
use glib::subclass::prelude::*;
use glib::subclass::Signal;
use glib::Properties;
use glib::subclass::object::DerivedObjectSignals;
use std::cell::RefCell;
use std::sync::OnceLock;

mod imp {
use super::*;

#[derive(Properties, Default)]
#[derive(glib::Properties, Default)]
#[properties(wrapper_type = super::Author)]
pub struct Author {
#[property(get, set)]
Expand All @@ -20,11 +19,17 @@ mod imp {
surname: RefCell<String>,
}

#[glib::signals(wrapper_type = super::Author)]
impl Author {

#[signal]
fn awarded(&self) {}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if it would be possible to make

Suggested change
fn awarded(&self) {}
fn awarded(&self);

and that means that the signal has no class handler, and

Suggested change
fn awarded(&self) {}
fn awarded(&self) { println!("stuff"); }

which means that it has a class handler with the body of the function.

In the first case, the macro would have to remove the whole function after expansion.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I almost never use class handlers in my signals so I would appreciate something like that too.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A function without a block isn't valid rust syntax outside of an extern block. I don't want to add some invalid Syntax that would be removed by the macro.

It's also way more complicated to parse this. I would have to write a custom parser based on syn::ItemImpl that accepts these "invalid" functions. And it confuses other parsing code for other macros or rustfmt.

I can add and parse a toplevel pseudo-macro, meaning something like this would be ok:

#[glib::signals(wrapper_type = super::FooObject)]
impl FooObject {
    signals! {
        fn awarded() -> i32;
    }
}

This pseudo-macro is valid Rust Syntax and should work with rustfmt and other macros.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe instead of using an impl Object, the usage could be trait ObjectSignals making it possible to provide a default impl?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Default implementations can only use methods provided by the trait's required subtraits and itself.

The class handler implementations would have to reside in an impl ObjectSignals for FooObject block.

I don't really see how I'd map a trait to a definition of signals, as I don't want the macro to modify the trait in ways that make it hard for a developer to understand what the trait will actually look like.

The problem I see is that a semicolon instead of a block in a trait definition means that you have to provide the class handler in your implementation of that trait, while you want it to mean that there is no class handler.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The trait itself wouldn't be used. You can then impl T for SomeObject and forward the calls when the trait has an impl, no?

Anyways, zbus is using something similar for it zbus::proxy! macro.

Copy link
Contributor Author

@PJungkamp PJungkamp Nov 25, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's an open issue around #[zbus::proxy] eating the trait definition.
I don't want to introduce these weird macro-related edge cases into glib.

What do you think about this?

#[glib::signals(wrapper_type = super::FooObject)]
trait FooObjectSignals {
    fn simple_signal(&self);
    
    fn complex_signal_without_class_handler(&self) -> i32;
    
    #[signal(class_handler, run_first]
    fn complex_signal_with_class_handler(&self, param: i32) -> i32;
}

impl FooObjectSignals for FooObject {
    fn complex_signal_with_class_handler(&self, param: i32) -> i32 {
        10
    }
}

I would make the macro produce something like:

trait FooObjectSignals: glib::subclass::types::ObjectSubclass<Type = super::FooObject> {
    // These functions have to be removed from the trait since they doesn't have a class handler one could implement:
    //     simple_signal(&self); 
    //     complex_signal_without_class_handler(&self) -> i32;
    // Providing a function body for signals without class handlers is forbidden. 
    
    // This function is allowed to have a default implementation but does not need to have one.    
    fn complex_signal_with_class_handler(&self) -> i32;
    
    #[doc(hidden)]
    fn __derived_signals() {
        static SIGNALS: std::sync::OnceLock<[glib::subclass::signal::Signal; 3]> = std::sync::OnceLock::new();
        SIGNALS.get_or_init(|| [
            glib::subclass::signal::Signal::builder("simple_signal").build(),
            glib::subclass::signal::Signal::builder("complex_signal_without_class_handler")
                .param_types([<i32 as glib::types::StaticType>::static_type()])
                .return_type::<i32>()
                .build(),
            glib::subclass::signal::Signal::builder("complex_signal_with_class_handler")
                .param_types([<i32 as glib::types::StaticType>::static_type()])
                .return_type::<i32>()
                .class_handler(|params| {
                    Some(<i32 as glib::value::ToValue>::to_value(
                        &Self::complex_signal_with_class_handler(
                            glib::subclass::types::ObjectSubclassExt::from_obj(&params[0].get::<Self::Type>().unwrap()),
                            params[1].get::<i32>().unwrap(),
                        )
                    )
                })
                .build(),
        ])
    }
}

A #[glib::derived_signals] macro could then be used to insert the __derived_signals function call into the ObjectImpl::signals function.

#[glib::derived_signals]
impl ObjectImpl for FooObject {}

}

#[glib::derived_properties]
impl ObjectImpl for Author {
fn signals() -> &'static [Signal] {
static SIGNALS: OnceLock<Vec<Signal>> = OnceLock::new();
SIGNALS.get_or_init(|| vec![Signal::builder("awarded").build()])
Self::derived_signals()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be nice to add a derived_signals macro as well, not much work and would make the migration to use the future signals macro more appealing

}
}

Expand All @@ -38,6 +43,7 @@ mod imp {
glib::wrapper! {
pub struct Author(ObjectSubclass<imp::Author>);
}

impl Author {
pub fn new(name: &str, surname: &str) -> Self {
glib::Object::builder()
Expand Down
19 changes: 18 additions & 1 deletion glib-macros/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,14 @@ mod properties;
mod shared_boxed_derive;
mod value_delegate_derive;
mod variant_derive;
mod signals;

mod utils;

use flags_attribute::AttrInput;
use proc_macro::TokenStream;
use proc_macro2::Span;
use syn::{parse_macro_input, DeriveInput};
use syn::{parse_macro_input, DeriveInput, Error};
use utils::{parse_nested_meta_items_from_stream, NestedMetaItem};

/// Macro for passing variables as strong or weak references into a closure.
Expand Down Expand Up @@ -1587,3 +1588,19 @@ pub fn derive_value_delegate(input: TokenStream) -> TokenStream {
pub fn async_test(args: TokenStream, item: TokenStream) -> TokenStream {
async_test::async_test(args, item)
}

#[proc_macro_attribute]
pub fn signals(attr: TokenStream, item: TokenStream) -> TokenStream {
let args = parse_macro_input!(attr as signals::SignalsArgs);
match syn::parse::<syn::ItemImpl>(item) {
Ok(input) => signals::impl_signals(input, args)
.unwrap_or_else(syn::Error::into_compile_error)
.into(),
Err(_) => Error::new(
Span::call_site(),
signals::WRONG_PLACE_MSG,
)
.into_compile_error()
.into(),
}
}
Loading
Loading