[Development] RFC: banning _q_slot() in favour of new-style connect()?

[Marc Mutz]

On Friday October 12 2012, Thiago Macieira wrote:
> On sexta-feira, 12 de outubro de 2012 07.27.51, Marc Mutz wrote:
> > Hi,
> >
> > I was wondering whether we should stop using the pattern of declaring
> > _q_privateSlots() in favour of connecting to functors or functions
> > directly.
>
> Makes sense, except that it's of inconvenient use:
>  - lambdas: can't use in Qt code
>  - functors: inconvenient (need to be out of the class)
>  - static member or free functions: will not have access to the target
> object - private member functions: disallowed by Qt coding guidelines
>
> > Rationale:
> > R1. the _q_ prefix is just a convention, the slot can still be called
> > through the meta-object and still can be reimplemented in more-derived
> > classes. With new-style connects, there'd be no slot to call or
> > reimplement, so the code would become easier to understand (d/t more
> > narrow scope).
>
> Good.
>
> > R2: less space used in meta-object
>
> True, but minimal impact.

QLineEdit has up to seven _q_ slots, vs. up to eight normal ones (exact number 
depends on #ifdefs). But yes, I need to measure whether the reduction in the 
meta-object isn't offset by the extra code for another QSlotObjectBase 
subclass.

So I replaced one of the _q_ slots in QLineEdit with a functor. Result:

   text    data     bss     dec     hex filename
6356969  296120    2712 6655801  658f39 lib/libQtWidgets-new.so.5.0.0
6356959  296184    2712 6655855  658f6f lib/libQtWidgets.so.5.0.0

(text + 10, data - 64)

relinfo results are identical between the two.

Given there are 226 occurrences of Q_PRIVATE_SLOT in QtWidgets, we're looking 
at a 2k text increase vs. a 14k data decrease.

I actually feared worse when I thought about all those additional 
QSlotObjectBase subclasses...

> > R3: allows patch releases to add or remove code that would formerly have
> >   required _q_slots().
>
> Not relevant, since this is already permitted.
>
> > Gotchas:
> > G1. The slot could have been overridden in a more-derived class
>
> Extremely unlikely, which is why they have a _q_ prefix in their names.

I'm not talking about accidental reimplementations, but intentional ones.

> > G2. The Private pointer could change in between the connect() and the
> > emit. Currently, this is transparently handled by injecting a call to
> > d_func() into the moc-generated code. For new-style connect, such a
> > situation would require a disconnect/connect pair.
>
> That's even more unlikely to the point I can confidently say it never
> happens. (remember Q_PRIVATE_SLOT is a private macro, so no one outside of
> Qt is using it)

QDBusConnectionPrivate::deliverCall does something 'funny' with 
QDBusContextPrivate (::set()), but I didn't look too deeply into it.

Anyway, I've used Q_PRIVATE_SLOT in all kinds of projects, including KDE, so 
it's not like no-one outside of Qt is using it.

Even so: a 'normal' slot (public Q_SLOTS:) would override the Q_PRIVATE_SLOT 
in the meta-object, too, so no Q_PRIVATE_SLOT use is required in the first 
place.

> > The full solution would be to try to remove all _q_slots() from Qt 5.0.
> > Seeing as this change could also be done in 5.1, I'd only propose to ban
> > _new_ _q_slots() from being added.
>
> If we agree on how to use them, yes. The only solution we *can* use is
> functors, which are hard to use.

Not harder to use than Q_PRIVATE_SLOT, IMO. More annoying, yes, because of 
boilerplate code (of which Q_PRIVATE_SLOT also has some, mind). Real-life 
example:

// .cpp:
struct UpdateAccessibilityOnCursorPositionChanged {
    QTextEdit *const q;
    explicit UpdateAccessibilityOnCursorPositionChanged(QTextEdit *te)
        : q(te) {}
    void operator()() const {
        QAccessibleTextCursorEvent event(q, q->textCursor().position());
        QAccessible::updateAccessibility(&event);
    }
};

vs.

// _p.h:

Q_PRIVATE_SLOT(d_func(), void _q_cursorPositionChanged())

// .cpp
void QTextEditPrivate::_q_cursorPositionChanged()
{
    Q_Q(QTextEdit);
    QAccessibleTextCursorEvent event(q, q->textCursor().position());
    QAccessible::updateAccessibility(&event);
}

And: the more functional code, the less the (constant) boilerplate code 
matters.

It's different pattern, but it isn't harder to learn from first principles 
than the Q_PRIVATE_SLOT one.

-- 
Marc Mutz <marc.mutz at kdab.com> | Senior Software Engineer
KDAB (Deutschland) GmbH & Co.KG, a KDAB Group Company
www.kdab.com || Germany +49-30-521325470 || Sweden (HQ) +46-563-540090
KDAB - Qt Experts - Platform-Independent Software Solutions