From ad9e790f7c5ab164d6dfc5cc5c9256951714e548 Mon Sep 17 00:00:00 2001 From: Antoni Boucher Date: Tue, 22 Oct 2019 05:52:35 -0400 Subject: [PATCH 01/15] Add clone macro --- Cargo.toml | 2 ++ examples/clone.rs | 61 +++++++++++++++++++++++++++++++++++++++++ src/clone.rs | 62 ++++++++++++++++++++++++++++++++++++++++++ src/lib.rs | 2 ++ tests/clone_panic.rs | 65 ++++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 192 insertions(+) create mode 100644 examples/clone.rs create mode 100644 src/clone.rs create mode 100644 tests/clone_panic.rs diff --git a/Cargo.toml b/Cargo.toml index a0db5345..5d26137a 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -33,6 +33,8 @@ glib-sys = { git = "https://github.com/gtk-rs/sys" } gobject-sys = { git = "https://github.com/gtk-rs/sys" } [dev-dependencies] +gio = { git = "https://github.com/gtk-rs/gio" } +gtk = { git = "https://github.com/gtk-rs/gtk" } tempfile = "3" [features] diff --git a/examples/clone.rs b/examples/clone.rs new file mode 100644 index 00000000..1ff95e0d --- /dev/null +++ b/examples/clone.rs @@ -0,0 +1,61 @@ +extern crate gio; +extern crate glib; +extern crate gtk; + +use std::cell::RefCell; +use std::rc::Rc; + +use gio::{ApplicationExt, ApplicationExtManual}; +use gtk::{ + Application, + ApplicationWindow, + Button, + ButtonExt, + ContainerExt, + GtkWindowExt, + WidgetExt, +}; +use glib::clone; + +struct State { + started: bool, + count: i32, +} + +impl State { + fn new() -> Self { + Self { + started: false, + count: 0, + } + } +} + +fn main() { + let application = Application::new( + Some("com.github.gtk-rs.examples.basic"), + Default::default(), + ).expect("failed to initialize GTK application"); + + let state = Rc::new(RefCell::new(State::new())); + + application.connect_activate(clone!(state => move |app| { + state.borrow_mut().started = true; + + let window = ApplicationWindow::new(app); + window.set_title("First GTK+ Program"); + window.set_default_size(350, 70); + + let button = Button::new_with_label("Click me!"); + button.connect_clicked(clone!(state => move |_| { + let mut state = state.borrow_mut(); + println!("Clicked (started: {}): {}!", state.started, state.count); + state.count += 1; + })); + window.add(&button); + + window.show_all(); + })); + + application.run(&[]); +} diff --git a/src/clone.rs b/src/clone.rs new file mode 100644 index 00000000..bca4a850 --- /dev/null +++ b/src/clone.rs @@ -0,0 +1,62 @@ +use std::rc::{self, Rc}; +use std::sync::{self, Arc}; + +pub trait Downgrade { + type Target; + + fn downgrade(&self) -> Self::Target; +} + +impl Downgrade for Arc { + type Target = sync::Weak; + + fn downgrade(&self) -> Self::Target { + Arc::downgrade(self) + } +} + +impl Downgrade for Rc { + type Target = rc::Weak; + + fn downgrade(&self) -> Self::Target { + Rc::downgrade(self) + } +} + +pub trait Upgrade { + type Target; + + fn upgrade(&self) -> Option; +} + +impl Upgrade for sync::Weak { + type Target = Arc; + + fn upgrade(&self) -> Option { + self.upgrade() + } +} + +impl Upgrade for rc::Weak { + type Target = Rc; + + fn upgrade(&self) -> Option { + self.upgrade() + } +} + +#[macro_export] +macro_rules! clone { + ($($n:ident),+ => move |$($p:pat),*| $body:expr) => ( + { + $( let $n = $crate::clone::Downgrade::downgrade(&$n); )+ + move |$($p,)*| { + $(let $n = match $crate::clone::Upgrade::upgrade(&$n) { + Some(val) => val, + None => panic!("cannot upgrade weak reference `{}`", stringify!($n)), + };)+ + $body + } + } + ); +} diff --git a/src/lib.rs b/src/lib.rs index 6d37efa6..eb4feacf 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -117,6 +117,8 @@ pub use value::{SendValue, ToSendValue, ToValue, TypedValue, Value}; pub use variant::{StaticVariantType, ToVariant, Variant}; pub use variant_type::{VariantTy, VariantType}; +#[macro_use] +pub mod clone; #[macro_use] pub mod wrapper; #[macro_use] diff --git a/tests/clone_panic.rs b/tests/clone_panic.rs new file mode 100644 index 00000000..92cc7173 --- /dev/null +++ b/tests/clone_panic.rs @@ -0,0 +1,65 @@ +extern crate gio; +extern crate glib; +extern crate gtk; + +use std::cell::RefCell; +use std::rc::Rc; + +use gio::{ApplicationExt, ApplicationExtManual}; +use gtk::{ + Application, + ApplicationWindow, + Button, + ButtonExt, + ContainerExt, + GtkWindowExt, + WidgetExt, +}; +use glib::clone; + +struct State { + started: bool, + count: i32, +} + +impl State { + fn new() -> Self { + Self { + started: false, + count: 0, + } + } +} + +#[test] +#[should_panic] +fn clone_panic() { + let application = Application::new( + Some("com.github.gtk-rs.examples.basic"), + Default::default(), + ).expect("failed to initialize GTK application"); + + { + let state = Rc::new(RefCell::new(State::new())); + + application.connect_activate(clone!(state => move |app| { + state.borrow_mut().started = true; + + let window = ApplicationWindow::new(app); + window.set_title("First GTK+ Program"); + window.set_default_size(350, 70); + + let button = Button::new_with_label("Click me!"); + button.connect_clicked(clone!(state => move |_| { + let mut state = state.borrow_mut(); + println!("Clicked (started: {}): {}!", state.started, state.count); + state.count += 1; + })); + window.add(&button); + + window.show_all(); + })); + } + + application.run(&[]); +} From f1f9aaa6d815a7d31258c64c84831bd6b23bc962 Mon Sep 17 00:00:00 2001 From: Antoni Boucher Date: Tue, 22 Oct 2019 08:45:16 -0400 Subject: [PATCH 02/15] Allow specifying whether it is a weak or strong reference --- examples/clone.rs | 2 +- src/clone.rs | 34 +++++++++++++++++++++++++++------- tests/clone_panic.rs | 2 +- 3 files changed, 29 insertions(+), 9 deletions(-) diff --git a/examples/clone.rs b/examples/clone.rs index 1ff95e0d..8eceb9b4 100644 --- a/examples/clone.rs +++ b/examples/clone.rs @@ -39,7 +39,7 @@ fn main() { let state = Rc::new(RefCell::new(State::new())); - application.connect_activate(clone!(state => move |app| { + application.connect_activate(clone!(@weak state => move |app| { state.borrow_mut().started = true; let window = ApplicationWindow::new(app); diff --git a/src/clone.rs b/src/clone.rs index bca4a850..65c2b1bb 100644 --- a/src/clone.rs +++ b/src/clone.rs @@ -45,16 +45,36 @@ impl Upgrade for rc::Weak { } } +#[macro_export] +macro_rules! to_type_before { + (_) => (); + ($variable:ident) => ( + let $variable = $variable.clone(); + ); + (@weak $variable:ident) => ( + let $variable = $crate::clone::Downgrade::downgrade(&$variable); + ); +} + +#[macro_export] +macro_rules! to_type_after { + (_) => (); + ($variable:ident) => (); + (@weak $variable:ident) => ( + let $variable = match $crate::clone::Upgrade::upgrade(&$variable) { + Some(val) => val, + None => panic!("cannot upgrade weak reference `{}`", stringify!($n)), + }; + ); +} + #[macro_export] macro_rules! clone { - ($($n:ident),+ => move |$($p:pat),*| $body:expr) => ( + ($($(@ $weak:ident)? $variables:ident),+ => move | $($pattern:pat),* | $body:block )=> ( { - $( let $n = $crate::clone::Downgrade::downgrade(&$n); )+ - move |$($p,)*| { - $(let $n = match $crate::clone::Upgrade::upgrade(&$n) { - Some(val) => val, - None => panic!("cannot upgrade weak reference `{}`", stringify!($n)), - };)+ + $( $crate::to_type_before!($(@ $weak)? $variables) )*; + move |$($pattern)*| { + $( $crate::to_type_after!($(@ $weak)? $variables ))*; $body } } diff --git a/tests/clone_panic.rs b/tests/clone_panic.rs index 92cc7173..20e22521 100644 --- a/tests/clone_panic.rs +++ b/tests/clone_panic.rs @@ -42,7 +42,7 @@ fn clone_panic() { { let state = Rc::new(RefCell::new(State::new())); - application.connect_activate(clone!(state => move |app| { + application.connect_activate(clone!(@weak state => move |app| { state.borrow_mut().started = true; let window = ApplicationWindow::new(app); From b43145aff7939d5641dc8d8e562fc0e39ac9d3eb Mon Sep 17 00:00:00 2001 From: Antoni Boucher Date: Tue, 22 Oct 2019 09:54:40 -0400 Subject: [PATCH 03/15] Fix clone macro --- src/clone.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/clone.rs b/src/clone.rs index 65c2b1bb..5862fac8 100644 --- a/src/clone.rs +++ b/src/clone.rs @@ -72,9 +72,9 @@ macro_rules! to_type_after { macro_rules! clone { ($($(@ $weak:ident)? $variables:ident),+ => move | $($pattern:pat),* | $body:block )=> ( { - $( $crate::to_type_before!($(@ $weak)? $variables) )*; + $( $crate::to_type_before!($(@ $weak)? $variables); )* move |$($pattern)*| { - $( $crate::to_type_after!($(@ $weak)? $variables ))*; + $( $crate::to_type_after!($(@ $weak)? $variables );)* $body } } From c4bbb0af3ddfe1d09a23bbc1d01cf3eb01020257 Mon Sep 17 00:00:00 2001 From: Antoni Boucher Date: Tue, 22 Oct 2019 10:13:13 -0400 Subject: [PATCH 04/15] Fix clone macro --- Cargo.toml | 2 -- examples/clone.rs | 61 -------------------------------------------- src/clone.rs | 11 +++++++- tests/clone_panic.rs | 60 +++++++++++++++++-------------------------- 4 files changed, 34 insertions(+), 100 deletions(-) delete mode 100644 examples/clone.rs diff --git a/Cargo.toml b/Cargo.toml index 5d26137a..a0db5345 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -33,8 +33,6 @@ glib-sys = { git = "https://github.com/gtk-rs/sys" } gobject-sys = { git = "https://github.com/gtk-rs/sys" } [dev-dependencies] -gio = { git = "https://github.com/gtk-rs/gio" } -gtk = { git = "https://github.com/gtk-rs/gtk" } tempfile = "3" [features] diff --git a/examples/clone.rs b/examples/clone.rs deleted file mode 100644 index 8eceb9b4..00000000 --- a/examples/clone.rs +++ /dev/null @@ -1,61 +0,0 @@ -extern crate gio; -extern crate glib; -extern crate gtk; - -use std::cell::RefCell; -use std::rc::Rc; - -use gio::{ApplicationExt, ApplicationExtManual}; -use gtk::{ - Application, - ApplicationWindow, - Button, - ButtonExt, - ContainerExt, - GtkWindowExt, - WidgetExt, -}; -use glib::clone; - -struct State { - started: bool, - count: i32, -} - -impl State { - fn new() -> Self { - Self { - started: false, - count: 0, - } - } -} - -fn main() { - let application = Application::new( - Some("com.github.gtk-rs.examples.basic"), - Default::default(), - ).expect("failed to initialize GTK application"); - - let state = Rc::new(RefCell::new(State::new())); - - application.connect_activate(clone!(@weak state => move |app| { - state.borrow_mut().started = true; - - let window = ApplicationWindow::new(app); - window.set_title("First GTK+ Program"); - window.set_default_size(350, 70); - - let button = Button::new_with_label("Click me!"); - button.connect_clicked(clone!(state => move |_| { - let mut state = state.borrow_mut(); - println!("Clicked (started: {}): {}!", state.started, state.count); - state.count += 1; - })); - window.add(&button); - - window.show_all(); - })); - - application.run(&[]); -} diff --git a/src/clone.rs b/src/clone.rs index 5862fac8..10dbf629 100644 --- a/src/clone.rs +++ b/src/clone.rs @@ -70,10 +70,19 @@ macro_rules! to_type_after { #[macro_export] macro_rules! clone { + ($($(@ $weak:ident)? $variables:ident),+ => move || $body:block )=> ( + { + $( $crate::to_type_before!($(@ $weak)? $variables); )* + move || { + $( $crate::to_type_after!($(@ $weak)? $variables );)* + $body + } + } + ); ($($(@ $weak:ident)? $variables:ident),+ => move | $($pattern:pat),* | $body:block )=> ( { $( $crate::to_type_before!($(@ $weak)? $variables); )* - move |$($pattern)*| { + move |$($pattern),*| { $( $crate::to_type_after!($(@ $weak)? $variables );)* $body } diff --git a/tests/clone_panic.rs b/tests/clone_panic.rs index 20e22521..1d3a1f85 100644 --- a/tests/clone_panic.rs +++ b/tests/clone_panic.rs @@ -1,20 +1,8 @@ -extern crate gio; extern crate glib; -extern crate gtk; use std::cell::RefCell; use std::rc::Rc; -use gio::{ApplicationExt, ApplicationExtManual}; -use gtk::{ - Application, - ApplicationWindow, - Button, - ButtonExt, - ContainerExt, - GtkWindowExt, - WidgetExt, -}; use glib::clone; struct State { @@ -32,34 +20,34 @@ impl State { } #[test] -#[should_panic] -fn clone_panic() { - let application = Application::new( - Some("com.github.gtk-rs.examples.basic"), - Default::default(), - ).expect("failed to initialize GTK application"); - - { +fn clone_closure() { let state = Rc::new(RefCell::new(State::new())); + assert_eq!(state.borrow().started, false); - application.connect_activate(clone!(@weak state => move |app| { - state.borrow_mut().started = true; + let closure = { - let window = ApplicationWindow::new(app); - window.set_title("First GTK+ Program"); - window.set_default_size(350, 70); + clone!(@weak state => move || { + state.borrow_mut().started = true; - let button = Button::new_with_label("Click me!"); - button.connect_clicked(clone!(state => move |_| { - let mut state = state.borrow_mut(); - println!("Clicked (started: {}): {}!", state.started, state.count); - state.count += 1; - })); - window.add(&button); + }) + }; - window.show_all(); - })); - } + closure(); + + assert_eq!(state.borrow().started, true); +} + +#[test] +#[should_panic] +fn clone_panic() { + let closure = { + let state = Rc::new(RefCell::new(State::new())); + + clone!(@weak state => move |_| { + state.borrow_mut().started = true; + + }) + }; - application.run(&[]); + closure(10); } From 2a6026564442d927ff387b909ecf642560b8153c Mon Sep 17 00:00:00 2001 From: Antoni Boucher Date: Tue, 22 Oct 2019 11:13:05 -0400 Subject: [PATCH 05/15] Default value instead of panic --- src/clone.rs | 24 ++++++++++++++++-------- tests/{clone_panic.rs => clone.rs} | 29 ++++++++++++----------------- 2 files changed, 28 insertions(+), 25 deletions(-) rename tests/{clone_panic.rs => clone.rs} (62%) diff --git a/src/clone.rs b/src/clone.rs index 10dbf629..cb51e391 100644 --- a/src/clone.rs +++ b/src/clone.rs @@ -58,32 +58,40 @@ macro_rules! to_type_before { #[macro_export] macro_rules! to_type_after { - (_) => (); - ($variable:ident) => (); - (@weak $variable:ident) => ( + (_ , $return_value:expr) => (); + ($variable:ident , $return_value:expr) => (); + (@weak $variable:ident , $return_value:expr) => ( let $variable = match $crate::clone::Upgrade::upgrade(&$variable) { Some(val) => val, - None => panic!("cannot upgrade weak reference `{}`", stringify!($n)), + None => return $return_value, }; ); } +#[macro_export] +macro_rules! to_return_value { + () => (()); + ($value:expr) => ( $value ); +} + #[macro_export] macro_rules! clone { - ($($(@ $weak:ident)? $variables:ident),+ => move || $body:block )=> ( + ($($(@ $weak:ident)? $variables:ident),+ => move || $body:block $(, $return_value:expr)? ) => ( { $( $crate::to_type_before!($(@ $weak)? $variables); )* + let return_value = $crate::to_return_value!($($return_value)?); move || { - $( $crate::to_type_after!($(@ $weak)? $variables );)* + $( $crate::to_type_after!($(@ $weak)? $variables, return_value );)* $body } } ); - ($($(@ $weak:ident)? $variables:ident),+ => move | $($pattern:pat),* | $body:block )=> ( + ($($(@ $weak:ident)? $variables:ident),+ => move | $($pattern:pat),* | $body:block $(, $return_value:expr)? ) => ( { $( $crate::to_type_before!($(@ $weak)? $variables); )* + let return_value = $crate::to_return_value!($($return_value)?); move |$($pattern),*| { - $( $crate::to_type_after!($(@ $weak)? $variables );)* + $( $crate::to_type_after!($(@ $weak)? $variables, return_value );)* $body } } diff --git a/tests/clone_panic.rs b/tests/clone.rs similarity index 62% rename from tests/clone_panic.rs rename to tests/clone.rs index 1d3a1f85..bb132323 100644 --- a/tests/clone_panic.rs +++ b/tests/clone.rs @@ -7,14 +7,12 @@ use glib::clone; struct State { started: bool, - count: i32, } impl State { fn new() -> Self { Self { started: false, - count: 0, } } } @@ -25,29 +23,26 @@ fn clone_closure() { assert_eq!(state.borrow().started, false); let closure = { - clone!(@weak state => move || { state.borrow_mut().started = true; - }) }; - closure(); + assert_eq!(closure(), ()); assert_eq!(state.borrow().started, true); } #[test] -#[should_panic] -fn clone_panic() { - let closure = { - let state = Rc::new(RefCell::new(State::new())); - - clone!(@weak state => move |_| { - state.borrow_mut().started = true; - - }) - }; - - closure(10); +fn clone_default_value() { + let closure = + { + let state = Rc::new(RefCell::new(State::new())); + clone!(@weak state => move |_| { + state.borrow_mut().started = true; + 10 + }, 42) + }; + + assert_eq!(42, closure(50)); } From 2f41c31ce02a91cbcef57292b700ae1fbe4f8f8d Mon Sep 17 00:00:00 2001 From: Antoni Boucher Date: Tue, 22 Oct 2019 11:18:47 -0400 Subject: [PATCH 06/15] Add tests --- tests/clone.rs | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/tests/clone.rs b/tests/clone.rs index bb132323..64c72332 100644 --- a/tests/clone.rs +++ b/tests/clone.rs @@ -6,12 +6,14 @@ use std::rc::Rc; use glib::clone; struct State { + count: i32, started: bool, } impl State { fn new() -> Self { Self { + count: 0, started: false, } } @@ -31,6 +33,23 @@ fn clone_closure() { assert_eq!(closure(), ()); assert_eq!(state.borrow().started, true); + assert_eq!(state.borrow().count, 0); + + let closure = { + let state2 = Rc::new(RefCell::new(State::new())); + assert_eq!(state.borrow().started, true); + + clone!(@weak state, state2 => move || { + state.borrow_mut().count += 1; + state.borrow_mut().started = true; + state2.borrow_mut().started = true; + }) + }; + + assert_eq!(closure(), ()); + + assert_eq!(state.borrow().count, 1); + assert_eq!(state.borrow().started, true); } #[test] From cea7448777209b79ccedd3f750e7cd4eb39dd754 Mon Sep 17 00:00:00 2001 From: Antoni Boucher Date: Tue, 22 Oct 2019 11:51:51 -0400 Subject: [PATCH 07/15] Move default return value in macro --- src/clone.rs | 4 ++-- tests/clone.rs | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/clone.rs b/src/clone.rs index cb51e391..fdf89a59 100644 --- a/src/clone.rs +++ b/src/clone.rs @@ -76,7 +76,7 @@ macro_rules! to_return_value { #[macro_export] macro_rules! clone { - ($($(@ $weak:ident)? $variables:ident),+ => move || $body:block $(, $return_value:expr)? ) => ( + ($($(@ $weak:ident)? $variables:ident),+ => $(@default-return $return_value:expr,)? move || $body:block ) => ( { $( $crate::to_type_before!($(@ $weak)? $variables); )* let return_value = $crate::to_return_value!($($return_value)?); @@ -86,7 +86,7 @@ macro_rules! clone { } } ); - ($($(@ $weak:ident)? $variables:ident),+ => move | $($pattern:pat),* | $body:block $(, $return_value:expr)? ) => ( + ($($(@ $weak:ident)? $variables:ident),+ => $(@default-return $return_value:expr ,)? move | $($pattern:pat),* | $body:block ) => ( { $( $crate::to_type_before!($(@ $weak)? $variables); )* let return_value = $crate::to_return_value!($($return_value)?); diff --git a/tests/clone.rs b/tests/clone.rs index 64c72332..8fcb88f9 100644 --- a/tests/clone.rs +++ b/tests/clone.rs @@ -57,10 +57,10 @@ fn clone_default_value() { let closure = { let state = Rc::new(RefCell::new(State::new())); - clone!(@weak state => move |_| { + clone!(@weak state => @default-return 42, move |_| { state.borrow_mut().started = true; 10 - }, 42) + }) }; assert_eq!(42, closure(50)); From 89b3c6d27707caae02c8e1cff4ab46c7b3dc70d8 Mon Sep 17 00:00:00 2001 From: Antoni Boucher Date: Wed, 23 Oct 2019 05:55:03 -0400 Subject: [PATCH 08/15] Add Downgrade and Upgrade implementation for GObjects --- src/object.rs | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/src/object.rs b/src/object.rs index 33a59513..de4c16e5 100644 --- a/src/object.rs +++ b/src/object.rs @@ -740,6 +740,24 @@ macro_rules! glib_object_wrapper { $crate::gobject_sys::g_value_set_object($crate::translate::ToGlibPtrMut::to_glib_none_mut(value).0, $crate::translate::ToGlibPtr::<*mut $ffi_name>::to_glib_none(&this).0 as *mut $crate::gobject_sys::GObject) } } + + #[doc(hidden)] + impl $crate::clone::Downgrade for $name { + type Target = $crate::object::WeakRef; + + fn downgrade(&self) -> Self::Target { + ::downgrade(&self) + } + } + + #[doc(hidden)] + impl $crate::clone::Upgrade for $crate::object::WeakRef<$name> { + type Target = $name; + + fn upgrade(&self) -> Option { + self.upgrade() + } + } }; (@munch_impls $name:ident, ) => { }; From ec7a8b33d579dc561119a9f454bc614182768130 Mon Sep 17 00:00:00 2001 From: Antoni Boucher Date: Wed, 23 Oct 2019 06:42:37 -0400 Subject: [PATCH 09/15] Fix panic in clone macro --- src/clone.rs | 4 ++-- tests/clone.rs | 29 +++++++++++++++++++++++++++++ 2 files changed, 31 insertions(+), 2 deletions(-) diff --git a/src/clone.rs b/src/clone.rs index fdf89a59..abdf364c 100644 --- a/src/clone.rs +++ b/src/clone.rs @@ -79,8 +79,8 @@ macro_rules! clone { ($($(@ $weak:ident)? $variables:ident),+ => $(@default-return $return_value:expr,)? move || $body:block ) => ( { $( $crate::to_type_before!($(@ $weak)? $variables); )* - let return_value = $crate::to_return_value!($($return_value)?); move || { + let return_value = $crate::to_return_value!($($return_value)?); $( $crate::to_type_after!($(@ $weak)? $variables, return_value );)* $body } @@ -89,8 +89,8 @@ macro_rules! clone { ($($(@ $weak:ident)? $variables:ident),+ => $(@default-return $return_value:expr ,)? move | $($pattern:pat),* | $body:block ) => ( { $( $crate::to_type_before!($(@ $weak)? $variables); )* - let return_value = $crate::to_return_value!($($return_value)?); move |$($pattern),*| { + let return_value = $crate::to_return_value!($($return_value)?); $( $crate::to_type_after!($(@ $weak)? $variables, return_value );)* $body } diff --git a/tests/clone.rs b/tests/clone.rs index 8fcb88f9..d7c311a0 100644 --- a/tests/clone.rs +++ b/tests/clone.rs @@ -1,7 +1,9 @@ extern crate glib; use std::cell::RefCell; +use std::panic; use std::rc::Rc; +use std::sync::{Arc, Mutex}; use glib::clone; @@ -65,3 +67,30 @@ fn clone_default_value() { assert_eq!(42, closure(50)); } + +#[allow(unreachable_code)] +#[test] +fn clone_panic() { + let state = Arc::new(Mutex::new(State::new())); + state.lock().unwrap().count = 20; + + let closure = + { + let state2 = Arc::new(Mutex::new(State::new())); + clone!(@weak state2, state => @default-return panic!(), move |_| { + state.lock().unwrap().count = 21; + state2.lock().unwrap().started = true; + 10 + }) + }; + + let result = panic::catch_unwind(|| { + closure(50); + }); + + if result.is_ok() { + assert!(false, "should panic"); + } + + assert_eq!(state.lock().unwrap().count, 20); +} From e3e1e7c388424e1992c6d3abe6e80f844d9655e7 Mon Sep 17 00:00:00 2001 From: Antoni Boucher Date: Wed, 23 Oct 2019 06:46:51 -0400 Subject: [PATCH 10/15] Fix panic in clone macro --- src/clone.rs | 6 +++--- tests/clone.rs | 1 - 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/src/clone.rs b/src/clone.rs index abdf364c..0bab2144 100644 --- a/src/clone.rs +++ b/src/clone.rs @@ -63,7 +63,7 @@ macro_rules! to_type_after { (@weak $variable:ident , $return_value:expr) => ( let $variable = match $crate::clone::Upgrade::upgrade(&$variable) { Some(val) => val, - None => return $return_value, + None => return ($return_value)(), }; ); } @@ -80,7 +80,7 @@ macro_rules! clone { { $( $crate::to_type_before!($(@ $weak)? $variables); )* move || { - let return_value = $crate::to_return_value!($($return_value)?); + let return_value = || $crate::to_return_value!($($return_value)?); $( $crate::to_type_after!($(@ $weak)? $variables, return_value );)* $body } @@ -90,7 +90,7 @@ macro_rules! clone { { $( $crate::to_type_before!($(@ $weak)? $variables); )* move |$($pattern),*| { - let return_value = $crate::to_return_value!($($return_value)?); + let return_value = || $crate::to_return_value!($($return_value)?); $( $crate::to_type_after!($(@ $weak)? $variables, return_value );)* $body } diff --git a/tests/clone.rs b/tests/clone.rs index d7c311a0..7ee6563b 100644 --- a/tests/clone.rs +++ b/tests/clone.rs @@ -68,7 +68,6 @@ fn clone_default_value() { assert_eq!(42, closure(50)); } -#[allow(unreachable_code)] #[test] fn clone_panic() { let state = Arc::new(Mutex::new(State::new())); From 0e18c8bc3725d3c0646d94dd995b4c9a9d627415 Mon Sep 17 00:00:00 2001 From: Antoni Boucher Date: Wed, 23 Oct 2019 09:02:58 -0400 Subject: [PATCH 11/15] Try to fix impl for GObjects --- src/object.rs | 42 +++++++++++++++++++++++++----------------- 1 file changed, 25 insertions(+), 17 deletions(-) diff --git a/src/object.rs b/src/object.rs index de4c16e5..fa00acfd 100644 --- a/src/object.rs +++ b/src/object.rs @@ -386,6 +386,30 @@ glib_wrapper! { } } +#[macro_export] +macro_rules! glib_weak_impl { + ($name:ident, $crate::wrapper::Void) => { + #[doc(hidden)] + impl $crate::clone::Downgrade for $name { + type Target = $crate::object::WeakRef; + + fn downgrade(&self) -> Self::Target { + ::downgrade(&self) + } + } + + #[doc(hidden)] + impl $crate::clone::Upgrade for $crate::object::WeakRef<$name> { + type Target = $name; + + fn upgrade(&self) -> Option { + self.upgrade() + } + } + }; + ($name:ident, $ty:ty) => (); +} + /// ObjectType implementations for Object types. See `glib_wrapper!`. #[macro_export] macro_rules! glib_object_wrapper { @@ -741,23 +765,7 @@ macro_rules! glib_object_wrapper { } } - #[doc(hidden)] - impl $crate::clone::Downgrade for $name { - type Target = $crate::object::WeakRef; - - fn downgrade(&self) -> Self::Target { - ::downgrade(&self) - } - } - - #[doc(hidden)] - impl $crate::clone::Upgrade for $crate::object::WeakRef<$name> { - type Target = $name; - - fn upgrade(&self) -> Option { - self.upgrade() - } - } + $crate::glib_weak_impl!($name, $ffi_class_name); }; (@munch_impls $name:ident, ) => { }; From 90eb5ec781819b072804176980a6409e2b085222 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Dr=C3=B6ge?= Date: Tue, 12 Nov 2019 11:52:56 +0100 Subject: [PATCH 12/15] Address various review comments for the clone! macro --- src/clone.rs | 11 +++++++++++ src/object.rs | 6 +++--- 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/src/clone.rs b/src/clone.rs index 0bab2144..5b8f4e16 100644 --- a/src/clone.rs +++ b/src/clone.rs @@ -45,9 +45,16 @@ impl Upgrade for rc::Weak { } } +#[doc(hidden)] #[macro_export] macro_rules! to_type_before { (_) => (); + (self) => ( + compile_error!("Can't use `self` as variable name for the `clone!` macro. Try storing it in a temporary variable."); + ); + (@weak self) => ( + compile_error!("Can't use `self` as variable name for the `clone!` macro. Try storing it in a temporary variable."); + ); ($variable:ident) => ( let $variable = $variable.clone(); ); @@ -56,9 +63,12 @@ macro_rules! to_type_before { ); } +#[doc(hidden)] #[macro_export] macro_rules! to_type_after { (_ , $return_value:expr) => (); + (self, $return_value:expr) => (); + (@weak self, $return_value:expr) => (); ($variable:ident , $return_value:expr) => (); (@weak $variable:ident , $return_value:expr) => ( let $variable = match $crate::clone::Upgrade::upgrade(&$variable) { @@ -68,6 +78,7 @@ macro_rules! to_type_after { ); } +#[doc(hidden)] #[macro_export] macro_rules! to_return_value { () => (()); diff --git a/src/object.rs b/src/object.rs index fa00acfd..29ed5801 100644 --- a/src/object.rs +++ b/src/object.rs @@ -386,9 +386,10 @@ glib_wrapper! { } } +#[doc(hidden)] #[macro_export] macro_rules! glib_weak_impl { - ($name:ident, $crate::wrapper::Void) => { + ($name:ident) => { #[doc(hidden)] impl $crate::clone::Downgrade for $name { type Target = $crate::object::WeakRef; @@ -407,7 +408,6 @@ macro_rules! glib_weak_impl { } } }; - ($name:ident, $ty:ty) => (); } /// ObjectType implementations for Object types. See `glib_wrapper!`. @@ -765,7 +765,7 @@ macro_rules! glib_object_wrapper { } } - $crate::glib_weak_impl!($name, $ffi_class_name); + $crate::glib_weak_impl!($name); }; (@munch_impls $name:ident, ) => { }; From 9bcacc52832c510b0e9224acd0441bca05080966 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Dr=C3=B6ge?= Date: Tue, 12 Nov 2019 12:11:33 +0100 Subject: [PATCH 13/15] Add clone! documentation, tighten the trait bounds of the clone macros and run rustfmt --- src/clone.rs | 95 +++++++++++++++++++++++++++++++++++++++++++------- tests/clone.rs | 32 ++++++++--------- 2 files changed, 97 insertions(+), 30 deletions(-) diff --git a/src/clone.rs b/src/clone.rs index 5b8f4e16..3ec8efd7 100644 --- a/src/clone.rs +++ b/src/clone.rs @@ -1,9 +1,15 @@ use std::rc::{self, Rc}; use std::sync::{self, Arc}; -pub trait Downgrade { - type Target; - +/// Trait for generalizing downgrading from a strong reference to a weak reference +pub trait Downgrade +where + Self: Sized, +{ + /// Weak reference type + type Target: Upgrade; + + /// Downgrade to a weak reference fn downgrade(&self) -> Self::Target; } @@ -23,9 +29,15 @@ impl Downgrade for Rc { } } -pub trait Upgrade { - type Target; +/// Trait for generalizing upgrading of weak references to a strong reference +pub trait Upgrade +where + Self: Sized, +{ + /// Strong reference type + type Target: Downgrade; + /// Try upgrading to a strong reference fn upgrade(&self) -> Option; } @@ -66,25 +78,82 @@ macro_rules! to_type_before { #[doc(hidden)] #[macro_export] macro_rules! to_type_after { - (_ , $return_value:expr) => (); - (self, $return_value:expr) => (); - (@weak self, $return_value:expr) => (); - ($variable:ident , $return_value:expr) => (); - (@weak $variable:ident , $return_value:expr) => ( + (_ , $return_value:expr) => {}; + (self, $return_value:expr) => {}; + (@weak self, $return_value:expr) => {}; + ($variable:ident , $return_value:expr) => {}; + (@weak $variable:ident , $return_value:expr) => { let $variable = match $crate::clone::Upgrade::upgrade(&$variable) { Some(val) => val, None => return ($return_value)(), }; - ); + }; } #[doc(hidden)] #[macro_export] macro_rules! to_return_value { - () => (()); - ($value:expr) => ( $value ); + () => { + () + }; + ($value:expr) => { + $value + }; } +/// Macro for passing variables as strong or weak references into a closure +/// +/// This macro can be useful in combination with closures, e.g. signal handlers, to reduce the +/// boilerplate required for passing strong or weak references into the closure. It will +/// automatically create the new reference and pass it with the same name into the closure. +/// +/// If upgrading the weak reference to a strong reference inside the closure is failing, the +/// closure is immediately returning an optional default return value. If none is provided, `()` is +/// returned. +/// +/// ### Passing a strong reference +/// ```rust +/// use glib::clone; +/// use std::rc::Rc; +/// +/// let v = Rc::new(1); +/// let closure = clone!(v => move |x| { +/// println!("v: {}, x: {}", v, x); +/// }); +/// +/// closure(2); +/// ``` +/// +/// ### Passing a strong and weak reference +/// ```rust +/// use glib::clone; +/// use std::rc::Rc; +/// +/// let v = Rc::new(1); +/// let u = Rc::new(2); +/// let closure = clone!(v, @weak u => move |x| { +/// println!("v: {}, u: {}, x: {}", v, u, x); +/// }); +/// +/// closure(3); +/// ``` +/// +/// ### Providing a default return value if upgrading a weak reference fails +/// ```rust +/// use glib::clone; +/// use std::rc::Rc; +/// +/// let v = Rc::new(1); +/// let closure = clone!(@weak v => @default-return false, move |x| { +/// println!("v: {}, x: {}", v, x); +/// true +/// }); +/// +/// // Drop value so that the weak reference can't be upgraded +/// drop(v); +/// +/// assert_eq!(closure(2), false); +/// ``` #[macro_export] macro_rules! clone { ($($(@ $weak:ident)? $variables:ident),+ => $(@default-return $return_value:expr,)? move || $body:block ) => ( diff --git a/tests/clone.rs b/tests/clone.rs index 7ee6563b..392b8f53 100644 --- a/tests/clone.rs +++ b/tests/clone.rs @@ -56,14 +56,13 @@ fn clone_closure() { #[test] fn clone_default_value() { - let closure = - { - let state = Rc::new(RefCell::new(State::new())); - clone!(@weak state => @default-return 42, move |_| { - state.borrow_mut().started = true; - 10 - }) - }; + let closure = { + let state = Rc::new(RefCell::new(State::new())); + clone!(@weak state => @default-return 42, move |_| { + state.borrow_mut().started = true; + 10 + }) + }; assert_eq!(42, closure(50)); } @@ -73,15 +72,14 @@ fn clone_panic() { let state = Arc::new(Mutex::new(State::new())); state.lock().unwrap().count = 20; - let closure = - { - let state2 = Arc::new(Mutex::new(State::new())); - clone!(@weak state2, state => @default-return panic!(), move |_| { - state.lock().unwrap().count = 21; - state2.lock().unwrap().started = true; - 10 - }) - }; + let closure = { + let state2 = Arc::new(Mutex::new(State::new())); + clone!(@weak state2, state => @default-return panic!(), move |_| { + state.lock().unwrap().count = 21; + state2.lock().unwrap().started = true; + 10 + }) + }; let result = panic::catch_unwind(|| { closure(50); From d34d1ce564fbdbd19c49355734e7b5d8b685cfa8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Dr=C3=B6ge?= Date: Tue, 12 Nov 2019 13:00:02 +0100 Subject: [PATCH 14/15] Fix trait impl conherence problems around the clone! macro traits --- src/clone.rs | 66 +++++++++++++++++++++++++++++---------------------- src/object.rs | 13 ++-------- 2 files changed, 39 insertions(+), 40 deletions(-) diff --git a/src/clone.rs b/src/clone.rs index 3ec8efd7..a4a2eecb 100644 --- a/src/clone.rs +++ b/src/clone.rs @@ -1,58 +1,66 @@ use std::rc::{self, Rc}; use std::sync::{self, Arc}; -/// Trait for generalizing downgrading from a strong reference to a weak reference +/// Trait for generalizing downgrading a strong reference to a weak reference. pub trait Downgrade where Self: Sized, { - /// Weak reference type - type Target: Upgrade; + /// Weak reference type. + type Weak; - /// Downgrade to a weak reference - fn downgrade(&self) -> Self::Target; + /// Downgrade to a weak reference. + fn downgrade(&self) -> Self::Weak; } -impl Downgrade for Arc { - type Target = sync::Weak; +/// Trait for generalizing upgrading a weak reference to a strong reference. +pub trait Upgrade +where + Self: Sized, +{ + /// Strong reference type. + type Strong; - fn downgrade(&self) -> Self::Target { - Arc::downgrade(self) - } + /// Try upgrading a weak reference to a strong reference. + fn upgrade(&self) -> Option; } -impl Downgrade for Rc { - type Target = rc::Weak; +impl Upgrade for crate::WeakRef { + type Strong = T; - fn downgrade(&self) -> Self::Target { - Rc::downgrade(self) + fn upgrade(&self) -> Option { + self.upgrade() } } -/// Trait for generalizing upgrading of weak references to a strong reference -pub trait Upgrade -where - Self: Sized, -{ - /// Strong reference type - type Target: Downgrade; +impl Downgrade for Arc { + type Weak = sync::Weak; - /// Try upgrading to a strong reference - fn upgrade(&self) -> Option; + fn downgrade(&self) -> Self::Weak { + Arc::downgrade(self) + } } impl Upgrade for sync::Weak { - type Target = Arc; + type Strong = Arc; - fn upgrade(&self) -> Option { + fn upgrade(&self) -> Option { self.upgrade() } } +impl Downgrade for Rc { + type Weak = rc::Weak; + + fn downgrade(&self) -> Self::Weak { + Rc::downgrade(self) + } +} + impl Upgrade for rc::Weak { - type Target = Rc; + type Strong = Rc; - fn upgrade(&self) -> Option { + fn upgrade(&self) -> Option { self.upgrade() } } @@ -101,7 +109,7 @@ macro_rules! to_return_value { }; } -/// Macro for passing variables as strong or weak references into a closure +/// Macro for passing variables as strong or weak references into a closure. /// /// This macro can be useful in combination with closures, e.g. signal handlers, to reduce the /// boilerplate required for passing strong or weak references into the closure. It will @@ -149,7 +157,7 @@ macro_rules! to_return_value { /// true /// }); /// -/// // Drop value so that the weak reference can't be upgraded +/// // Drop value so that the weak reference can't be upgraded. /// drop(v); /// /// assert_eq!(closure(2), false); diff --git a/src/object.rs b/src/object.rs index 29ed5801..ef57924a 100644 --- a/src/object.rs +++ b/src/object.rs @@ -392,21 +392,12 @@ macro_rules! glib_weak_impl { ($name:ident) => { #[doc(hidden)] impl $crate::clone::Downgrade for $name { - type Target = $crate::object::WeakRef; + type Weak = $crate::object::WeakRef; - fn downgrade(&self) -> Self::Target { + fn downgrade(&self) -> Self::Weak { ::downgrade(&self) } } - - #[doc(hidden)] - impl $crate::clone::Upgrade for $crate::object::WeakRef<$name> { - type Target = $name; - - fn upgrade(&self) -> Option { - self.upgrade() - } - } }; } From 7d73b51d27027982f98f8ac7f454673f6e38f49f Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Thu, 14 Nov 2019 13:18:00 +0100 Subject: [PATCH 15/15] Improve doc styling --- src/clone.rs | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/src/clone.rs b/src/clone.rs index a4a2eecb..b45451ce 100644 --- a/src/clone.rs +++ b/src/clone.rs @@ -109,7 +109,7 @@ macro_rules! to_return_value { }; } -/// Macro for passing variables as strong or weak references into a closure. +/// Macro for passing variables as strong or weak references into a closure. /// /// This macro can be useful in combination with closures, e.g. signal handlers, to reduce the /// boilerplate required for passing strong or weak references into the closure. It will @@ -119,8 +119,9 @@ macro_rules! to_return_value { /// closure is immediately returning an optional default return value. If none is provided, `()` is /// returned. /// -/// ### Passing a strong reference -/// ```rust +/// ### Passing a strong reference +/// +/// ``` /// use glib::clone; /// use std::rc::Rc; /// @@ -132,8 +133,9 @@ macro_rules! to_return_value { /// closure(2); /// ``` /// -/// ### Passing a strong and weak reference -/// ```rust +/// ### Passing a strong and weak reference +/// +/// ``` /// use glib::clone; /// use std::rc::Rc; /// @@ -146,8 +148,9 @@ macro_rules! to_return_value { /// closure(3); /// ``` /// -/// ### Providing a default return value if upgrading a weak reference fails -/// ```rust +/// ### Providing a default return value if upgrading a weak reference fails +/// +/// ``` /// use glib::clone; /// use std::rc::Rc; ///