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 732746 - Interface properties shall be instance data
Interface properties shall be instance data
Status: RESOLVED FIXED
Product: glibmm
Classification: Bindings
Component: object
2.41.x
Other All
: Normal normal
: ---
Assigned To: gtkmm-forge
gtkmm-forge
Depends on:
Blocks:
 
 
Reported: 2014-07-04 17:13 UTC by Kjell Ahlstedt
Modified: 2014-07-21 13:54 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Test case (7.19 KB, text/x-c++src)
2014-07-04 17:13 UTC, Kjell Ahlstedt
  Details
Output from the test case (1.47 KB, text/plain)
2014-07-04 17:20 UTC, Kjell Ahlstedt
  Details
patch: Make custom interface properties instance data (4.20 KB, patch)
2014-07-07 16:59 UTC, Kjell Ahlstedt
none Details | Review
Test case 2, source code and results (2.23 KB, application/x-compressed-tar)
2014-07-09 14:51 UTC, Kjell Ahlstedt
  Details

Description Kjell Ahlstedt 2014-07-04 17:13:37 UTC
Created attachment 279926 [details]
Test case

A patch by José Alburquerque has made it possible to make custom classes that
derive from interfaces with properties, such as
  class CustomAction : public Gio::Action, public Glib::Object

This is discussed in bug 697229 comments 13-16, 18-19, 21-25.
That patch is basically good and welcome, but some of my questions were never
really sorted out. I think some details could and should be improved.

I repeat here some of the questions I asked in bug 697229 comment 21, together
with new comments.

1. Would it be a good idea to add
   g_param_value_set_default(iface_props[p], g_value) in
   Interface::Interface(const Interface_Class& interface_class) and
   Class::custom_class_init_function(void* g_class, void* class_data)?

g_param_value_set_default() was added to the Interface constructor, but not
to Class::custom_class_init_function(). That's easily fixed.

3. The Class::properties_type vector that holds the values of the overridden
   properties of implemented interfaces is attached to the custom_type with
   g_type_set_qdata(). Those values are thus class data. They shall be instance
   data. Each instance of the custom class shall have its own set of property
   values.

The comments in bug 697229 indicate that José does not agree with me.
I claim that everything about a property (name, type, default value, etc.)
except its value is class data. But each instance of the class shall have its
own copy of the property's value.

I attach a test case that shows how a custom implementation of Gio::Action
(with read-only properties) and Gtk::Orientable (with a read-write property)
work.

4. Don't we need protected access methods to the overridden properties?
   Accesses through property_xxx() methods generated by _WRAP_PROPERTY are
   probably enough for read-write props, but what about read-only and write-
   only props? The class that implements the interface must be able to write
   a read-only prop, and read a write-only prop.

Comments in the test case show that this is more complicated than I thought
when I first made this comment. Perhaps a reasonable "solution" is that
a custom class that need read-only or write-only properties from an
implemented interface must provide set-methods for write-only props and
get-methods for read-only props, and private data members to store the values.
Comment 1 Kjell Ahlstedt 2014-07-04 17:20:06 UTC
Created attachment 279927 [details]
Output from the test case

The output shows:

1. The test case fails to set a value to the read-only Gio::Action::
   property_name(). "So what?" you might say. "Of course you shall not be able
   to write to a read-only property." But a read-only property is like a
   private data member with a get-method but no set-sethod. The object that
   owns the property (in this case a CustomAction) shall be able to both read
   and write it.

2. When a value is set to Gtk::Orientable::property_orientation(), the value
   pertains to both CustomAction objects. That's because the value is class
   data instead of instance data. That's not how properties shall behave.

What shall be done? I suggest that items 1 and 3 in comment 0 shall be fixed.
Drop item 4. It's probably no big issue and there is an easy workaround
(mentioned after item 4).

The patches in bug 731484 comment 0 and bug 731484 comment 2 also deserve
mentioning here.
Comment 2 Kjell Ahlstedt 2014-07-07 16:59:00 UTC
Created attachment 280075 [details] [review]
patch: Make custom interface properties instance data

This patch fixes items 1 and 3.
I'll let it rest here for a while before I push it. It not obvious how to best
fix this. Hopefully someone will have a look at it.
Comment 3 Kjell Ahlstedt 2014-07-09 14:51:25 UTC
Created attachment 280278 [details]
Test case 2, source code and results

A tar file with the files
  testinterfaceprops2.cc     Source code
  testinterfaceprops2-1.txt  Results without the patch in comment 2
  testinterfaceprops2-2.txt  Results with the patch

This is a better test case. It tests both how default values are handled,
and if the properties are instance data.
Comment 4 Kjell Ahlstedt 2014-07-21 13:54:31 UTC
I have pushed the patch in comment 2, with minor changes in comments.
https://git.gnome.org/browse/glibmm/commit/?id=08da2fbbb763b81fd6f1e3074aaaf83694c012ed

I wanted to give José a chance to comment, since he has fixed most of the
interface property stuff in custom classes. Then I happened to see
bug 686909 comment 9, and realized that he will not have the time to do so.
Understandable but unfortunate.