[Development] Qt API review with clazy

[Jérémie Delaitre]

Can the same checks be implemented in clang-tidy instead of having yet
another tool? clang-tidy now has boost specific checks so maybe they would
also accept a Qt specific module in there.

On Sun, 11 Sep 2016 at 03:29 Sergio Martins <sergio.martins at kdab.com> wrote:

> 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
> _______________________________________________
> Development mailing list
> Development at qt-project.org
> http://lists.qt-project.org/mailman/listinfo/development
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.qt-project.org/pipermail/development/attachments/20160911/6ca21a2a/attachment.html>