After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 621941 - Add Glib::Variant<> classes to wrap glib's GVariant API
Add Glib::Variant<> classes to wrap glib's GVariant API
Status: RESOLVED FIXED
Product: glibmm
Classification: Bindings
Component: general
unspecified
Other Linux
: Normal enhancement
: ---
Assigned To: gtkmm-forge
gtkmm-forge
Depends on:
Blocks:
 
 
Reported: 2010-06-17 22:14 UTC by José Alburquerque
Modified: 2010-06-20 04:15 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
A preliminary implementation of the Glib::Variant<> classes (14.33 KB, patch)
2010-06-17 22:14 UTC, José Alburquerque
none Details | Review

Description José Alburquerque 2010-06-17 22:14:43 UTC
Created attachment 163969 [details] [review]
A preliminary implementation of the Glib::Variant<> classes

The attached patch is a preliminary attempt at wrapping glib's GVariant API.  It's only preliminary because not everything is there yet, but posting it allows comments to be made and the patch to be refined.

My thoughts for the future is to add methods like create_array() to the Glib::Variant<> class specializations which would return a Glib::VariantBase (or Glib::Variant<> specialization) containing the array and methods like create_tuple(), create_maybe(), and create_dict_entry() to the appropriate specializations, also returning a Glib::VariantBase or something more appropriate.  I acknowledge that there may be conceptual, organizational or other errors but at least the patch is available for comment, review and modification.

I'd be happy to modify the patch where appropriate.
Comment 1 Murray Cumming 2010-06-18 22:22:20 UTC
In your patch, VariantBase is implicitly shared (copying just increases the reference). That would be the first implicitly shared class in gtkmm. It will mean that, when doing this:
  VariantBase a;
  a.set_something(1);
  variantBase b = A;
  b.set_something(2);
both a and b will return 2 for get_something().

Qt has implicit sharing for lots of classes, but at least avoids the above problem by doing a deep copy whenever you call a set*() method. But even if we wanted to do that, there's still no way to copy a GVariant.

At the moment I guess that we'll just need to use Variant with Glib::RefPtr, though it won't be pretty.
Comment 2 José Alburquerque 2010-06-18 23:11:34 UTC
(In reply to comment #1)
> In your patch, VariantBase is implicitly shared (copying just increases the
> reference). That would be the first implicitly shared class in gtkmm. It will
> mean that, when doing this:
>   VariantBase a;
>   a.set_something(1);
>   variantBase b = A;
>   b.set_something(2);
> both a and b will return 2 for get_something().

Oh yes.  Interstingly enough, I've not been able to figure out how a variant would be set because the API docs doesn't seem to mention any set methods.  Still as you point out that would not be desirable behavior.

> 
> Qt has implicit sharing for lots of classes, but at least avoids the above
> problem by doing a deep copy whenever you call a set*() method. But even if we
> wanted to do that, there's still no way to copy a GVariant.
> 
> At the moment I guess that we'll just need to use Variant with Glib::RefPtr,
> though it won't be pretty.

I guess it won't.  An updated patch to follow.
Comment 3 Murray Cumming 2010-06-18 23:42:03 UTC
> Interstingly enough, I've not been able to figure out how a variant
> would be set because the API docs doesn't seem to mention any set methods. 
> Still as you point out that would not be desirable behavior.

Actually, I notice that GVariants are meant to always be const. You set the value when you create it, via the g_variant_new_*() method:
http://library.gnome.org/devel/glib/unstable/glib-GVariant.html

So this implicit-sharing might be totally OK. Please commit your patch, and let's try it out.
Comment 4 José Alburquerque 2010-06-19 00:16:29 UTC
Committed.  There are a few other changes that I'd like to add to what's in git later if that's okay.
Comment 5 Murray Cumming 2010-06-19 08:17:09 UTC
By the way, this can probably be done with .hg and .ccg files, so we can use _WRAP_METHOD(). Maybe _CLASS_BOXEDTYPE()
  http://library.gnome.org/devel/gtkmm-tutorial/unstable/sec-wrapping-hg-files.html.en#gmmproc-class-boxedtype
or just _CLASS_GENERIC if that doesn't work
  http://library.gnome.org/devel/gtkmm-tutorial/unstable/sec-wrapping-hg-files.html.en#gmmproc-class-generic
Comment 6 José Alburquerque 2010-06-20 04:15:41 UTC
Okay.  I modified what's in git to use .hg and .ccg files along with the _CLASS_GENERIC macro because it doesn't look like GVariant registers a GType.  The documentation changes I wanted to add to git are now also done so the rest can be added as needed.  I hope there are no errors logically or otherwise.