Troubleshooting sip assignHelper != NULL assertion

[Scott Talbert]

On Sat, 30 May 2020, Phil Thompson wrote:

> On 28/05/2020 16:52, Scott Talbert wrote:
>> On Thu, 28 May 2020, Phil Thompson wrote:
>> 
>>> On 28/05/2020 16:30, Scott Talbert wrote:
>>>> On Thu, 28 May 2020, Phil Thompson wrote:
>>>> 
>>>>>>>> Hi,
>>>>>>>> 
>>>>>>>> I'm running into the following assertion in wxPython that I don't
>>>>>>>> quite understand:
>>>>>>>> 
>>>>>>>> python3: ../../../../sip/siplib/siplib.c:3444: parseResult: Assertion
>>>>>>>> `assign_helper != NULL' failed.
>>>>>>>> 
>>>>>>>> This is the relevant C++ class:
>>>>>>>> 
>>>>>>>> class wxPGWindowList
>>>>>>>> {
>>>>>>>> public:
>>>>>>>>     wxPGWindowList(wxWindow* primary, wxWindow* secondary = NULL)
>>>>>>>>         : m_primary(primary)
>>>>>>>>         , m_secondary(secondary)
>>>>>>>>     {
>>>>>>>>     }
>>>>>>>>
>>>>>>>>     void SetSecondary(wxWindow* secondary) { m_secondary = secondary; 
>>>>>>>> }
>>>>>>>>
>>>>>>>>     wxWindow* GetPrimary() const { return m_primary; }
>>>>>>>>     wxWindow* GetSecondary() const { return m_secondary; }
>>>>>>>>
>>>>>>>>     wxWindow*   m_primary;
>>>>>>>>     wxWindow*   m_secondary;
>>>>>>>> };
>>>>>>> 
>>>>>>> It doesn't matter what the C++ class looks like, it's the 
>>>>>>> corresponding .sip that would be of interest.
>>>>>> 
>>>>>> My bad, here it is:
>>>>>> class wxPGWindowList
>>>>>> {
>>>>>>     %Docstring
>>>>>>         PGWindowList(primary, secondary=None)
>>>>>>
>>>>>>         Contains a list of editor windows returned by CreateControls.
>>>>>>     %End
>>>>>>     %TypeHeaderCode
>>>>>>         #include <wx/propgrid/editors.h>
>>>>>>     %End
>>>>>> 
>>>>>> public:
>>>>>>     wxPGWindowList(
>>>>>>         wxWindow * primary,
>>>>>>         wxWindow * secondary = NULL
>>>>>>     );
>>>>>>
>>>>>>     void SetSecondary(
>>>>>>         wxWindow * secondary
>>>>>>     );
>>>>>>     %Docstring
>>>>>>         SetSecondary(secondary)
>>>>>>     %End
>>>>>>
>>>>>>     wxWindow * GetPrimary() const;
>>>>>>     %Docstring
>>>>>>         GetPrimary() -> wx.Window
>>>>>>
>>>>>>         Gets window of primary editor.
>>>>>>     %End
>>>>>>
>>>>>>     wxWindow * GetSecondary() const;
>>>>>>     %Docstring
>>>>>>         GetSecondary() -> wx.Window
>>>>>>
>>>>>>         Gets window of secondary editor.
>>>>>>     %End
>>>>>>
>>>>>>     public:
>>>>>> 
>>>>>>
>>>>>>     %Property(name=Primary, get=GetPrimary)
>>>>>>     %Property(name=Secondary, get=GetSecondary, set=SetSecondary)
>>>>>> };  // end of class wxPGWindowList
>>>>>> 
>>>>>> 
>>>>>>>> The assertion occurs when trying to return an instance of
>>>>>>>> wxPGWindowList in a Python method, e.g.:
>>>>>>>> 
>>>>>>>> def foo():
>>>>>>>>     return wxpg.PGWindowList(a, b)
>>>>>>>> 
>>>>>>>> From what I can tell, there is no assignment helper assigned by sip
>>>>>>>> because there is no default constructor?  I may be missing something,
>>>>>>>> but I can't see why a default constructor would be needed.
>>>>>>> 
>>>>>>> You don't say what version of the sip module you are using but I'm 
>>>>>>> guessing that that assertion is when it's parsing the Python object 
>>>>>>> returned by a re-implementation of a C++ virtual. That doesn't seem to 
>>>>>>> be happening in the above which suggests things aren't happening where 
>>>>>>> you think they are.
>>>>>> 
>>>>>> sip module version is 4.19.19.  You are correct, I oversimplified what
>>>>>> is actually happening.  This is really what is happening:
>>>>>> 
>>>>>> class LargeImageEditor(wxpg.PGEditor):
>>>>>>     def CreateControls(self, propgrid, property, pos, sz):
>>>>>>         ...
>>>>>>         return wxpg.PGWindowList(self.tc, btn)
>>>>>> 
>>>>>> Where CreateControls is a C++ virtual, relevant .sip snippet:
>>>>>>
>>>>>>     virtual
>>>>>>     wxPGWindowList CreateControls(
>>>>>>         wxPropertyGrid * propgrid,
>>>>>>         wxPGProperty * property,
>>>>>>         const wxPoint & pos,
>>>>>>         const wxSize & size
>>>>>>     ) const = 0;
>>>>>>     %Docstring
>>>>>>         CreateControls(propgrid, property, pos, size) -> PGWindowList
>>>>>>
>>>>>>         Instantiates editor controls.
>>>>>>     %End
>>>>> 
>>>>> So SIP need to copy the wxPGWindowList from the stack to the heap. 
>>>>> Shouldn't CreateControls return a pointer to the wxPGWindowList?
>>>>> 
>>>>> Of course SIP should detect this when generating the code rather than 
>>>>> rely on a runtime assertion.
>>>> 
>>>> That is probably how I would have designed the API, but alas it wasn't
>>>> done that way.  :)
>>>> 
>>>> Shouldn't a (default) copy constructor be sufficient to copy the
>>>> wxPGWindowList in this case?
>>> 
>>> SIP may be being too conservative when determining if there is an implied 
>>> copy ctor. Try adding an explicit copy ctor to the wxPGWindowList .sip 
>>> file.
>> 
>> I don't think that will be enough, though.  SIP seems to only set the
>> assignment helper if there are *both* a public default constructor and
>> a public copy constructor, see:
>> 
>> https://www.riverbankcomputing.com/hg/sip/file/d85e9957e726/sipgen/transform.c#l596
>> 
>> Scott
>
> Should be fixed in the current repo. SIP v4.19.23 will be released next week.
>
> Be aware that the fix may expose missing private ctors in other .sip files - 
> PyQt had two cases.

Thanks for the quick fix.

However, I think I found a situation with the new code where an assign 
function is getting created where it probably shouldn't be.

struct wxSplitterRenderParams
{
     %Docstring
         SplitterRenderParams(widthSash_, border_, isSens_)

         This is just a simple struct used as a return value of
         wxRendererNative::GetSplitterParams().
     %End
     %TypeHeaderCode
         #include <wx/renderer.h>
     %End

     wxSplitterRenderParams(
         wxCoord widthSash_,
         wxCoord border_,
         bool isSens_
     );

     const wxCoord border;

     const bool isHotSensitive;

     const wxCoord widthSash;

};  // end of class wxSplitterRenderParams

This results in:

../../../../sip/cpp/sip_corewxSplitterRenderParams.cpp: In function ‘void 
assign_wxSplitterRenderParams(void*, Py_ssize_t, void*)’:
../../../../sip/cpp/sip_corewxSplitterRenderParams.cpp:31:125: error: use 
of deleted function ‘wxSplitterRenderParams& 
wxSplitterRenderParams::operator=(const wxSplitterRenderParams&)’
    31 |     reinterpret_cast< ::wxSplitterRenderParams 
*>(sipDst)[sipDstIdx] = *reinterpret_cast< ::wxSplitterRenderParams 
*>(sipSrc);
       | 
^
In file included from 
../../../../sip/cpp/sip_corewxSplitterRenderParams.cpp:12:
../../../../../Phoenix/ext/wxWidgets/include/wx/renderer.h:97:25: note: 
‘wxSplitterRenderParams& wxSplitterRenderParams::operator=(const 
wxSplitterRenderParams&)’ is implicitly deleted because the default 
definition would be ill-formed:
    97 | struct WXDLLIMPEXP_CORE wxSplitterRenderParams
       |                         ^~~~~~~~~~~~~~~~~~~~~~
../../../../../Phoenix/ext/wxWidgets/include/wx/renderer.h:97:25: error: 
non-static const member ‘const wxCoord wxSplitterRenderParams::widthSash’, 
cannot use default assignment operator
../../../../../Phoenix/ext/wxWidgets/include/wx/renderer.h:97:25: error: 
non-static const member ‘const wxCoord wxSplitterRenderParams::border’, 
cannot use default assignment operator
../../../../../Phoenix/ext/wxWidgets/include/wx/renderer.h:97:25: error: 
non-static const member ‘const bool 
wxSplitterRenderParams::isHotSensitive’, cannot use default assignment 
operator

Scott