Author Topic: wxtrunk breaks compiling wxSmith; ctor of wxPGProperty has been made protected  (Read 1502 times)

Offline blauzahn

  • Multiple posting newcomer
  • *
  • Posts: 101
Compiling cb trunk (svn11763) with current trunk of wxWidgets fails within wxSmith plugin since yesterday (jun 29) with:

Code: [Select]
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: [Select]
/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: [Select]
(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

Quote
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: [Select]
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: [Select]
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.

Offline Miguel Gimenez

  • Regular
  • ***
  • Posts: 333
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: [Select]
#define wxParentProperty                wxBoolProperty

Offline blauzahn

  • Multiple posting newcomer
  • *
  • Posts: 101
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: [Select]
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: [Select]
#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?

Offline sodev

  • Regular
  • ***
  • Posts: 314
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.

Quote
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?

Offline Miguel Gimenez

  • Regular
  • ***
  • Posts: 333
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.
« Last Edit: July 02, 2019, 02:56:45 pm by Miguel Gimenez »

Offline sodev

  • Regular
  • ***
  • Posts: 314
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.

Offline blauzahn

  • Multiple posting newcomer
  • *
  • Posts: 101
I will have a look into it tomorrow.

Thank you.

Offline BlueHazzard

  • Developer
  • Lives here!
  • *****
  • Posts: 2455
I would really love to have a ticket about this ;) here it will get lost for sure

Offline Miguel Gimenez

  • Regular
  • ***
  • Posts: 333
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.

Offline oBFusCATed

  • Developer
  • Lives here!
  • *****
  • Posts: 11932
    • Travis build status
Compile problems never get lost... :)
(most of the time I ignore long posts)
[strangers don't send me private messages, I'll ignore them; post a topic in the forum, but first read the rules!]

Offline oBFusCATed

  • Developer
  • Lives here!
  • *****
  • Posts: 11932
    • Travis build status
Fixed in trunk.
(most of the time I ignore long posts)
[strangers don't send me private messages, I'll ignore them; post a topic in the forum, but first read the rules!]

Offline sodev

  • Regular
  • ***
  • Posts: 314
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.

Offline blauzahn

  • Multiple posting newcomer
  • *
  • Posts: 101
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.

Offline Miguel Gimenez

  • Regular
  • ***
  • Posts: 333
The attached patch fixes wxParentProperty in wxschart. Also reduces code duplication using the macro NEW_IN_WXPG14X like wxsstatusbar does.

Offline oBFusCATed

  • Developer
  • Lives here!
  • *****
  • Posts: 11932
    • Travis build status
Does this fail in any build?
(most of the time I ignore long posts)
[strangers don't send me private messages, I'll ignore them; post a topic in the forum, but first read the rules!]

Offline sodev

  • Regular
  • ***
  • Posts: 314
Does this fail in any build?

Does your commit from yesterday fail in any build?

The answer to both questions is the same :).

The removal of the #ifdefs is the same like in my patch and the same like the current code already did in other locations so this doesn't change anything. The only question is if setting the readonly property is possible in previous propgrid versions.

Offline oBFusCATed

  • Developer
  • Lives here!
  • *****
  • Posts: 11932
    • Travis build status
@sodev: This is not helpful.
(most of the time I ignore long posts)
[strangers don't send me private messages, I'll ignore them; post a topic in the forum, but first read the rules!]

Offline sodev

  • Regular
  • ***
  • Posts: 314
Maybe i didn't understand your question correctly, i thought you wanted to know if this patch does do anything different than the last one you already applied in regard to features that may cause compatibility issues. I just confirmed that it does exactly the same like the last one plus that the other optimizations are safe.

If that wasn't your question, what was it then?

Offline oBFusCATed

  • Developer
  • Lives here!
  • *****
  • Posts: 11932
    • Travis build status
No, this was not my question. The question is "which build fails without this patch".
I'm asking this because I can build the CodeBlocks_wx30-unix.workspace just fine without it.
(most of the time I ignore long posts)
[strangers don't send me private messages, I'll ignore them; post a topic in the forum, but first read the rules!]

Offline Miguel Gimenez

  • Regular
  • ***
  • Posts: 333
wxsChart uses wxParentProperty, jusrt like wxsStatusBar does. There where a build problem (identical to wxsStatusBar one) and both were fixed in commit 11783.

The issue fixed by this patch is making the wxParentProperty read only, like was done to wxsStatusBar in 11783.

Also I have unified some code using the macro NEW_IN_WXPG14X defined in wxsproperty.h just like wxsStatusBar does. No real change.

Offline sodev

  • Regular
  • ***
  • Posts: 314
Compiling cb trunk (svn11763) with current trunk of wxWidgets fails within wxSmith plugin since yesterday (jun 29) with:

Doesn't build since 8c0a210a75e6790c8383b326cff7c434d8835f7f which is far ahead of wx30 ;).

Offline oBFusCATed

  • Developer
  • Lives here!
  • *****
  • Posts: 11932
    • Travis build status
Doesn't build since 8c0a210a75e6790c8383b326cff7c434d8835f7f which is far ahead of wx30 ;).
So the build for you is failing? I'm on wx-master from yesterday and on linux everything builds. If you have build problem either post a build log or a patch.
(most of the time I ignore long posts)
[strangers don't send me private messages, I'll ignore them; post a topic in the forum, but first read the rules!]

Offline sodev

  • Regular
  • ***
  • Posts: 314
The build was failing before CodeBlocks 11783 but is working since then.

The additional patch does not fix building but fixes 11783 which enables editing of elements which should not be editable simply because 11783 forgot to change these elements accordingly. I hope this clears the confusion.

1) wxparentproperty.patch + wxschart.patch -> Fix building by using readonly parent property
2) wxs-1-composed.patch -> Fix buildind by using composed parent property

1) and 2) are mutually exclusive, you applied only half of 1), the other half is missing :)
« Last Edit: July 10, 2019, 10:55:00 pm by sodev »

Offline Miguel Gimenez

  • Regular
  • ***
  • Posts: 333
Created ticket 853 with the patch in Reply #13

https://sourceforge.net/p/codeblocks/tickets/853/