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 767951 - Gtk::ApplicationWindow is a _CLASS_GOBJECT
Gtk::ApplicationWindow is a _CLASS_GOBJECT
Status: RESOLVED FIXED
Product: gtkmm
Classification: Bindings
Component: general
3.21.x
Other All
: Normal normal
: ---
Assigned To: gtkmm-forge
gtkmm-forge
Depends on:
Blocks:
 
 
Reported: 2016-06-22 15:09 UTC by Kjell Ahlstedt
Modified: 2016-06-29 17:02 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
patch: class_gtkobject.m4: Add _CUSTOM_WRAP_FUNCTION and _NO_WRAP_FUNCTION (2.59 KB, patch)
2016-06-23 17:11 UTC, Kjell Ahlstedt
committed Details | Review
patch: ApplicationWindow: Define it as a _CLASS_GTKOBJECT (4.03 KB, patch)
2016-06-23 17:12 UTC, Kjell Ahlstedt
committed Details | Review

Description Kjell Ahlstedt 2016-06-22 15:09:55 UTC
applicationwindow.hg contains
 _CLASS_GOBJECT(ApplicationWindow, GtkApplicationWindow, GTK_APPLICATION_WINDOW,
                Gtk::Window, GtkWindow)

This is strange. Is it a mistake? Shouldn't it be _CLASS_GTKOBJECT and
_UNMANAGEABLE?

If it's a mistake, can it be fixed without breaking ABI?
A fix will certainly break API. After a fix, the return type of wrap() will be
different, and there will be no gobj_copy().
Comment 1 Kjell Ahlstedt 2016-06-22 17:26:04 UTC
I've compared the generated files with _CLASS_GOBJECT and with
_CLASS_GTKOBJECT + _UNMANAGEABLE.

There are some trivial differences like 'using' vs. 'typedef' and the order of
some declarations.
There are 3 non-trivial differences:

1. The destructor calls destroy_() only in a _CLASS_GTKOBJECT.

2. gobj_copy() exists only in a _CLASS_GOBJECT.

3. wrap() returns a Gtk::ApplicationWindow* in a _CLASS_GTKOBJECT,
   but a Glib::RefPtr<Gtk::ApplicationWindow> in a _CLASS_GOBJECT.

Difference 1 is not serious. destroy_() will be called anyway, from the
destructor of the base class Window.

Difference 2 will not break ABI/API, if we add a hand-coded and deprecated
gobj_copy().

Difference 3 is the only serious one. We can't deprecate the present erroneous
wrap() function and add a correct overloaded one. Only the return type would
be different. It's possible (probable?) that ABI would be broken if we just
remove the present wrap() and add a wrap() with another return type.
  Glib::RefPtr<Gtk::ApplicationWindow> ==> Gtk::ApplicationWindow*
Comment 2 Murray Cumming 2016-06-22 18:27:40 UTC
(In reply to Kjell Ahlstedt from comment #0)
> applicationwindow.hg contains
>  _CLASS_GOBJECT(ApplicationWindow, GtkApplicationWindow,
> GTK_APPLICATION_WINDOW,
>                 Gtk::Window, GtkWindow)
> 
> This is strange. Is it a mistake? Shouldn't it be _CLASS_GTKOBJECT and
> _UNMANAGEABLE?

Yes. It must just be a mistake.
Comment 3 Murray Cumming 2016-06-22 18:31:06 UTC
(In reply to Kjell Ahlstedt from comment #1)
> Difference 3 is the only serious one. We can't deprecate the present
> erroneous
> wrap() function and add a correct overloaded one. Only the return type would
> be different. It's possible (probable?) that ABI would be broken if we just
> remove the present wrap() and add a wrap() with another return type.
>   Glib::RefPtr<Gtk::ApplicationWindow> ==> Gtk::ApplicationWindow*

Maybe we need to keep the wrong wrap() and document it as broken.
Comment 4 Kjell Ahlstedt 2016-06-23 17:11:25 UTC
Created attachment 330275 [details] [review]
patch: class_gtkobject.m4: Add _CUSTOM_WRAP_FUNCTION and _NO_WRAP_FUNCTION
Comment 5 Kjell Ahlstedt 2016-06-23 17:12:18 UTC
Created attachment 330276 [details] [review]
patch: ApplicationWindow: Define it as a _CLASS_GTKOBJECT

These two patches can act as a temporary fix, until the next ABI break.
What do you think? Shall the wrap() function be marked deprecated?
Comment 6 Murray Cumming 2016-06-29 14:36:35 UTC
Thanks for that. Please go ahead. I look forward to us being able to fix all these various little things when we can break ABI one day, even if most people won't notice.
Comment 7 Kjell Ahlstedt 2016-06-29 16:23:36 UTC
Review of attachment 330276 [details] [review]:

I've pushed a very similar patch. In the pushed patch the broken wrap() function is deprecated.
Comment 8 Kjell Ahlstedt 2016-06-29 17:02:14 UTC
I've pushed this patch to gtkmm-documentation because of the changes in
_CLASS_GTKOBJECT: https://git.gnome.org/browse/gtkmm-documentation/commit/?id=343023f8e13224adcecec7b94373729f4440591a