Sign up for the KDAB Newsletter
Stay on top of the latest news, publications, events and more.
Go to Sign-up
In a previous blog post we introduced clazy, a clang plugin which makes the compiler understand Qt semantics, allowing you to get compile-time warnings about Qt best practices ranging from unneeded memory allocations to misuse of API, including fix-its for automatic refactoring.
Today we'll do a round-up and present the checks related to connect statements, Qt containers (including QString) and misc Qt facilities.
Finds connect()
statements still using the old SIGNAL()
/SLOT()
syntax. The Qt 5 pointer-to-member syntax allows you to catch errors at compile time rather than runtime. A fixit is included for automatically rewriting your connects to the new form.
Warns when connecting a non-signal to something.
connect(obj, &MyObj::mySlot, &MyObj::mySlot2);
The above doesn't make sense, but it compiles just fine, since it's valid C++.
Warns when a lambda inside a connect()
captures local variables by reference.
int a;
connect(obj, &MyObj::mySignal, [&a] { ... });
This usually results in a crash since the lambda might get called after the captured variable went out of scope.
For readability purposes you should always use emit
(or Q_EMIT
) when calling
a signal. Conversely, you should not use those macros when calling something other than a signal.
clazy will warn if you forget to use emit
(or Q_EMIT
) or if you use them when you shouldn't.
Finds temporary containers being created needlessly due to careless API misuse.
For example, it's common for developers to assume QHash::values()
and QHash::keys()
are free and abuse them, failing to realize their implementation internally actually iterates the whole container, allocates memory, and fills a new QList.
The good news is, fixing these inefficiencies is fun and usually all you have to do is delete code or call another function instead.
for (auto value : myMap.values()) // Bad
for (auto value : myMap) // Good, no temporary allocations
// Also no allocations, but iterating keys now
for (auto it = map.keyBegin(), end = map.keyEnd(); it != end; ++it)
int n = set.toList()[0]; // Bad
int n = set.constFirst(); // Good
if (hash.keys().contains(key)) // Bad
if (hash.contains(key)) // Good
int sz = hash.values().size(); // Bad
int sz = hash.size(); // Good
hash.values().contains(v); // Bad, use std::find() instead
mySets.intersect(other).isEmpty() // Bad
!mySets.intersects(other) // Good
Looks for QMap
or QHash
having key types that can be modified by external factors.
The key's value should never change, as it's needed for sorting or hashing, but with some types, such as non-owning smart pointers it might happen.
The key types that this check warns for are: QPointer
, QWeakPointer
, weak_ptr
and QPersistentModelIndex
.
Detects when using iterators from temporary containers in for loops.
// temporary list returned by function
QList<int> getList()
{
QList<int> list;
... add some items to list ...
return list;
}
for (auto it = getList().begin(); it != getList().end(); ++it)
{
...
}
This will crash because the end
iterator was returned from a different container object than the begin
iterator.
Warns when calling non-const member functions on temporary containers.
QList<SomeType> MyClass::getList()
{
// Qt uses implicit sharing, so the list's element aren't actually copied, unless...
return m_list;
}
void bad()
{
// Ouch, triggers deep-copy of all elements, use constFirst() instead.
SomeType t = myClass.getList().first();
}
In the example above, the container detaches, causing a needless deep-copy of all elements.
Suggests usage of reserve()
calls. Whenever you know how many elements a container will hold, you should reserve
space in order to avoid repeated memory allocations. One big allocation before appending items is much faster than multiple small ones and reduces memory fragmentation.
Warns where a qDeleteAll()
has a redundant values()
or keys()
call.
// Bad, implies a malloc() call and filling a temporary container
qDeleteAll(bananas.values());
// Good
qDeleteAll(bananas);
// Bad, implies a malloc() call and filling a temporary container
qDeleteAll(oranges.keys());
// Good
qDeleteAll(oranges.keyBegin(), oranges.keyBegin());
values()
and keys()
create a temporary QList
and allocate memory. They are usually not needed.
Catches usages of QList<T>
where sizeof(T) > sizeof(void*)
. Use QVector<T>
for these cases and read qlist-considered-harmful from Marc's blog if you're still using QList
.
The soft version will only warn for local lists that are not returned or passed to any function. Otherwise you would get many warnings that couldn't be fixed due to source and ABI compatibility constraints.
Detects usage of range-loop with non-const Qt containers (potential detach/deep-copy).
Also warns when you should pass by const-ref in case the type is big or has non-trivial constructors/destructors.
// Potential deep-copy
for (auto item : m_items)
// Good
for (auto item : qAsConst(m_items))
// Bad, should probably by passed by <em>const-ref</em>
for (auto v : getQVariants())
There's also a discontinued check called foreach, it's disabled by default as range-loop is encouraged.
Finds code that should be using QDateTime::currentDateTimeUTC()
to avoid expensive timezone code paths.
// Bad, QDateTime::currentDateTimeUtc().toTime_t() is faster
QDateTime::currentDateTime().toTime_t()
// Bad, QDateTime::currentDateTimeUtc() is faster
QDateTime::currentDateTime().toUTC()
Finds places using QFileInfo(filename).exists()
instead of the faster form QFileInfo::exists(filename)
.
Warns on inefficient usages of qgetenv()
which usually allocate memory. Prefer using qEnvironmentVariableIsSet()
, qEnvironmentVariableIsEmpty()
and qEnvironmentVariableIntValue()
instead.
Warns when you should be passing parameters by const-ref in order to avoid copying big types or to avoid calling non-trivial constructors/destructors.
Finds misuse of some Qt macros. The two supported cases are:
Q_OS_WINDOWS
instead of Q_OS_WIN
(The former doesn't exist).Q_OS_XXX
macro before including qglobal.h
.Finds places where an event is not correctly passed to QCoreApplication::postEvent()
.
QCoreApplication::postEvent()
expects an heap allocated event, not a stack allocated one, otherwise it might crash.
Unless you have a reason not to use Q_OBJECT
(like compilation time) you should use it.
Here's a non-exhaustive list of features depending on this macro:
QObject::inherits
qobject_cast
metaObject()->className()
Catches return false;
inside QObject::event()
and QObject::eventFilter()
re-implementations.
Instead, the base class method should be returned, so the event is correctly handled.
Finds places where qobject_cast(event->child())
is being used inside QObject::childEvent()
or equivalent (QObject::event()
and QObject::eventFilter()
).
qobject_cast
can fail because the child might not be totally constructed yet.
Suggests using Q_ENUM
instead of Q_ENUMS
.
Suggests usage of Q_GLOBAL_STATIC
whenever you have non-POD global static variables.
Finds Q_GLOBAL_STATIC
being used with trivial types. This is unnecessary and creates code bloat.
Catches non-ascii literals being passed to QLatin1String
. It isn't usually what you want, since your source files are probably in UTF-8.
Suggests using QString::at(0)
instead of QString::left(1)
.
The later form is more expensive, as it deep-copies the string. Just be sure the string isn't empty, otherwise at(0)
asserts.
Detects chained QString::arg()
calls which should be replaced by the multi-arg overload to save memory allocations.
QString("%1 %2").arg(a).arg(b); // Bad
QString("%1 %2").arg(a, b); // one less temporary heap allocation
Or you could concatenate with QStringBuilder
, if you prefer that style.
Finds unneeded memory allocations such as
str.toLower().contains("foo")
which should be rewritten as
str.contains("foo", Qt::CaseInsensitive)
to avoid the temporary QString
caused by toLower
.
Matches any of the following cases:
str.{toLower, toUpper}().{contains, compare, startsWith, endsWith}()
Suggests usage of QString::fooRef()
instead of QString::foo()
, to avoid temporary heap allocations.
str.mid(5).toInt(ok) // Bad
str.midRef(5).toInt(ok) // Good
Where mid
can be any of: mid
, left
, right
, and toInt
can be any of: compare
, contains
, count
, startsWith
, endsWith
, indexOf
, isEmpty
, isNull
, lastIndexOf
, length
, size
, to*
, trimmed
Warns when auto
is deduced to be QStringBuilder
instead of QString
, introducing crashes.
// Oops, path is not what you expect it to be
const auto path = "hello " + QString::fromLatin1("world");
qDebug() << path; // Crash if you're using QStringBuilder
That's all, thanks for reading! I promise the next blog post will be much shorter. I plan to blog every 5 new checks. If you have any questions about building or using clazy, just check the README or ask me directly on IRC (#kde-clazy on freenode).
Please comment with suggestions for new clazy checks, I'll implement the 5 I like the most for version 1.2.
Stay on top of the latest news, publications, events and more.
Go to Sign-up
Upgrade your applications from Qt 5 to Qt 6 with KDAB’s migration services. Get a free migration assessment and join a hands-on workshop to prepare your team for a successful transition!
Learn more
Learn Modern C++
Our hands-on Modern C++ training courses are designed to quickly familiarize newcomers with the language. They also update professional C++ developers on the latest changes in the language and standard library introduced in recent C++ editions.
Learn more
25 Comments
18 - Apr - 2017
Andy
Thanks Sérgio - this is great stuff.
I used to have it working with Qt Creator on macOS, but now I just get "error: unable to load plugin 'ClangLazy.dylib'" errors even though the path to that lib is in LD_LIBRARY_PATH and DYLD_LIBRARY_PATH. It also doesn't compile with the latest homebrew llvm (which is 4.0), so you need to install 3.9 explicitly.
Any chance clazy will be added as a tool in Qt Creator at some point? Would be more convenient!
20 - Apr - 2017
Sérgio Martins
On macOS you must make sure QtCreator is not using Apple-clang, and make sure ClangLazy.dylib is somewhere in your library path. Does it work through the command line ?
I've filed https://bugs.kde.org/show_bug.cgi?id=378994 for your llvm 4.0 build failure. For QtCreator support there's https://bugreports.qt.io/browse/QTCREATORBUG-15157, please vote for it!
24 - Apr - 2017
Andy
"make sure QtCreator is not using Apple-clang,"
This was it! I have clazy set up as a compiler in the prefs and it points to the script, but I needed to add an environment variable CLANGXX "//opt/llvm@3.9/bin/clang++" in Creator.
It would be nice if there were a way to have this set by default in the clazy script based on the llvm install it was built against. The way it is now, CLANGXX has to be set for every project individually. Or maybe use CLAZY_CLANGXX that can be set in the global environment and then checked in the script with a fallback to CLANGXX.
Voted! Thanks Sérgio!
19 - Apr - 2017
Nierob
That should not crash... Static assert is needed.
19 - Apr - 2017
Andrew Gunnerson
qDeleteAll(oranges.keyBegin(), oranges.keyBegin());
Should the second argument be
oranges.keyEnd()
?This is great stuff! I can't wait to give Clazy a try.
20 - Apr - 2017
Sérgio Martins
Indeed, thank you Andrew.
20 - Apr - 2017
Laszlo Nagy
Hey Sérgio,
very nice job! I'm wondering, would you write a short intro about your tools on the Clang external examples page? And some checkers (like temporary iterator) might be interesting for wilder audience. Do you have plan to move that into Clang-tidy?
Regards, Laszlo
20 - Apr - 2017
Kevin Funk
We've already added an entry to the external examples page, for what it's worth: https://clang.llvm.org/docs/ExternalClangExamples.html
We're considering making it possible to use clazy from clang-tidy, yes. But I'll let Sergio talk about this.
20 - Apr - 2017
Sérgio Martins
Probably not moving into clang-tidy (too big effort), but finding some way for clang-tidy to use clazy's checks seems like a good idea.
20 - Apr - 2017
CarelC
Are there any plans to integrate this with the clang code model available in QtCreator?
20 - Apr - 2017
Sérgio Martins
Let's discuss here: https://bugreports.qt.io/browse/QTCREATORBUG-15157
20 - Apr - 2017
Henry Miller
We are still in the middle of our qt4-qt5 port, right now we support both from one code base. That means at least some of the checks do not apply (we cannot use the new style connects for example). Is it easy to disable those checks?
20 - Apr - 2017
Kevin Funk
Yes, very easy. See: https://github.com/KDE/clazy#selecting-which-checks-to-enable
For example:
20 - Apr - 2017
Markus
Thanks for the very informative article!
Trying to get the prebuilt binaries to work, I get: msvcprt.lib(MSVCP140.dll) : fatal error LNK1112: module machine type 'X86' conflicts with target machine type 'x64' Qt5Gui.lib(Qt5Gui.dll) : fatal error LNK1112: module machine type 'X86' conflicts with target machine type 'x64'
I assume that this means that the prebuilt msvc2015 binaries are requiring that the project is built with a 64 bit version of Qt?
If so, does anybody have 32 bit binaries for clazy on windows available?
20 - Apr - 2017
Sérgio Martins
I'm not on Windows right now to check, but I assume how the compiler was built should not matter. Can you try passing /MACHINE:X86 explicitly ?
Anyhow, I'll have to play with it next time I get a chance. I've added https://bugs.kde.org/show_bug.cgi?id=378999 to track this.
20 - Apr - 2017
Markus
Where should I pass /MACHINE:X86?
According to the instructions, I just use the following calls for building:
set PATH=C:\Qt\clazy_v11\bin;%PATH% qmake my_proj_file.pro spec=win32-msvc2015 QMAKE_CXX="clazy-cl.bat" nmake
20 - Apr - 2017
Sergio Martins
I've just checked, and this works:
Open clazy-cl.bat, and shove a -m32 somewhere. Should look like this: @echo off %~dp0\clang\clang.exe --driver-mode=cl -m32 -Qunused-arguments -Xclang -load -Xclang ClangLazy.dll -Xclang -add-plugin -Xclang clang-lazy -Wno-microsoft-enum-value %*
21 - Apr - 2017
Markus
I can confirm that this works. Big thanks for the solution!
24 - Apr - 2017
Kuba Ober
Re lambda-in-connect: This is valid when
obj
is a local object and doesn't outlive the locals captured by reference.24 - Apr - 2017
Sérgio Martins
Yep, if QObject was stack allocated clazy won't warn.
24 - Apr - 2017
Serhii
Thanks, it's great tool! The only thing is that I can't supress warnings from 3rd party code (I use QuickFlux in my project). I added QMAKE_CXXFLAGS += -isystem $$PWD/../quickflux (it's relative path to the QuickFlux sources) to the .pro file to include it with -isystem instead of -I, but it doesn't help, there are still warning from it.
22 - Jan - 2018
William
Hi, I have Fedora 27 x86_64 with clang-5.0.0 and llvm4.0-4.0.1 provided by Fedora. cmake for clazy runs cleanly, but make fails on the first file with errors like the ones below. Is clazy supposed to build with clang 5 and llvm 4? If I will have to build clang and llvm from source, what are the best versions to use with the current clazy git? Thanks, William
[ 1%] Building CXX object CMakeFiles/ClangLazy.dir/src/checks/level0/qcolor-from-literal.cpp.o In file included from /usr/include/clang/AST/DeclCXX.h:19:0, from /u/clazy/src/Utils.h:31, from /u/clazy/src/StringUtils.h:29, from /u/clazy/src/checks/level0/qcolor-from-literal.cpp:22: /usr/include/clang/AST/ASTContext.h: In member function 'llvm::ArrayRef clang::ASTContext::getModulesWithMergedDefinition(const clang::NamedDecl)': /usr/include/clang/AST/ASTContext.h:939:46: error: invalid conversion from 'const clang::NamedDecl' to 'clang::NamedDecl*' [-fpermissive] auto MergedIt = MergedDefModules.find(Def);
/usr/include/clang/Frontend/CodeGenOptions.def:32:1: error: 'DebugCompressionType' in namespace 'llvm' does not name a type ENUM_CODEGENOPT(CompressDebugSections, llvm::DebugCompressionType, 2,
22 - Jan - 2018
Sérgio Martins
Hi William,
clazy needs at least clang/llvm 3.8. The problem is that your clang headers don't match the LLVM ones. Both LLVM and Clang must be the same version. Please double-check you didn't make any mistake installing the Fedora packages.
23 - Jan - 2018
William
Thanks!!! That was the problem. I removed all of the llvm4.0 packages, and then cmake found the llvm 5 packages that matched clang, and clazy built cleanly. I ran the default install into /usr/local, then moved the executables to /usr/bin so they could find Fedora's clang files. I ran it with find . -name '*.cpp' -print0 | xargs -0 clazy-standalone -checks=level2 -p build/compile_commands.json and now I have a lot of diagnostics to look at. It core dumped on a Qt-generated mocs_compilation.cpp file. Is there a place to file a report?
23 - Jan - 2018
Sérgio Martins
Please report here : https://bugs.kde.org/enter_bug.cgi?product=clazy with a backtrace, or attach your moc file if you think I can reproduce without needing extra code, thanks