wxtrunk breaks compiling wxSmith; ctor of wxPGProperty has been made protected

[blauzahn]

Compiling cb trunk (svn11763) with current trunk of wxWidgets fails within wxSmith plugin since yesterday (jun 29) with:

Code

libtool: compile:  g++ -std=c++11 -DHAVE_CONFIG_H -I. -I../../../../../../../trunk/src/plugins/contrib/wxSmith/wxwidgets/defitems -I../../../../../../src/include -I/usr/local/lib/wx/include/gtk2-unicode-3.1 -I/usr/local/include/wx-3.1 -D_FILE_OFFSET_BITS=64 -DWXUSINGDLL -D__WXGTK__ -pthread -I../../../../../../../trunk/src/include -I../../../../../../../trunk/src/sdk/wxscintilla/include -DCB_AUTOCONF -DCB_PRECOMP -DPIC -I../../../../../../../trunk/src/include/tinyxml -DTIXML_USE_STL=YES -O2 -ffast-math -Winvalid-pch -fPIC -fexceptions -MT wxsstatusbar.lo -MD -MP -MF .deps/wxsstatusbar.Tpo -c ../../../../../../../trunk/src/plugins/contrib/wxSmith/wxwidgets/defitems/wxsstatusbar.cpp  -fPIC -DPIC -o .libs/wxsstatusbar.o
In file included from /usr/local/include/wx-3.1/wx/propgrid/propgridiface.h:19:0,
                 from /usr/local/include/wx-3.1/wx/propgrid/propgrid.h:28,
                 from ../../../../../../../trunk/src/plugins/contrib/wxSmith/wxwidgets/defitems/../properties/../../properties/wxsproperty.h:34,
                 from ../../../../../../../trunk/src/plugins/contrib/wxSmith/wxwidgets/defitems/../properties/../../properties/wxsproperties.h:8,
                 from ../../../../../../../trunk/src/plugins/contrib/wxSmith/wxwidgets/defitems/../properties/wxsarraystringcheckproperty.h:26,
                 from ../../../../../../../trunk/src/plugins/contrib/wxSmith/wxwidgets/defitems/../wxsexproperties.h:26,
                 from ../../../../../../../trunk/src/plugins/contrib/wxSmith/wxwidgets/defitems/../wxsbaseproperties.h:26,
                 from ../../../../../../../trunk/src/plugins/contrib/wxSmith/wxwidgets/defitems/../wxsitem.h:29,
                 from ../../../../../../../trunk/src/plugins/contrib/wxSmith/wxwidgets/defitems/../wxsparent.h:26,
                 from ../../../../../../../trunk/src/plugins/contrib/wxSmith/wxwidgets/defitems/../wxstool.h:26,
                 from ../../../../../../../trunk/src/plugins/contrib/wxSmith/wxwidgets/defitems/wxsstatusbar.h:26,
                 from ../../../../../../../trunk/src/plugins/contrib/wxSmith/wxwidgets/defitems/wxsstatusbar.cpp:24:
/usr/local/include/wx-3.1/wx/propgrid/propgridpagestate.h:270:82: warning: ‘visibility’ attribute ignored [-Wattributes]
 typedef WXDLLIMPEXP_PROPGRID wxPGIterator<wxPGProperty, wxPropertyGridPageState> wxPropertyGridIterator;
                                                                                  ^
/usr/local/include/wx-3.1/wx/propgrid/propgridpagestate.h:271:94: warning: ‘visibility’ attribute ignored [-Wattributes]
 typedef WXDLLIMPEXP_PROPGRID wxPGIterator<const wxPGProperty, const wxPropertyGridPageState> wxPropertyGridConstIterator;
                                                                                              ^
In file included from /usr/local/include/wx-3.1/wx/propgrid/propgrid.h:27:0,
                 from ../../../../../../../trunk/src/plugins/contrib/wxSmith/wxwidgets/defitems/../properties/../../properties/wxsproperty.h:34,
                 from ../../../../../../../trunk/src/plugins/contrib/wxSmith/wxwidgets/defitems/../properties/../../properties/wxsproperties.h:8,
                 from ../../../../../../../trunk/src/plugins/contrib/wxSmith/wxwidgets/defitems/../properties/wxsarraystringcheckproperty.h:26,
                 from ../../../../../../../trunk/src/plugins/contrib/wxSmith/wxwidgets/defitems/../wxsexproperties.h:26,
                 from ../../../../../../../trunk/src/plugins/contrib/wxSmith/wxwidgets/defitems/../wxsbaseproperties.h:26,
                 from ../../../../../../../trunk/src/plugins/contrib/wxSmith/wxwidgets/defitems/../wxsitem.h:29,
                 from ../../../../../../../trunk/src/plugins/contrib/wxSmith/wxwidgets/defitems/../wxsparent.h:26,
                 from ../../../../../../../trunk/src/plugins/contrib/wxSmith/wxwidgets/defitems/../wxstool.h:26,
                 from ../../../../../../../trunk/src/plugins/contrib/wxSmith/wxwidgets/defitems/wxsstatusbar.h:26,
                 from ../../../../../../../trunk/src/plugins/contrib/wxSmith/wxwidgets/defitems/wxsstatusbar.cpp:24:
/usr/local/include/wx-3.1/wx/propgrid/property.h: In member function ‘virtual void wxsStatusBar::OnAddExtraProperties(wxsPropertyGridManager*)’:
/usr/local/include/wx-3.1/wx/propgrid/property.h:1897:5: error: ‘wxPGProperty::wxPGProperty(const wxString&, const wxString&)’ is protected
     wxPGProperty(const wxString& label, const wxString& name);
     ^
../../../../../../../trunk/src/plugins/contrib/wxSmith/wxwidgets/defitems/wxsstatusbar.cpp:151:120: error: within this context
         wxPGId ParentProp = Grid->Append(NEW_IN_WXPG14X wxParentProperty(wxString::Format(_("Field %d"),i+1),wxPG_LABEL));
                                                                                                                        ^
In file included from /usr/local/include/wx-3.1/wx/propgrid/propgrid.h:27:0,
                 from ../../../../../../../trunk/src/plugins/contrib/wxSmith/wxwidgets/defitems/../properties/../../properties/wxsproperty.h:34,
                 from ../../../../../../../trunk/src/plugins/contrib/wxSmith/wxwidgets/defitems/../properties/../../properties/wxsproperties.h:8,
                 from ../../../../../../../trunk/src/plugins/contrib/wxSmith/wxwidgets/defitems/../properties/wxsarraystringcheckproperty.h:26,
                 from ../../../../../../../trunk/src/plugins/contrib/wxSmith/wxwidgets/defitems/../wxsexproperties.h:26,
                 from ../../../../../../../trunk/src/plugins/contrib/wxSmith/wxwidgets/defitems/../wxsbaseproperties.h:26,
                 from ../../../../../../../trunk/src/plugins/contrib/wxSmith/wxwidgets/defitems/../wxsitem.h:29,
                 from ../../../../../../../trunk/src/plugins/contrib/wxSmith/wxwidgets/defitems/../wxsparent.h:26,
                 from ../../../../../../../trunk/src/plugins/contrib/wxSmith/wxwidgets/defitems/../wxstool.h:26,
                 from ../../../../../../../trunk/src/plugins/contrib/wxSmith/wxwidgets/defitems/wxsstatusbar.h:26,
                 from ../../../../../../../trunk/src/plugins/contrib/wxSmith/wxwidgets/defitems/wxsstatusbar.cpp:24:
/usr/local/include/wx-3.1/wx/propgrid/property.h: In member function ‘virtual void wxsStatusBar::OnExtraPropertyChanged(wxsPropertyGridManager*, wxPGProperty*)’:
/usr/local/include/wx-3.1/wx/propgrid/property.h:1897:5: error: ‘wxPGProperty::wxPGProperty(const wxString&, const wxString&)’ is protected
     wxPGProperty(const wxString& label, const wxString& name);
     ^
../../../../../../../trunk/src/plugins/contrib/wxSmith/wxwidgets/defitems/wxsstatusbar.cpp:197:128: error: within this context
                 wxPGId ParentProp = Grid->Append(NEW_IN_WXPG14X wxParentProperty(wxString::Format(_("Field %d"),i+1),wxPG_LABEL));
                                                                                                                                ^
Makefile:914: die Regel für Ziel „wxsstatusbar.lo“ scheiterte
make[7]: *** [wxsstatusbar.lo] Fehler 1

The relevant line of this is:

Code

/usr/local/include/wx-3.1/wx/propgrid/property.h:1897:5: error: ‘wxPGProperty::wxPGProperty(const wxString&, const wxString&)’ is protected

I observed this on several GNU/Linux systems: arch x86_64, ubuntu-16.04 x86_64, raspbian buster armv7 32 bit.
This is new and hasn't been already added to https://sourceforge.net/p/codeblocks/tickets.

On all systems I configured it via:

Code

(cd build && ../trunk/configure --prefix=/usr/local --with-contrib-plugins=all,-NassiShneiderman)

wxWidgets.org reference has documented that change from 3.0 to trunk. This is trunk:
https://docs.wxwidgets.org/trunk/classwx_p_g_property.html#adde08613b47eda8e324483ac754e8920

wxPGProperty::wxPGProperty(const wxString & label, const wxString & name )    
It is protected because wxPGProperty is only a base class for other property classes. Non-abstract property classes should have constructor of this style:
MyProperty( const wxString& label, const wxString& name, const T& value )

and this is 3.0:
https://docs.wxwidgets.org/3.0/classwx_p_g_property.html#adde08613b47eda8e324483ac754e8920

Here is the commit of wxWidgets that changed wxPGProperty:

Code

SHA1 ID: 8c0a210a75e6790c8383b326cff7c434d8835f7f
Autor: Artur Wieczorek <artwik@wp.pl>  2019-06-24 23:17:33
Eintragender: Artur Wieczorek <artwik@wp.pl>  2019-06-29 11:11:35
Eltern: 315ff49136d3966eb3fd837592598f4a0a913584 (Fix setting column widths after changing the page in wxPropertyGridManager)
Kind:  7a5fbbc06f649567237a329fbf63fc268c9da6d7 (Use separate event handlers to handle various mouse events in wxPGSpinButton)
Zweig: master, remotes/origin/master
Folgt auf: v3.1.2
Vorgänger von: 

    Make wxPGProperty an abstract class
    
    wxPGProperty is intended to be only a base class for property classes and therefore shouldn't be instantiated directly.

Since I do not use wxsmith and my knowledge of wxWidgets is close to zero, I do not dare to make a fix for general use.
Can one of the concete helper classes introduced here be used instead of wxPGProperty?

Code

SHA1 ID: 69632371e3c0192e940527c50e2a29898bb5d5ff
Autor: Artur Wieczorek <artwik@wp.pl>  2019-06-28 20:45:04
Eintragender: Artur Wieczorek <artwik@wp.pl>  2019-06-29 11:13:13
Eltern: bd1b5c41114a4402f4b1ef277a7e27bb6e5b3fae (Simplify calls for numeric validation)
Kind:  48adc38bbb8c847a0587d5e07e865fd87a63e97e (Refactor code for numeric validation in numeric wxPG properties)
Vorgänger von: 

    Implement wxNumericProperty as a base class for all wxPG numeric properties
    
    All numeric properties (wxIntProperty, wxUIntProperty, wxFloatProperty) share some features (like specific attributes, numeric validation, SpinCtrl editor support) so for the sake of clear design it would be good to derive them from the common base class (wxNumericProperty) in which all shared functions are implemented. This class is not intended to be instantiated so it's an abstract class.

Thank you.

[Miguel Gimenez]

Can't test with wxTrunk, but probably you can use any of the predefined derived classes (wxBoolProperty looks a simple choice)

Instead of editing wxsstatusbar.cpp you can change the definition in wxsproperty.h:51

Code

#define wxParentProperty                wxBoolProperty

[blauzahn]

At least that compiles again. But it is still not very satisfactory.

The fact, that a ctor of an abstract base class has been called not via a ctor of a derived class remains suspicious. In addition to that I am not sure how to check, whether everything still works as expected.

And it is a macro. Surely, in this legacy code macros are still abundant and will stay proably forever. Yet in this case we could replace it by c++11 using, so that the compiler sees it and can protect us better from errors:

Code

using wxParentProperty = wxBoolProperty;

This should work platform independent even on older compilers where these are still the only way to compile cb (supported from g++-4.7, clang++-3.0, MSVC 2013).

The block where this line can be changed contains 3 similar defines which currently work but IMHO can be changed as well to:

Code

#if wxCHECK_VERSION(3, 0, 0) || wxCHECK_PROPGRID_VERSION(1, 4, 0)
using wxPGVariant =                     wxVariant;
#define wxPGId                          wxPGProperty*
using wxParentProperty =                wxBoolProperty;
using wxCustomPropertyClass =           wxPGProperty;
using wxEnumPropertyClass =             wxEnumProperty;
#define wxPG_VALUETYPE(T)               wxT(#T)
#define wxPGVariantToWxObjectPtr(A,B)   wxDynamicCast(A.GetWxObjectPtr(),B)
#define wxPG_PROP_UNSPECIFIED           wxPG_EX_AUTO_UNSPECIFIED_VALUES
#define NEW_IN_WXPG14X                  new
#else
#define NEW_IN_WXPG14X
#endif

Any objections? Can and should this be committed after it has been tested?

[sodev]

According to the wxPropgrid changelog the wxParentProperty was used to create composite properties and their propsed replacement is to use a wxStringProperty and supply the special value <composed> to that one.

As a new feature in version 1.3.1, basicly any property can have children. There are few limitations, however.

Remarks:

        Names of properties with non-category, non-root parents are not stored in hash map. Instead, they can be accessed with strings like "Parent.Child". For instance, in the sample below, child property named "Max. Speed (mph)" can be accessed by global name "Car.Speeds.Max Speed (mph)".
        If you want to property's value to be a string composed based on the values of child properties, you must use wxStringProperty as parent and use value "<composed>".
        Events (eg. change of value) that occur in parent do not propagate to children. Events that occur in children will propagate to parents, but only if they are wxStringProperties with "<composed>" value.
        Old wxParentProperty class is deprecated, and remains as a typedef of wxStringProperty. If you want old value behavior, you must specify "<composed>" as wxStringProperty's value.

Now the question is, because the current code does not use a wxStringProperty nor set that value, what did a plain wxPGProperty do? Do we need the old behavior or is a plain wxStringProperty enough?

[Miguel Gimenez]

The plain wxPGProperty does not have any property, it is used only as header for subproperties.

Using wxStringProperty (without <composed>) has the same visual behaviour of wxPGProperty, the caveat is that you can edit it. I have been trying to make it read only (with SetFlagsFromString()) to no avail; only calling Enable(false) blocks editing but then the label is grayed.

The solution can be an empty derived class; I did a quick attempt but had problems with wxPG_IMPLEMENT_PROPERTY_CLASS.

@blauzahn, to check this you just have to add a wxStatusBar to a wxFrame and just look at the field enumeration, p.e. "Field 1".

EDIT: the wxWidgets documentation site was offline, now I have seen ChangeFlag(wxPG_PROP_READONLY, true) and it works with wxWidgets 3.1.2 (should work with wxTrunk).

A patch is attached. This patch also sets reasonable limits for the Fields property, as I wrote 11111 and wxSmith created that number of fields.

[sodev]

Using a wxStringProperty with the value <composed> does actually do what is required, it creates a composite property like it is used in current property grids: the parent property contains the child values as semicolon separated string, you can edit the values in the parent property or in the childs, the changes get synchronized and validated. Check the propgrid sample, there it works the same. The wxs file and the generated code stays identical.

This patch adds the required changes following the present style, i also removed some #ifdefs that could be spared by the defined macros (looks like these locations have been missed before), sadly it was not easy to separate both changesets so it's an all-in-one patch.

Tested only with wx-master on wxMSW, don't have the setup for the other variants.

[blauzahn]

I will have a look into it tomorrow.

Thank you.

[BlueHazzard]

I would really love to have a ticket about this here it will get lost for sure

[Miguel Gimenez]

I have created a ticket (847) with the number of fields limitation part of my patch.
I think the OP should create the ticket for the original subject.

[oBFusCATed]

Compile problems never get lost...

[oBFusCATed]

Fixed in trunk.

[sodev]

Only partly, all locations that use a wxStringProperty as parent property but not as composed property should be made readonly, otherwise they can be edited with no effect. wxsStatusBar was one of the locations but you missed the other one, wxsChart.

[blauzahn]

cb compiles again with current wx-trunk.

Thank you very much, Obfuscated.

PS: Sorry, that I have not examined that myself. I was just too exhausted to concentrate on that after work.

[Miguel Gimenez]

The attached patch fixes wxParentProperty in wxschart. Also reduces code duplication using the macro NEW_IN_WXPG14X like wxsstatusbar does.

[oBFusCATed]

Does this fail in any build?