[Development] Q_Q(), static_cast or reinterpret_cast?

[Thiago Macieira]

On sexta-feira, 29 de junho de 2012 13.14.41, Alberto Mardegan wrote:
> Hi all,
>    I just run into an issue in Qt 4.8 QDeclarativeItem (but please read
> on, the topic regards qt5 too): the geometryChanged() virtual method is
> not invoked when the size of the item is changed by modifying the
> "width" or "height" properties (for instance, from QML).
> The QDeclarativeItem's geometryChanged() method, however, is called (I
> can tell it because it's the only place where the widthChanged() signal
> is emitted, and I verified that the signal is indeed emitted).

You said that QDeclarativeItem geometryChanged() isn't called, but 
QDeclarativeITem's geometryChanged() is called. :-)

> Here is the code of setWidth():
> 
> ===========
> void QDeclarativeItemPrivate::setWidth(qreal w)
> {
>      Q_Q(QDeclarativeItem);
>      if (qIsNaN(w))
>          return;
> 
>      widthValid = true;
>      if (mWidth == w)
>          return;
> 
>      qreal oldWidth = mWidth;
> 
>      q->prepareGeometryChange();
>      mWidth = w;
> 
>      q->geometryChanged(QRectF(q->x(), q->y(), width(), height()),
>                      QRectF(q->x(), q->y(), oldWidth, height()));
> }
> ===========
> 
> So, it seems that q->geometryChanged() invokes just
> QDeclarativeItem::geometryChanged(), ignoring the fact that it's a
> virtual method. That is because the q_func() invoked with the Q_Q()
> macro uses a static_cast(), which doesn't care about derived classes,
> opposed to Q_D() which uses a reinterpret_cast().

That can't be. The q pointer is of type QDeclarativeItem* and 
geometryChanged() is a new virtual protected method there. It will do a 
virtual invocation in any case. Disassembly confirms it:

     9bf:       callq  *%r13
     9c2:       jmpq   832 <QDeclarativeItemPrivate::setWidth(double)+0x62>

     832:       mov    0x98(%rsp),%rbx
     83a:       mov    0xa0(%rsp),%rbp
     842:       mov    0xa8(%rsp),%r12
     84a:       mov    0xb0(%rsp),%r13
     852:       add    $0xb8,%rsp
     859:       retq   

That first instruction is an indirect call, which all virtual calls need to do. 
And you can see that's the last thing before the function returns.

> Is there some good reason not to use reinterpret_cast() in q_func()?
> If so, I will file a bug just against QDeclarativeItem, but I suspect
> that the pattern of calling virtual methods through the "q" pointer
> might be easily found in other classes too.
> 
> Note that q_func() uses static_cast() in Qt5 too.

Yes, it's necessary. It's because this doesn't compile:

class BasePrivate;
class Base {
protected:
    BasePrivate *d_ptr;
};

class DerivedPrivate;
class Derived : public Base {
    DerivedPrivate *d_func() { return static_cast<DerivedPrivate *>(d_ptr): }
};

It says:
<stdin>:9:74: error: invalid static_cast from type ‘BasePrivate*’ to type 
‘DerivedPrivate*’

The compiler doesn't know, at that point, that BasePrivate and DerivedPrivate 
have inheritance relationship, so static_cast is not allowed. That's why we 
need to use reinterpret_cast for the privates, but we can use static_cast for 
the publics.

reinterpret_cast between pointer types does exactly what C casts do: it 
changes the type without changing the value. That's fine, as long as there is 
no multiple inheritance involved. But note that the reinterpret_cast here 
applies to the QGraphicsItem - QDeclarativeItemPrivate inheritance, which is 
not MI. There should be no problem with this particular reinterpret_cast.

You might have found a problem with virtual functions, but I don't think it's 
because of the reasons you posted. My guess would be that it's because those 
functions are being called from inside the constructor of QDeclarativeItem. In 
that case, the object is not fully constructed and the virtual table is that 
of a QDeclarativeItem -- virtual overrides will not be called.
-- 
Thiago Macieira - thiago.macieira (AT) intel.com
  Software Architect - Intel Open Source Technology Center
     Intel Sweden AB - Registration Number: 556189-6027
     Knarrarnäsgatan 15, 164 40 Kista, Stockholm, Sweden
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 190 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.qt-project.org/pipermail/development/attachments/20120629/36120bef/attachment.sig>