GNOME Bugzilla – Bug 655416
append_column_numeric_editable doesn't compile with short or unsigned short
Last modified: 2011-09-07 14:37:38 UTC
Created attachment 192750 [details] Basic test case With a TreeModel column short or unsigned short, use of append_column_numeric_editable doesn't compile; if it isn't editable, it compile and works. I've tested it with gtkmm 2.4 (apt: 1:2.24.0-1) and 3.0 (3.0.1-1), on Debian testing. Message thrown by compiler is: In file included from /usr/include/gtkmm-2.4/gtkmm/combobox.h:34:0, from /usr/include/gtkmm-2.4/gtkmm.h:123, from main.cc:3: /usr/include/gtkmm-2.4/gtkmm/treeview.h: In function ‘void Gtk::TreeView_Private::_auto_store_on_cellrenderer_text_edited_string(const Glib::ustring&, const Glib::ustring&, int, const Glib::RefPtr<Gtk::TreeModel>&) [with ColumnType = short int]’: /usr/include/gtkmm-2.4/gtkmm/treeview.h:2463:82: instantiated from ‘void Gtk::TreeView_Private::_connect_auto_store_editable_signal_handler(Gtk::TreeView*, Gtk::CellRenderer*, const Gtk::TreeModelColumn<ColumnType>&) [with ColumnType = short int]’ /usr/include/gtkmm-2.4/gtkmm/treeview.h:2181:5: instantiated from ‘int Gtk::TreeView::append_column_numeric_editable(const Glib::ustring&, const Gtk::TreeModelColumn<ColumnType>&, const Glib::ustring&) [with ColumnType = short int]’ main.cc:63:57: instantiated from here /usr/include/gtkmm-2.4/gtkmm/treeview.h:2492:9: error: invalid cast from type ‘const Glib::ustring’ to type ‘short int’ And, for 3.0: In file included from /usr/include/gtkmm-3.0/gtkmm/combobox.h:36:0, from /usr/include/gtkmm-3.0/gtkmm/appchooserbutton.h:27, from /usr/include/gtkmm-3.0/gtkmm.h:101, from main.cc:3: /usr/include/gtkmm-3.0/gtkmm/treeview.h: In function ‘void Gtk::TreeView_Private::_auto_store_on_cellrenderer_text_edited_string(const Glib::ustring&, const Glib::ustring&, int, const Glib::RefPtr<Gtk::TreeModel>&) [with ColumnType = short int]’: /usr/include/gtkmm-3.0/gtkmm/treeview.h:2242:82: instantiated from ‘void Gtk::TreeView_Private::_connect_auto_store_editable_signal_handler(Gtk::TreeView*, Gtk::CellRenderer*, const Gtk::TreeModelColumn<ColumnType>&) [with ColumnType = short int]’ /usr/include/gtkmm-3.0/gtkmm/treeview.h:1960:5: instantiated from ‘int Gtk::TreeView::append_column_numeric_editable(const Glib::ustring&, const Gtk::TreeModelColumn<ColumnType>&, const Glib::ustring&) [with ColumnType = short int]’ main.cc:63:57: instantiated from here /usr/include/gtkmm-3.0/gtkmm/treeview.h:2271:9: error: invalid cast from type ‘const Glib::ustring’ to type ‘short int’ This bug it's (maybe) related with https://bugzilla.gnome.org/show_bug.cgi?id=118922
The test case contains the comment /* If not editable, it works */ m_TreeView.append_column_numeric_editable("index", model_columns.col, "%d"); That's part of the truth. There are 4 member functions to choose from. The results when they are used for a TreeModelColumn<short> or TreeModelColumn<unsigned short> are: append_column_numeric_editable(): Compilation error as specified in the bug description. append_column_editable(): Compilation error as specified in the bug description. append_column_numeric(): Works correctly. append_column(): Compiles without errors or warnings. Runtime warnings: GLib-GObject-WARNING **: unable to set property `text' of type `gchararray' from value of type `glibmm__CustomBoxed_s' Nothing written in the TreeView cell. append_column_numeric_editable() and append_column_editable() can be made to work for [unsigned] short by adding template function specializations of _connect_auto_store_editable_signal_handler<>() for types short and unsigned short, but that won't fix append_column(). As described in bug 118922 comment 32, the conversion of numeric data to a string in a column added with append_column() (or, I guess, insert_column()) depends on conversion functions registered by Glib with g_value_register_transform_func(). No conversion functions from [unsigned] short to string are registered, probably because [unsigned] short are not fundamental types in the GType system. See http://developer.gnome.org/gobject/unstable/gobject-Type-Information.html The chapter on the TreeView widget in the gtkmm tutorial, http://developer.gnome.org/gtkmm-tutorial/unstable/sec-treeview.html.en says about TreeView::append_column(): Note that (unsigned) short is not supported by default - You could use (unsigned) int or (unsigned) long as the column type instead. The reference documentation of TreeView::append_column(), http://developer.gnome.org/gtkmm/unstable/classGtk_1_1TreeView.html does not mention [unsigned] short, but gives examples of allowed data types, and mentions the warning "GLib-GObject-WARNING **: unable to set property `text' ..." Some alternative ways to proceed: 1. Improve the reference documentation of TreeView::append_column(), append_column_editable(), append_column_numeric_editable(), insert_column(), insert_column_editable(), mentioning that [unsigned] short are unsupported types. 2. (can be combined with 1) Add declarations, but not definitions, of template function specializations of _connect_auto_store_editable_signal_handler<>() for types short and unsigned short. That will make the compilation errors from ..._editable() slightly more understandable. /opt/gnome/include/gtkmm-3.0/gtkmm/treeview.h:2077:6: warning: inline function ‘void Gtk::TreeView_Private::_connect_auto_store_editable_signal_handler(Gtk::TreeView*, Gtk::CellRenderer*, const Gtk::TreeModelColumn<ColumnType>&) [with ColumnType = short unsigned int]’ used but never defined /tmp/ccqn5gRK.o: In function `int Gtk::TreeView::append_column_numeric_editable<unsigned short>(Glib::ustring const&, Gtk::TreeModelColumn<unsigned short> const&, Glib::ustring const&)': /opt/gnome/include/gtkmm-3.0/gtkmm/treeview.h:1962: undefined reference to `void Gtk::TreeView_Private::_connect_auto_store_editable_signal_handler<unsigned short>(Gtk::TreeView*, Gtk::CellRenderer*, Gtk::TreeModelColumn<unsigned short> const&)' 3. Add definitions of template function specializations of _connect_auto_store_editable_signal_handler<>() for types short and unsigned short. That will make ..._editable() work, but it's inconsistent to make append_column_editable() work, but not append_column(). 4. (shall be combined with 3) Register suitable conversion functions with g_value_register_transform_func(). Is it really wise to do that? g_value_transform() is used by g_object_set_property(), so some new registered conversion functions can affect just about any GObject class.
(In reply to comment #1) > 3. Add definitions of template function specializations of > _connect_auto_store_editable_signal_handler<>() for types short and > unsigned short. That will make ..._editable() work, but it's inconsistent > to make append_column_editable() work, but not append_column(). Wrong! Shall read: 3. Add definitions of template function specializations of _connect_auto_store_editable_signal_handler<>() for types short and unsigned short. That will make append_column_numeric_editable() work. append_column_editable() will behave as append_column(), i.e. it compiles without errors or warnings, but it prints run-time warnings. It writes nothing in the TreeView cell. The behaviour of append_column_editable() actually becomes worse with this modification. A compile-time error is removed, but a run-time error remains. I think it's possible to make a reasonably simple modification that affects only append_column_numeric_editable(). Then append_column_numeric() and append_column_numeric_editable() can be used for [unsigned] short data, but append_column() and append_column_editable() cannot. Hopefully I can attach a patch later today. If not, it will take a few weeks. After today I will make a break from all work with gtkmm for 2-3 weeks.
Created attachment 193314 [details] [review] patch: Make TreeView::append_column_numeric_editable work for [unsigned] short Here's a patch that makes TreeView::append_column_numeric_editable() work for [unsigned] short columns. A few questions: -- I've added a function template, and modified other function templates in treeview.hg. Can that be done without breaking ABI? If any of the supported compilers save information about function templates in an object code library, I suppose my changes might break ABI. Is that possible, or is all information about function templates always fetched from the header files when an application file is compiled? -- Why aren't there any insert_column_numeric() and insert_column_numeric_editable() functions?
(In reply to comment #3) > Created an attachment (id=193314) [details] [review] > patch: Make TreeView::append_column_numeric_editable work for [unsigned] short > > Here's a patch that makes TreeView::append_column_numeric_editable() work for > [unsigned] short columns. A few questions: I'm a little surprised that a) We had all those template specializations that seem to have the same copy/pasted implementations. Are they really all the same? b) You now just call the new function from them. Are they even needed? > -- I've added a function template, and modified other function templates in > treeview.hg. Can that be done without breaking ABI? If any of the supported > compilers save information about function templates in an object code library, > I suppose my changes might break ABI. Is that possible, or is all information > about function templates always fetched from the header files when an > application file is compiled? I don't think that's a problem. > -- Why aren't there any insert_column_numeric() and > insert_column_numeric_editable() functions? Laziness?
(In reply to comment #4) > I'm a little surprised that > a) We had all those template specializations that seem to have the same > copy/pasted implementations. Are they really all the same? Yes, the 6 template specializations for int, unsigned int, long, unsigned long, float, double are really the same. > b) You now just call the new function from them. Are they even needed? I hope they are not needed, because I don't like them. This kind of repetition of identical code for different data types is exactly what you can often replace by a C++ template function. But I don't know how we can do without them. The problem is explained in a comment in the patch in comment 3. I'll try another explanation here: append_column_editable<>() and insert_column_editable<>() (without the patch also append_column_numeric_editable<>()) call template function _connect_auto_store_editable_signal_handler<ColumnType>(). There are 3 versions of _connect_auto_store_editable_signal_handler<>(): 1. The general version for string data. It connects CellRendererText::signal_edited() to template function _auto_store_on_cellrenderer_text_edited_string<ColumnType>(). 2. A bool specialization. It connects CellRendererToggle::signal_toggled() to function _auto_store_on_cellrenderer_toggle_edited_with_model(). 3. Six identical (except for the type name) specializations for numeric data. They connect CellRendererText::signal_edited() to template function _auto_store_on_cellrenderer_text_edited_numerical<ColumnType>(). Perhaps the version for numeric data could be the general one? Then there could be specializations for all possible string types. Are there other acceptable string types than Glib::ustring and std::string?
Thanks for the explanation. I guess the general string one is also for char*, const char*, char[], etc, so we'd end up adding those specializations instead. It's a pity that we can't just say some_specialization<Numeric Types>, but nevermind. Please commit, but with a comment saying that we need all those specializations, with identical implementations, because we chose to avoid multiple specializations for the string types by dealing with string types in the default implementation.
Created attachment 195879 [details] [review] patch: Make TreeView::append_column_numeric_editable work for [unsigned] short Second version of this patch, with a long comment in treeview.hg explaining why we need all those template specializations. I have pushed the patch, and thus this bug has been fixed.