[PyQt] QIconEngine ownership regression in PyQt5 5.12

[Phil Thompson]

On 19 Mar 2019, at 12:35 pm, Ales Erjavec <ales.erjavec324 at gmail.com> wrote:
> 
> Maybe the ownership could be transferred to the C++ (sip)QIconEngine itself?

That's what the previous buggy code actually did.

> If I construct the QIcon with:
> 
> ## code
> 
> def qicon_transfer(engine):
>    # type: (QIconEngine) -> QIcon
>    import sip
>    icon = QIcon(engine)
>    sip.transferto(engine, engine)
>    return icon
> 
> ## /code

sip.transferto(engine, None) is better.

> Then the engine is preserved until the last icon is deleted.

That's because QIconEngine has a virtual dtor which is used to detect when C++ invokes it.

> ## code
> 
> def test_icon_transfer():
>    from weakref import ref
>    eng = IconEngine("a", QColor("red"))
>    engref = ref(eng)
> 
>    icon = qicon_transfer(eng)
> 
>    del eng
>    assert engref() is not None
>    icon1 = QIcon(icon)
> 
>    del icon
>    assert engref() is not None
>    icon2 = QIcon(icon1)
> 
>    del icon1
>    assert engref() is not None
> 
>    del icon2
>    assert engref() is None
> 
> ## /code
> 
> There should be no issue with the transfer back because there is no
> way to get the engine out of the QIcon.
> 
> I think this preserves the Qt's design. QIcon has value type semantics
> and practically all API's that accept a QIcon do so by value (copy construct).

It is very unusual (I can't think of another case) where a class with value type semantics can take ownership of another object. sip does not have explicit support for this pattern so I'll add handwritten code to do the right thing.

Thanks,
Phil