[Development] Qt API review with clazy

[Konstantin Tokarev]

10.09.2016, 18:29, "Sergio Martins" <sergio.martins at kdab.com>:
> 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

Q_ENUM increases amount of metacode generated for enum. Is it really a good idea to replace Q_ENUMS everywhere?

>
> * 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
> _______________________________________________
> Development mailing list
> Development at qt-project.org
> http://lists.qt-project.org/mailman/listinfo/development

-- 
Regards,
Konstantin