[Development] Qt API review with clazy

[Sergio Martins]

Hi,


I've been developing a Qt oriented clang compiler plugin called clazy [1].

It has about 50 custom checks/warnings for common Qt and C++ mistakes.
I would like to propose clazy to be used to sanitize API of new Qt modules. Of 
course, only a few of the 50 checks are relevant to API, so I will only talk 
about those.


** Checks that can find bugs **


* clazy-copyable-polymorphic

polymorphic copyable classes are usually vulnerable to slicing [2]
The cases I found were either real bugs or easily fixable by adding 
Q_DISABLE_COPY.



* clazy-rule-of-two-soft

Finds when you're calling the compiler generated copy-ctor of a class that has 
a user-defined assign operator (and vice-versa).
You'll want to write both, or none. I've found many bugs where one of them was 
missing (where the copy-ctor didn't have the same behavior as the assign-
operator).

In most cases you can simply remove the user defined methods and let the 
compiler generate them.




* clazy-rule-of-three

Either implement all 3 dtor, copy-ctor and assign-op or none of them.

This is my favorite as it finds many bugs which are easy to fix.

It's very common to have a class that is holding a resource and frees it in 
the DTOR, but if that class gets copied then two DTORs might run, freeing the 
resource twice, which might crash or misbehave the app.

See for example https://codereview.qt-project.org/#/c/151488/ , QBasicTimer is 
copyable, but when a copy is destroyed it stops both timers.

Also common is copying a class that has a d-pointer and getting a crash when 
both DTORs are run because you forgot the copy-ctor and assign-op.

So either implement the copy-ctor and assign-op too, or use Q_DISABLE_COPY, or 
in the majority of cases, just remove your DTOR if it doesn't do any work.



* mutable-container-key

QMap or QHash having a key that can be modified by external factors (like 
QPointer)





** Nice to haves (don't find real bugs but are related to performance) **


* clazy-function-args-by-ref

Pass big or non-trivial types (so copy-ctor and dtor don't get called) by 
const-ref



* clazy-missing-typeinfo

Types used in containers but missing a Q_DECLARE_TYPEINFO



* clazy-qenums:

Use Q_ENUM instead of Q_ENUMS


* clazy-missing-qobject-macro

missing a Q_OBJECT macro



** Debatable **

* clazy-function-args-by-value
Pass small and trivial types by value


Thanks for reading.


[1] https://github.com/KDE/clazy
[2] https://en.wikipedia.org/wiki/Object_slicing



Regards,
-- 
Sérgio Martins | sergio.martins at kdab.com | Senior Software Engineer
Klarälvdalens Datakonsult AB, a KDAB Group company
Tel: Sweden (HQ) +46-563-540090, USA +1-866-777-KDAB(5322)
KDAB - The Qt Experts