Deprecated QScoketNotifier::activated signal overload

[Phil Thompson]

On 20/03/2023 16:52, Florian Bruhin wrote:
> On Mon, Mar 20, 2023 at 04:21:46PM +0000, Phil Thompson wrote:
>> 
>> On 17/03/2023 17:36, Florian Bruhin wrote:
>> > Hey,
>> >
>> > When connecting to a QSocketNotifier's activated signal, it looks like
>> > PyQt insists on using the deprecated variant:
>> >
>> > QSocketNotifier::activated(int socket)
>> > https://doc.qt.io/qt-6/qsocketnotifier-obsolete.html#activated-1
>> >
>> > Rather than using the new replacement (since Qt 5.15):
>> >
>> > QSocketNotifier::activated(QSocketDescriptor socket,
>> > QSocketNotifier::Type type)
>> > https://doc.qt.io/qt-6/qsocketnotifier.html#activated
>> >
>> > [...]
>> 
>> I didn't implement the newer signal because of the undocumented
>> QSocketDescriptor class. I have a general rule that if something isn't
>> documented then it's not part of the public API.
> 
> It looks like it is documented in the source:
> https://code.qt.io/cgit/qt/qtbase.git/tree/src/corelib/kernel/qsocketnotifier.cpp?h=v6.4.3#n338
> 
> Why it doesn't end up in the generated documentation is beyond me.

Probably because it's marked as internal.

>> Why specify QSocketDescriptor when QSocketNotifier uses qintptr to
>> represent a socket?
>> The whole thing had a bad smell.
> 
> From what I understand, the issue is that the socket Qt::HANDLE can be
> 64bit on Windows, but gets truncated to a 32bit int (and not a qintptr)
> in the signal:
> 
> https://bugreports.qt.io/browse/QTBUG-70441
> https://codereview.qt-project.org/c/qt/qtbase/+/294902

Understood but as it has no API you can't do anything with it anyway.

>> Is the loss of the second argument a problem?
> 
> Not for me - the truncation of the socket *might* be, but then again,
> I can't say I actually ran into trouble in practice so far. I mostly
> happened to see the deprecation warning and thought I'd mention it,
> since PyQt's policy seemed to be to not use/expose deprecated API where
> possible.

I'd prefer a deprecated API to an undocumented/internal API.

Phil