Skip to content

Clazy 1.4 released presenting 10 new Qt compile-time checks

Clazy 1.4 has been released and brings 10 new checks.

Clazy is a clang compiler plugin which emits warnings related to Qt best practices. We’ll be showing Clazy at Qt World Summit in Boston, Oct 29-30, where we are a main Sponsor.

You can read more about it in our previous blog posts:

Today I’ll go through all 10 new warnings, one by one. For other changes, check the complete 1.4 Changelog. So let’s start. I’ve ordered them according to my personal preference, starting with the ones that have helped me uncover the most bugs and finishing with some exotic ones which you’ll rarely need.

1. skipped-base-method

Warns when calling a method from the grand-base class instead of the direct base class one.

Example:

class MyFrame : public QFrame
{
    Q_OBJECT
public:
    bool event(QEvent *ev) override
    {
        (...)
        // warning: Maybe you meant to call QFrame::event() instead [-Wclazy-skipped-base-method]
        return QWidget::event(ev); 
    }
};

The motivation came after hitting bugs when overriding QObject::event() and QObject::eventFilter(). I would suggest always using this check and annotating your legit cases with // clazy:exclude=skipped-base-method, to make your intention clear. The same way you would comment your fallthroughs in switch statements.

This check might get removed in the future, as in the end it’s not specific to Qt and clang-tidy recently got a similar feature.

2. wrong-qevent-cast

Warns when a QEvent is cast to the wrong derived class via static_cast.

Example:

switch (ev->type()) {
    case QEvent::MouseMove:
        auto e = static_cast<QKeyEvent*>(ev);
}

Only casts inside switch statements are verified.

3. qhash-with-char-pointer-key

Finds cases of QHash<const char *, T>. It’s error-prone as the key is just compared by the address of the string literal, and not the value itself.

This check is disabled by default as there are valid use cases. But again, I would suggest always using it and adding a // clazy:exclude= comment.

4. fully-qualified-moc-types

Warns when a signal, slot or invokable declaration is not using fully-qualified type names, which will break old-style connects and interaction with QML.

Also warns if a Q_PROPERTY of type gadget is not fully-qualified (Enums and QObjects in Q_PROPERTY don’t need to be fully qualified).

Example:

namespace MyNameSpace {

    struct MyType { (...) };

    class MyObject : public QObject
    {
        Q_OBJECT
        Q_PROPERTY(MyGadget myprop READ myprop); // Wrong, needs namespace
    Q_SIGNALS:
        void mySignal(MyType); // Wrong
        void mySignal(MyNameSpace::MyType); // OK
    };
}

Beware that fixing these type names might break user code if they are connecting to them via old-style connects, since the users might have worked around your bug and not included the namespace in their connect statement.

5. connect-by-name

Warns when auto-connection slots are used. They’re also known as “connect by name“, an old and unpopular feature which isn’t recommended. Consult the official documentation for more information.

These types of connections are very brittle, as a simple object rename would break your code. In Qt 5 the pointer-to-member-function connect syntax is recommended as it catches errors at compile time.

6. empty-qstringliteral

Suggests to use an empty QString instead of an empty QStringLiteral. The later causes unneeded code bloat.

You should use QString() instead of QStringLiteral() and QString("") instead of QStringLiteral("").

7. qt-keywords (with fixit)

Warns when using Qt keywords such as emit, slots, signals or foreach.

This check is disabled by default. Using the above Qt keywords is fine unless you’re using 3rdparty headers that also define them, in which case you’ll want to use Q_EMIT, Q_SLOTS, Q_SIGNALS or Q_FOREACH instead.

This check is mainly useful due to its fixit to automatically convert the keywords to their Q_ variants. Once you’ve converted all usages, then simply enforce them via CONFIG += no_keywords (qmake) or ADD_DEFINITIONS(-DQT_NO_KEYWORDS) (CMake).

8. raw-environment-function

Warns when putenv() or qputenv() are being used and suggests the Qt thread-safe equivalents instead. This check is disabled by default and should be enabled manually if thread-safety is important for you.

9. qstring-varargs

This implements the equivalent of -Wnon-pod-varargs but only for QString.

This check is only useful in cases where you don’t want to enable -Wnon-pod-varargs. For example on projects with thousands of benign warnings (like with MFC’s CString), where you might only want to fix the QString cases.

10. static-pmf

Warns when storing a pointer to a QObject member function and passing it to a connect statement. Passing such variable to a connect() is known to fail at runtime when using MingW.

You can check if you’re affected by this problem with the following snippet:

static auto pmf = &QObject::destroyed;
if (pmf == &QObject::destroyed) // Should be false for MingW

Conclusion and thoughts for version 1.5

Clazy has matured and it’s getting increasingly difficult to come up with new ideas for checks. For version 1.5 I won’t be focusing in writing new warnings, but instead figuring out how to organize the existing ones.

This project has come a long way, there’s now 77 checks and I feel the current classification by false-positive probability (levels 0, 1, 2, 3) is not scaling anymore. I will try to organize them by categories (bug, performance, readability, containers, etc), which would be orthogonal to levels and hopefully also answer the following questions:

  • What’s the absolute sub-set of checks that every project should use ?
  • Which ones should abort the build if triggered (-Werror style) ?
  • How to make clazy useful in CI without getting in the way with annoying false-positives ?
  • How to let the user configure all this in an easy way ?

But of course, if you know of any interesting check that wouldn’t cost me many days of work please file a suggestion or catch me at #kde-clazy (freenode) and it might make it to the next release.

Categories: C++ / KDAB on Qt / Tooling

7 thoughts on “Clazy 1.4 released”

  1. Sorry, left this comment on an older post by accident. So now here where it actually fits.

    Clazy is a must-have in our company. Thanks for giving birth to such a useful tool. 🙂

    If you are looking for inspiration for new checks, maybe ask on the kde-i18n-doc mailing list.
    A few years ago, there were two or three things programmers got wrong that only broke translations. I think
    i18n(“foo %1”).arg(bar)
    was one of them. It had to be
    i18n(“foo %1”, bar)
    to work as expected. But well, I am away from i18n for some time and my knowledge is growing old. You might want to talk to Albert (aacid) about it.

    Cheers

  2. I always use QString() instead of QStringLiteral(“”). Never thought about using QString(“”) instead. What would be the advantage?

    1. Sérgio Martins

      It’s just a difference, rather than an advantage.

      QString().isNull() is true, while QStringLiteral(“”).isNull() is false. If you have a big codebase and are replacing QStringLiteral(“”) with QString() then you might be introducing bugs if somewhere someone is using isNull() instead of isEmpty().

      I would go directly to QString() if isNull() isn’t an issue.

      Furthermore, it seems even QStringLiteral().isNull() is false (unlike QString() and QLatin1String), so I better update this check’s readme.

  3. I’m having trouble building 1.4 on Ubuntu 16.04. I’ve tried building with clang 3.8 (default), clang 5.0 and clang 7.0 as well as gcc 5.4.

    I’m always getting an error on:

    clazy-1.4/src/SourceCompatibilityHelpers.h:52:12: error: no viable conversion from returned value
    of type ‘std::pair’ to function return type ‘clang::CharSourceRange’
    return sm.getImmediateExpansionRange(macroLoc);
    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

    1. Sérgio Martins

      Try clang 5.0 again, but make sure cmake says it’s using the headers from 5.0, and not from 3.8. They are not compatible. Also check which llvm-config you have in PATH.

Leave a Reply

Your email address will not be published. Required fields are marked *

This site uses Akismet to reduce spam. Learn how your comment data is processed.

By continuing to use the site, you agree to the use of cookies. More information

The cookie settings on this website are set to "allow cookies" to give you the best browsing experience possible. If you continue to use this website without changing your cookie settings or you click "Accept" below then you are consenting to this.

Close