GNOME Bugzilla – Bug 776537
Shall Gio::PollableInputStream derive from InputStream?
Last modified: 2017-07-25 16:53:21 UTC
gio/src/pollableinputstream.hg contains: //TODO: Instead derive from InputStream, when we can break ABI, //because the GPollableInputStream interface requires the GInputStream interface. //LoadableIcon does a similar thing correctly, for instance. // However, GInputStream is an actuall class, deriving from GObject, // but GPollableInputStream is a GInterface, that "requires" GInputStream. gio/src/pollableoutputstream.hg contains a similar comment. Is it really a good idea to derive a Glib::Interface from a Glib::Object? You would get a strange inheritance structure when an object is derived from the interface. In this case when ConverterInputStream is derived from PollableInputStream and FilterInputStream. See the diagram at https://developer.gnome.org/glibmm/unstable/classGio_1_1ConverterInputStream.html If PollableInputStream were derived from InputStream, a ConverterInputStream would contain 2 copies of Glib::Object and 2 copies of Gio::InputStream. Gio::TlsClientConnection, TlsServerConnection and TlsFileDatabase are interfaces which are actually derived from objects, e.g. class TlsClientConnection : public Glib::Interface, public TlsConnection I suppose this is acceptable only because no class in derived from any of these interfaces. If anyone thinks it's worthwhile, I can make some tests to see if it's possible to derive PollableInputStream from InputStream. Otherwise, I'd prefer to just remove the TODO comments in PollableInputStream and PollableOutputStream, and replace them with a comment saying why these interfaces shall not be derived from InputStream and OutputStream.
Created attachment 353993 [details] Test case I made a test case with ConverterOutputStream, which implements PollableOutputStream. First I noticed that Gio::CharsetConverter does not implement Gio::Initable. GCharsetConverter implements GInitable, and that must be reflected in glibmm. Gio::CharsetConverter's constructor must call Gio::Initable::init(). When CharsetConverter had been fixed, the test case worked as expected. Then I modified PollableOutputStream, making it derive from OutputStream. As expected that didn't work. A ConverterOutputStream object then contains two instances of Glib::Object and Gio::OutputStream, one via FilterOutputStream, the other via PollableOutputStream. class FilterOutputStream : public OutputStream class PollableOutputStream : public Glib::Interface, public OutputStream class ConverterOutputStream : public FilterOutputStream, public PollableOutputStream When compiling you'll notice that calls to OutputStream methods become ambiguous. You have to specify which OutputStream you want to call, e.g. converter_stream->Gio::FilterOutputStream::write(data); If that is fixed, and the program compiles, it crashes during execution. I have not bothered to find out why it crashes. It's not important. What's important is never to have more than one instance of Glib::Object in a derived class.
Can't virtual inheritance be of use here?
Do you mean that OutputStream should be a virtual base of PollableOutputStream? I suppose that it then also must be a virtual base of FilterOutputStream and possibly other classes that derive directly from OutputStream. It would mean that the most derived class (e.g. Gio::ConverterOutputStream or a user-supplied MyConverterOutputStream) decides which of OutputStream's constructors is called. I don't think it's a good idea. When I looked in more detail at some glib/gio interfaces and how they are used in glib, I found that several people (including me) have probably misunderstood what is meant by an interface requiring another interface or a class. Some citations from https://developer.gnome.org/gobject/unstable/gobject-Type-Information.html G_TYPE_IS_INTERFACE GLib interfaces are somewhat analogous to Java interfaces and C++ classes containing only pure virtual functions, with the difference that GType interfaces are not derivable. g_type_interface_add_prerequisite() Adds prerequisite_type to the list of prerequisites of interface_type. This means that any type implementing interface_type must also implement prerequisite_type. Prerequisites can be thought of as an alternative to interface derivation (which GType doesn't support). An interface can have at most one instantiatable prerequisite type. My interpretation: "Interface A requires interface B" means "Every class that implements A must also implement B." "Interface A requires class C" means "Only C and subclasses of C can implement A." GLoadableIcon: G_DEFINE_INTERFACE(GLoadableIcon, g_loadable_icon, G_TYPE_ICON) struct _GLoadableIconIface { GTypeInterface g_iface; ... }; GLoadableIcon requires GIcon, but the first member pf _GLoadableIconIface is not struct _GIconIface. GBytesIcon and GFileIcon implement both GIcon and GLoadableIcon. GPollableOutputStream: G_DEFINE_INTERFACE (GPollableOutputStream, g_pollable_output_stream, G_TYPE_OUTPUT_STREAM) struct _GPollableOutputStreamInterface { GTypeInterface g_iface; ... }; GPollableOutputStream requires GOutputStream. It is implemented by GConverterOutputStream, GMemoryOutputStream, GSocketOutputStream, and GUnixOutputStream. All of them are subclasses of GOutputStream. If I'm right, every interface in the mm-modules shall be derived directly from Glib::Interface and nothing else, whatever other interface or class it requires. It's up to every implementing class to meet the requirements.
Long ago someone knew that an interface cannot be derived from another interface. The code generated by _CLASS_INTERFACE contains the comment // We can not derive from another interface, and it is not necessary anyway. I think the interfaces in glibmm/gio shall be changed like this: ActionGroup Base class of DBus::ActionGroup when RemoteActionGroup is not a subclass of ActionGroup Icon Base class of FileIcon when LoadableIcon is not a subclass of Icon LoadableIcon : public Icon Derive directly from Glib::Interface NetworkMonitor : public Initable Derive directly from Glib::Interface PollableInputStream OK, but modify or remove a comment PollableOutputStream OK, but modify or remove a comment RemoteActionGroup : public ActionGroup Derive directly from Glib::Interface Base class of Application TlsClientConnection : public Glib::Interface, public TlsConnection Derive only from Glib::Interface TlsServerConnection : public Glib::Interface, public TlsConnection Derive only from Glib::Interface
TlsFileDatabase : public Glib::Interface, public TlsDatabase Derive only from Glib::Interface tlsfiledatabase.{hg,ccg} exist but tlsfiledatabase.hg is not included in filelist.am. tlsfiledatabase.{h,cc} are not built.
Created attachment 355016 [details] [review] patch: Derive all interfaces directly from Glib::Interface This is what I suggest shall be changed. It's a subset of my suggestions in comment 4. ActionGroup Base class of DBus::ActionGroup when RemoteActionGroup is not a subclass of ActionGroup Icon Base class of FileIcon when LoadableIcon is not a subclass of Icon LoadableIcon : public Icon Derive directly from Glib::Interface NetworkMonitor : public Initable Derive directly from Glib::Interface PollableInputStream Modify a comment PollableOutputStream Modify a comment RemoteActionGroup : public ActionGroup Derive directly from Glib::Interface Differences compared to comment 4: RemoteActionGroup is not a base class of Gio::Application. I was wrong there. GApplication does not implement GRemoteActionGroup. TlsClientConnection, TlsServerConnection and TlsFileDatabase are not changed. These interfaces are unusual. The underlying glib/gio interfaces have _new() functions that return a pointer to an instance of an unspecified class that implements the interface. GTlsClientConnection and GTlsServerConnection require GTlsConnection, but GTlsConnection itself does not implement those interfaces. Only some subclasses of GTlsConnection do, and those subclasses are not wrapped in glibmm. I don't think they are meant to be directly accessed by applications.
I have pushed the patch to the master branch.