[Development] Is overriding an existing virtual method 'BC' in Qt 4?

[Thiago Macieira]

On quarta-feira, 26 de setembro de 2012 13.54.10, Stephen Kelly wrote:
> Hi there,
> 
> In
> 
>  https://codereview.qt-project.org/#patch,unified,35581,1,/COMMIT_MSG
> 
> we have a discussion about whether to reimplement a virtual method (the
> obvious way to fix this bug, as was done in the Qt 5 patch), or to avoid
> doing so.

"You can ... reimplement virtual functions defined in the primary base class 
hierarchy (that is, virtuals defined in the first non-virtual base class, or in 
that class's first non-virtual base class, and so forth) if it is safe that 
programs linked with the prior version of the library call the implementation 
in the base class rather than the new one."

The last part (safe calling the previous implementation) applies in two cases:
 1) when the compiler knows the class type and does not do a virtual call 
(e.g., in the constructor of a derived class or when the object was 
instantiated in the same function)

 2) when overriding this particular virtual in a further-derived class.

Marc pointed out that either case is unlikely to happen, so overriding should 
be ok.

> In this case, code which gets recompiled will get the bug fix, and code that
> does not get recompiled will not get the bug fix.

That's not what Marc said. Since there isn't much devirtualisation going on, 
all code will get the fix, whether it's recompiled or not.

> Do we care about cases like that? Existing compiled code will continue to
> work as before. From the point of view of the dynamic linker, it is BC as
> there is no change to the virtual table size etc (AFAIK).
> 
> Do we have a hard policy for this? 'It's fine to add virtual method
> overrides as long as the existing compiled code continues to work without
> new bugs' makes sense, but do we have it written down anywhere authoritive?

We do have it written down: the KDE page on binary compatibility with C++.

But note that there's one stricter requirement: the forwards compatibility 
that applies within a patch series. Adding this new virtual within the same 
patch series means a new, public symbol, which could get used in applications.

If there's a sensible implementation that does not require overriding the 
virtual, then we should use it. Otherwise, I'd say go for it.

-- 
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/20120926/482ed65d/attachment.sig>