QEP 77: Require use of new style Qt connections for newly added signals/slots

[nyalldawson]

QGIS Enhancement 77: Require use of new style Qt connections for newly added signals/slots

Date 2016/10/28

Author Nyall Dawson (@nyalldawson)

Contact nyall dot dawson at gmail dot com

Version QGIS 3.0

Summary

Many bugs in QGIS stem for incorrectly connected/disconnect signals. The signature of a signal or slot changes in a subtle way, and without any warning (assuming no unit test coverage of the affected code!) boom ... something breaks.

Qt5 brings a new method to connect signals and slots. The main benefit of the new method is a compile time error when the connection cannot be created. Having this breakage occur at compile time (and hence caught by the CI testing... and also... you know... people actually compiling their own changes) means that the issue is caught before the offending code is ever committed to the codebase. Win!

A good summary of the new connection method, including some of the advantages (and disadvantages), and possible traps, is available at https://wiki.qt.io/New_Signal_Slot_Syntax

Proposed Solution

We require use of the new style connections for ALL NEWLY INTRODUCED signals/slots ONLY.

The important point here is that this only applies to new signals and slots. Adding new code which connects existing signals to existing slots is not affected by this QEP. The rationale behind this distinction is that it is not possible to disconnect using the old way any connections which have been made using the new method. If careful consideration is not made when creating a connection using existing signals and slots then it is possible that existing code which disconnects these signals/slots will no longer function correctly. Rather then apply a blanket rule of "all new code must use the new method" it is instead proposed a safer approach of:

• any NEWLY introduced signals/slots must use the new style connects/disconnects
• you should continue to use old style connections for existing signals/slots UNLESS you carefully do due diligence to ensure that no old style disconnects are in place anywhere else in the existing code which may be affected by use of a new style connection

Example(s)

See the Qt docs at https://wiki.qt.io/New_Signal_Slot_Syntax

Old

connect(sender, SIGNAL (valueChanged(QString,QString)), receiver, SLOT (updateValue(QString)) );

New

connect(sender, &Sender::valueChanged, receiver, &Receiver::updateValue );

Backwards Compatibility

N/A

Votes

(required)

[NathanW2]

+1 I don't see any downsides to this at all.

Link seems to be dead though.

[sbrunner]

You insist that will affect only the new signals, why don't migrate all the current old style signal and slot?

[nyalldawson]

You mean in one go? Two reasons:

1. It's a horrible tedious job

[sbrunner]

OK, then the goal is to migrate them progressively :-) right?

[nyalldawson]

1. There's big potential to introduce new bugs if not careful. Eg there's traps with handling overloaded slots. I'd rather play it safe and upgrade them on demand.

[nyalldawson]

Bah... I can't type at the moment ;)

[nyalldawson]

Yeah - I think it should be a "fix it if you're refactoring an area" thing

[mhugo]

+1 for this proposal, everything that can be caught at compilation time is a win

[vpicavet]

Could you specify if it affects the Python API too or not ?

[nyalldawson]

This direct impact python code. That can (and should already) be using newer style connections:

http://nathanw.net/2015/04/24/psa-use-new-style-qt-signals-and-slots-not-the-old-style

[nyalldawson]

That should be "doesn't impact". Autocorrect on phone my seems to get worse every day....

[luipir]

+1 from my side

[m-kuhn]

+1

[NathanW2]

I have approved this due to the number of votes and how there is no downsides.

[NathanW2]

@nyalldawson Just added the example from the Qt docs into QEP just for quick reference.