ThreadGuard: Implement ToValue, Fromvalue and Default traits

[alatiera]

This will also allow ThreadGuard to be used with the new property macro.

[sdroege]

CC @ranfdev

[ranfdev]

Actually, unless the type has a "direct" mapping to a ParamSpec, the type shouldn't implement HasParamSpec but the Property trait.

HasParamSpec creates a direct mapping to a ParamSpec and adds some metadata to the type system (eg: the associated ParamSpecXBuilder for the specific type).

Property is made for "wrapper" types, like Cell or Mutex and handles recursive types.
Thanks to the Property trait we can support types like Rc<RefCell<String>> or even (I think)
Rc<Arc<ThreadGuard<String>>>

[sdroege]

If we can go without all the value trait impls on the ThreadGuard that would indeed be preferable. Can Property work with types that don't offer interior mutability though? I guess so because that's how Rc works?

[sdroege]

Yeah that part works but it only allows

            #[property(name = "thread-guard", get, set)]
            thread_guard: ThreadGuard<Mutex<String>>,

It does not allow the actually sensible one

            #[property(name = "thread-guard", get, set)]
            thread_guard: Mutex<ThreadGuard<String>>,

error[E0277]: the trait bound `ThreadGuard<std::string::String>: glib::ToValue` is not satisfied
error[E0277]: the trait bound `ThreadGuard<std::string::String>: FromValue<'_>` is not satisfied

or

            #[property(name = "thread-guard", get, set)]
            thread_guard: Mutex<ThreadGuard<Arc<String>>>,

error[E0277]: the trait bound `ThreadGuard<Arc<std::string::String>>: glib::ToValue` is not satisfied
error[E0277]: the trait bound `ThreadGuard<Arc<std::string::String>>: FromValue<'_>` is not satisfied

or

            #[property(name = "thread-guard", get, set)]
            thread_guard: Mutex<ThreadGuard<Option<std::sync::Arc<String>>>>,

error[E0277]: the trait bound `std::sync::Mutex<ThreadGuard<Option<Arc<std::string::String>>>>: Property` is not satisfied

The last one is actually approximately what @alatiera wants to use: Mutex<Option<ThreadGuard<gdk::Paintable>>>.

I think the same problem also exists in other type nestings with Option and Rc.

[sdroege]

FTR

diff --git a/glib/src/property.rs b/glib/src/property.rs
index 2183b9b4c8..75783c1deb 100644
--- a/glib/src/property.rs
+++ b/glib/src/property.rs
@@ -9,6 +9,7 @@ use std::sync::Arc;
 use std::sync::Mutex;
 use std::sync::RwLock;
 
+use crate::thread_guard::ThreadGuard;
 use crate::HasParamSpec;
 
 // rustdoc-stripper-ignore-next
@@ -50,6 +51,9 @@ impl<T: Property> Property for Rc<T> {
 impl<T: Property> Property for Arc<T> {
     type Value = T::Value;
 }
+impl<T: Property> Property for ThreadGuard<T> {
+    type Value = T::Value;
+}
 
 // rustdoc-stripper-ignore-next
 /// A container type implementing this trait can be read by the default getter generated by the `Props` macro.
@@ -196,6 +200,19 @@ impl<T: PropertySetNested> PropertySetNested for Arc<T> {
     }
 }
 
+impl<T: PropertyGet> PropertyGet for ThreadGuard<T> {
+    type Value = T::Value;
+    fn get<R, F: Fn(&Self::Value) -> R>(&self, f: F) -> R {
+        self.get_ref().get(f)
+    }
+}
+impl<T: PropertySetNested> PropertySetNested for ThreadGuard<T> {
+    type SetNestedValue = T::SetNestedValue;
+    fn set_nested<F: FnOnce(&mut Self::SetNestedValue)>(&self, f: F) {
+        self.get_ref().set_nested(f)
+    }
+}
+
 macro_rules! impl_atomic {
     ($atomic:ty, $valuety:ty) => {
         impl Property for $atomic {

[ranfdev]

That thought doesn't return a ThreadGuard from the getter. It returns the type contained in the ThreadGuard, doesn't it? I actually want that, but that's not what we were trying to do yesterday.

Yesterday we wanted to return the entire ThreadGuard:

let t: ThreadGuard<_> = obj.prop("thread-guard").get().

[sdroege]

Yes, I don't think that's sensible. You can't store a ThreadGuard in a Value, so that wrapping will necessarily go away. Nothing can know about it at the Value level.

I think the properties macro should consider the ThreadGuard just as a storage container.

[sdroege]

I thought we decided that this is not actually useful and correct

[ranfdev]

I don't know, I thought we would still merge what's possible.

This PR still works, even though the use cases it covers are limited.

[ranfdev]

The code works correctly, just it doesn't cover every use case we have thought of.

[alatiera]

I am fine either way, though not sure that would want get prop to not return a threadguard

[sdroege]

Let's close this then in favour of #984