Explanation for checker connect-by-name (level0)
connect-by-name
Warns when "auto-connection slots" are used. They're also known as "connect by name", a very old and unpopular feature which shouldn't be used anymore. See http://doc.qt.io/qt-5/qobject.html#auto-connection for more information about them.
These types of connections are very brittle, as a simple object rename would break your code. In Qt 5 the PMF connect syntax is recommended as it catches errors at compile time.
This check simply warns for any slot named like on_*_*, because even if you're not using .ui files this naming is misleading and not good for readability, as the reader would think you're using auto-connection.
Explanation for checker connect-non-signal (level0)connect-non-signal
Warns when connecting a non-signal to something.
For example:
connect(obj, &MyObj::mySlot, ...);
Only works with the new Qt5 connect syntax (PMF).
Explanation for checker connect-not-normalized (level0)connect-not-normalized
Warns when the contents of SIGNAL(), SLOT(), Q_ARG() and Q_RETURN_ARG() are not normalized.
Using normalized signatures allows to avoid unneeded memory allocations.
For signals and slots it only warns for connect statements, not disconnect, since it only
impacts the performance of the former.
See QMetaObject::normalizedSignature() for more information.
container-anti-pattern
Finds when temporary containers are being created needlessly. These cases are usually easy to fix by using iterators, avoiding memory allocations.
Matches code like:
{QMap, QHash, QSet}.values().*
{QMap, QHash}.keys().*
{QVector, QSet}.toList().*
QList::toVector().*
QSet::intersect(other).isEmpty()
for (auto i : {QHash, QMap}.values()) {}
foreach (auto i, {QHash, QMap}.values()) {}
Example
set.toList()[0]; // use set.constFirst() instead
hash.values().size(); // Use hash.size() instead
hash.keys().contains(); // Use hash.contains() instead
hash.values().contains(); // Use std::find(hash.cbegin(), hash.cend(), myValue) instead
map.values(k).foo ; // Use QMap::equal_range(k) instead
for (auto i : hash.values()) {} // Iterate the hash directly instead: for (auto i : hash) {}
QSet::intersect(other).isEmpty() // Use QSet::intersects() instead, avoiding memory allocations and iterations, since Qt 5.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 use QString("") instead of QStringLiteral("").
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
Explanation for checker lambda-in-connect (level0)lambda-in-connect
Warns when a lambda inside a connect captures local variables by reference. This usually results in a crash since the lambda might get called after the captured variable went out of scope.
Example:
int a;
connect(obj, &MyObj::mySignal, [&a]{ ... });
Although it's dangerous to capture by reference in other situations too, this check only warns for
connects, otherwise it would generate false-positives in legitimate situations where you only
use the lambda before going out of scope.
lambda-unique-connection
Finds usages of Qt::UniqueConnection when the slot is a functor, lambda or non-member function.
That connect() overload does not support Qt::UniqueConnection.
mutable-container-key
Looks for QMap or QHash having key types which can be modified due to 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 supported key types are: QPointer, QWeakPointer, weak_ptr and QPersistentModelIndex.
qcolor-from-literal
Warns when a QColor is being constructed from a string literal such as "#RRGGBB".
This is less performant than calling the ctor that takes ints, since it creates temporary QStrings.
Example:
QColor c("#000000"); // Use QColor c(0, 0, 0) instead
c.setNamedColor("#001122"); // Use c = QColor(0, 0x11, 0x22) instead
qdatetime-utc
Finds calls to QDateTime::currentDateTime() which should be replaced by
QDateTime::currentDateTimeUTC() in order to avoid expensive timezone code paths.
The two supported cases are: - QDateTime::currentDateTime().toTime_t() -> QDateTime::currentDateTimeUtc().toTime_t() - QDateTime::currentDateTime().toUTC() -> QDateTime::currentDateTimeUtc()
Explanation for checker qenums (level0)qenums
Warns when you're using Q_ENUMS. Use Q_ENUM instead.
qfileinfo-exists
Finds places using QFileInfo("filename").exists() instead of the faster version QFileInfo::exists("filename").
According to Qt's docs: "Using this function is faster than using QFileInfo(file).exists() for file system access."
Explanation for checker qgetenv (level0)qgetenv
Warns on innefficient usages of qgetenv() which usually allocate memory.
Suggests usage of qEnvironmentVariableIsSet(), qEnvironmentVariableIsEmpty() and qEnvironmentVariableIntValue().
These replacements are available since Qt 5.5.
Explanation for checker qmap-with-pointer-key (level0)qmap-with-pointer-key
Finds cases where you're using QMap<K,T> and K is a pointer.
QMap has the particularity of sorting it's keys, but sorting by memory
address makes no sense.
Use QHash instead, which provides faster lookups.
qstring-arg
Implements two warnings:
Detects when you're using chained
QString::arg()calls and should instead use the multi-arg overload to save memory allocationsQString("%1 %2").arg(a).arg(b); QString("%1 %2").arg(a, b); // one less temporary heap allocationDetects when you're using misleading
QString::arg()overloadsQString arg(qlonglong a, int fieldwidth = 0, int base = 10, QChar fillChar = QLatin1Char(' ')) const QString arg(qulonglong a, int fieldwidth = 0, int base = 10, QChar fillChar = QLatin1Char(' ')) const QString arg(long a, int fieldwidth = 0, int base=10, QChar fillChar = QLatin1Char(' ')) const QString arg(ulong a, int fieldwidth = 0, int base=10, QChar fillChar = QLatin1Char(' ')) const QString arg(int a, int fieldWidth = 0, int base = 10, QChar fillChar = QLatin1Char(' ')) const QString arg(uint a, int fieldWidth = 0, int base = 10, QChar fillChar = QLatin1Char(' ')) const QString arg(short a, int fieldWidth = 0, int base = 10, QChar fillChar = QLatin1Char(' ')) const QString arg(ushort a, int fieldWidth = 0, int base = 10, QChar fillChar = QLatin1Char(' ')) const QString arg(double a, int fieldWidth = 0, char fmt = 'g', int prec = -1, QChar fillChar = QLatin1Char(' ')) const QString arg(char a, int fieldWidth = 0, QChar fillChar = QLatin1Char(' ')) const QString arg(QChar a, int fieldWidth = 0, QChar fillChar = QLatin1Char(' ')) const QString arg(const QString &a, int fieldWidth = 0, QChar fillChar = QLatin1Char(' ')) const
because they are commonly misused, for example:
int hours = ...;
int minutes = ...;
// This won't do what you think it would at first glance.
QString s("The time is %1:%2").arg(hours, minutes);
To reduce false positives, some cases won't be warned about:
str.arg(hours, 2); // User explicitly used a integer literal, it's probably fine
str.arg(foo); // We're only after cases where the second argument (or further) is specified, so this is safe
str.arg(foo, width); // Second argument is named width, or contains the name "width", it's safe. Same for third argument and "base".
Using these misleading overloads is perfectly valid, so only warning (1) is enabled by default.
To enable warning (2), export CLAZY_EXTRA_OPTIONS="qstring-arg-fillChar-overloads"
qstring-insensitive-allocation
Finds unneeded memory allocations such as
if (str.toLower().contains("foo")) which you should fix as
if (str.contains("foo", Qt::CaseInsensitive)) to avoid the heap allocation caused by toLower().
Matches any of the following cases:
str.{toLower, toUpper}().{contains, compare, startsWith, endsWith}()
Pitfalls
Qt::CaseInsensitive is different from QString::toLower() comparison for a few code points, but it
should be very rare: http://lists.qt-project.org/pipermail/development/2016-February/024776.html
qstring-ref
Finds places where QString::fooRef() should be used instead of QString::foo(), to avoid temporary heap allocations.
Example
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
FixIts
Fixing the above cases can be automated with:
export CLAZY_FIXIT="fix-missing-qstringref"
qt-macros
Finds misusages of some Qt macros.
The two cases are:
- Using Q_OS_WINDOWS instead of Q_OS_WIN (The former doesn't exist).
- Testing a Q_OS_XXX macro before including qglobal.h
qvariant-template-instantiation
Detects when you're using QVariant::value<Foo>() instead of QVariant::toFoo().
The former results in more code being generated.
Explanation for checker strict-iterators (level0)strict-iterators
Warns when iterator objects are implicitly cast to const_iterator.
This is mostly equivalent to passing -DQT_STRICT_ITERATORS to the compiler.
This prevents detachments but also caches subtle bugs such as:
QHash<int, int> wrong;
if (wrong.find(1) == wrong.cend()) {
qDebug() << "Not found";
} else {
qDebug() << "Found"; // find() detached the container before cend() was called, so it prints "Found"
}
QHash<int, int> right;
if (right.constFind(1) == right.cend()) {
qDebug() << "Not found"; // This is correct now !
} else {
qDebug() << "Found";
}
temporary-iterator
Detects when you're using using functions returning iterators (eg. begin() or end()) on a temporary container.
Example
// temporary list returned by function
QList<type> getList()
{
QList<type> list;
... add some items to list ...
return list;
}
// Will cause a crash if iterated using:
for (QList<type>::iterator it = getList().begin(); it != getList().end(); ++it)
{
...
}
because the end iterator was returned from a different container object than the begin iterator.
Explanation for checker unused-non-trivial-variable (level0)unused-non-trivial-variable
Warns about unused Qt value classes. Compilers usually only warn when trivial classes are unused and don't emit warnings for non-trivial classes.
This check has a whitelist of common Qt classes such as containers, QFont, QUrl, etc and warns for those too.
See UnusedNonTrivialType::isInterestingType(QualType t) for a list of all types.
It's possible to extend the whitelist with user types, by setting the env variable CLAZY_UNUSED_NON_TRIVIAL_VARIABLE_WHITELIST.
It accepts a comma separate name of types.
It's possible to disable the whitelist via exporting CLAZY_EXTRA_OPTIONS=unused-non-trivial-variable-no-whitelist,
when this env variable is set clazy will warn for any unused non-trivial type. This will create many false positives,
such as RAII classes, but still useful to run at least once on your codebase. When disabling the whitelist this way it's also possible
to black list types, by setting a comma separated list of types to CLAZY_UNUSED_NON_TRIVIAL_VARIABLE_BLACKLIST
writing-to-temporary
Catches when calling setters on temporaries.
Example
widget->sizePolicy().setHorizontalStretch(1);
Which should instead be:
QSizePolicy sp = widget->sizePolicy();
sp.setHorizontalStretch(1);
widget->setSizePolicy(sp);
Requirements
The method must be of void return type, and must belong to one of these whitelisted classes: QList, QVector, QMap, QHash, QString, QSet, QByteArray, QUrl, QVarLengthArray, QLinkedList, QRect, QRectF, QBitmap, QVector2D, QVector3D, QVector4D, QSize, QSizeF, QSizePolicy
Optionally, if you set env variable
CLAZY_EXTRA_OPTIONS="writing-to-temporary-widen-criteria", it will warn on any method with name starting with "set", regardless of it being whitelisted, example:foo().setBar()
wrong-qevent-cast
Warns when a QEvent is possibly cast to the wrong derived class via static_cast.
Example:
switch (ev->type()) {
case QEvent::MouseMove:
auto e = static_cast
Currently only casts inside switches are verified.
Explanation for checker wrong-qglobalstatic (level0)wrong-qglobalstatic
Finds Q_GLOBAL_STATICs being used with trivial types.
This is unnecessary and creates code bloat.
Example:
struct Trivial
{
int v;
};
Q_GLOBAL_STATIC(Trivial, t); // Wrong
static Trivial t; // Correct
auto-unexpected-qstringbuilder
Finds places where auto is deduced to be QStringBuilder instead of QString, which introduces crashes.
Also warns for lambdas returning QStringBuilder.
Example
#define QT_USE_QSTRINGBUILDER
#include <QtCore/QString>
(...)
const auto path = "hello " + QString::fromLatin1("world");
qDebug() << path; // CRASH
Fixits
export CLAZY_FIXIT="fix-auto-unexpected-qstringbuilder"
child-event-qobject-cast
Finds places where qobject_cast<MyType>(event->child()) is being used inside QObject::childEvent() or equivalent (QObject::event() or QObject::eventFilter()).
qobject_cast can fail because the child might not be totally constructed yet.
connect-3arg-lambda
Warns when using the 3-arg QObject::connect that takes a lambda.
The recommendation is to use the 4-arg overload, which takes a context object
so that the lambda isn't executed when the context object is deleted.
It's very common to use lambdas to connect signals to slots with different number of arguments. This can result in a crash if the signal is emitted after the receiver is deleted.
Another reason for using a context-object is so you explicitly think about in
which thread you want the slot to run in. Note that with a context-object the
connection will be of type Qt::AutoConnection instead of Qt::DirectConnection,
which you can control if needed, via the 5th (optional) argument.
In order to reduce false-positives, it will only warn if the lambda body dereferences at least one QObject (other than the sender).
It's very hard to not have any false-positives. If you find any you probably can just pass the sender again, as 3rd parameter.
This will also warn if you pass a lambda to QTimer::singleShot() without
using the overload that takes a context object.
const-signal-or-slot
Warns when a signal or non-void slot is const.
This aims to prevent unintentionally marking a getter as slot, or connecting to the wrong method.
For signals it's more of a minor issue. Prevents you from emitting signals from const methods, as these methods shouldn't change state, and a signal implies state was changed. Helps minimizing having global state (which is the only state you can change from a const method).
Warns for the following cases:
- non-void const method marked as slot
- const method marked as signal
- connecting to a method which isn't marked as slot, is const and returns non-void
For exposing methods to QML prefer either Q_PROPERTY or Q_INVOKABLE.
Explanation for checker detaching-temporary (level1)detaching-temporary
Finds places where you're calling non-const member functions on temporaries.
For example getList().first(), which would detach if the container is shared.
There can be some false-positives, for example someHash.values().first() because refcount is 1.
But constFirst() is a good default, so you should try to use it wherever you can, since it's not practical to inspect all code and figure out if the container is shared or not.
foreach
- Finds places where you're detaching the
foreachcontainer. - Finds places where big or non-trivial types are passed by value instead of const-ref.
- Finds places where you're using
foreachon STL containers. It causes deep-copy. Use C++11 range-loop instead.
Note: range-loop is prefered over foreach since the compiler generates less and more optimized code.
Use range-loop if your container is const, otherwise a detach will happen.
This check is disabled for Qt >= 5.9
Explanation for checker incorrect-emit (level1)incorrect-emit
For readability purposes you should always use emit (or Q_EMIT) when calling a signal. Conversely, you should not use those macros when calling a non-signal.
clazy will warn if you forget to use emit (or Q_EMIT) or if you use them on a non-signal.
Additionally, it will warn when emitting a signal from a constructor, because there's nothing connected to the signal yet.
Explanation for checker inefficient-qlist-soft (level1)inefficient-qlist-soft
A less aggressive version of the inefficient-qlist check.
Finds QList<T> where sizeof(T) > sizeof(void*). QVector<T> should be used instead.
Only warns if the container is a local variable and isn't passed to any method or returned,
unlike inefficient-qlist. This makes it easier to fix the warnings without concern about source and binary compatibility.
install-event-filter
Warns on potential misuse of QObject::installEventFilter().
To install an event filter you should call monitoredObject->installEventFilter(this), but sometimes
you'll write installEventFilter(filterObject) by mistake, which compiles fine.
In rare cases you might actually want to install the event filter on this, in which case this is a false-positive.
non-pod-global-static
Warns about non-POD [1] global statics. CTORS from globals are run before main, on library load, slowing down startup. This is more a problem for libraries, since usually the app won't use every feature the library provides, so it's a waste of resources to initialize CTORs from unused features.
It's tolerated to have global statics in executables, however, clazy doesn't know if it's compiling an executable or a library, so it's your job to run this check only on libraries. It doesn't harm, though, to also remove global statics from executables, because they're usually evil.
The same goes for DTORs at library unload time. A good way to fix them is by using Q_GLOBAL_STATIC.
[1] The term "POD" is too strict. The correct term is "types with a trivial dtor and trivial ctor", and that's how this check is implemented.
Explanation for checker overridden-signal (level1)overridden-signal
Warns when overriding a signal, which might make existing connects not work, if done unintentionally. Doesn't warn when the overridden signal has a different signature.
Warns for: - Overriding signal with non-signal - Overriding non-signal with signal - Overriding signal with signal
Explanation for checker post-event (level1)post-event
Finds places where an event is not correctly passed to QCoreApplication::postEvent().
QCoreApplication::postEvent() expects an heap allocated event, not a stack allocated one.
QCoreApplication::sendEvent() correctness is not checked due to false-positives.
qdeleteall
Finds places where a call to qDeleteAll() has a redundant values() or keys() call.
Those calls create a temporary QList<int> and allocate memory.
Example
QSet<Cookies> set;
// BAD: Unneeded container iteration and memory allocation to construct list of values
qDeleteAll(set.values());
// GOOD: Unneeded container iteration and memory allocation to construct list of values
qDeleteAll(set);
Pitfalls
Very rarely you might be deleting a list of QObjects who's destroyed() signal is connected to some code
that modifies the original container. In the case of this contrived example iterating over the container copy is safer.
qhash-namespace
Warns when a qHash() function is not declared inside the namespace of it's argument.
qHash() needs to be inside the namespace for ADL lookup to happen.
qlatin1string-non-ascii
Finds places where you're using QLatin1String with a non-ascii literal like QLatin1String("é").
This is almost always a mistake, since source files are usually in UTF-8.
qproperty-without-notify
Warns when a non-CONSTANT Q_PROPERTY is missing a NOTIFY signal.
Objects used in QML (e.g. Qt Quick or Declarative Widgets) need to notify when a property changes. This is also useful when viewing QObject properties in Gammaray.
Explanation for checker qstring-left (level1)qstring-left
Finds places where you're using QString::left(1) instead of QString::at(0).
The later form is cheaper, as it doesn't deep-copy the string.
There's however another difference between the two: left(1) will return an empty
string if the string is empty, while QString::at(0) will assert. So be sure
that the string can't be empty, or add a if (!str.isEmpty() guard, which is still
faster than calling left() for the cases which deep-copy.
range-loop
Finds places where you're using C++11 range-loops with non-const Qt containers (potential detach).
Fix it by marking the container const, or, since Qt 5.7, use qAsConst():
Example
for (auto i : qAsConst(list)) { ... }
Also warns if you're passing structs with non-trivial copy-ctor or non-trivial dtor by value, use const-ref so that the copy-ctor and dtor don't get called.
Explanation for checker returning-data-from-temporary (level1)returning-data-from-temporary
Warns when returning the data from a QByteArray that will soon be destroyed.
Examples
QByteArray b = ...;
return b.data();
return funcReturningByteArray().data();
return funcReturningByteArray().constData();
const char * getFoo()
{
QByteArray b = ...;
return b; // QByteArray can implicitly cast to char*
}
const char *c1 = getByteArray();
const char *c2 = str.toUtf8().data();
Note that in some cases it might be fine, since the method can return the data of a global static QByteArray. However such code is brittle, it could start crashing if it ceased to be static.
Explanation for checker rule-of-two-soft (level1)rule-of-two-soft
Finds places where: 1. You're calling a trivial copy-ctor of a class which has a non-trivial copy-assignment operator 2. You're calling a trivial copy-assignment operator of a class which has a non-trivial copy-ctor
It won't warn on classes that violate the rule of two unless you actually use the copy-ctor or the copy-assignment operator, otherwise it will generate lots of warnings. If you're really interested in all warnings, see the rule-of-three check instead of rule-of-two-soft.
Beware that removing copy-ctors or copy-assignment operators might make the class trivially copiable, which is not ABI compatible.
Explanation for checker skipped-base-method (level1)skipped-base-method
Warns when calling a method from the "grand-base class" instead of the base-class method.
Example: class MyFrame : public QFrame { Q_OBJECT public: bool event(QEvent *ev) override { (...) return QWidget::event(ev); // warning: Maybe you meant to call QFrame::changeEvent() instead [-Wclazy-skipped-base-method] } };
Try to avoid jumping over the direct base method. If you really need to then at least add a comment in the code, so people know it was intentional. Or even better, an clazy:exclude=skipped-base-method comment, which also sliences this warning.
Explanation for checker virtual-signal (level1)virtual-signal
Warns when a signal is virtual. Virtual signals make it very hard to read connect statements since people don't know they are virtual, and don't expect them to be.
moc also discourages the use of virtual signals, by printing a non-fatal warning:
Warning: Signals cannot be declared virtual
base-class-event
Warns when you return false inside your QObject::event() or QObject::eventFilter() reimplementation.
Instead you should probably call the base class method, so the event is correctly handled.
copyable-polymorphic
Finds polymorphic classes that are copyable. These classes are usually vulnerable to slicing [1].
To fix these warnings use Q_DISABLE_COPY or delete the copy-ctor yourself.
[1] https://en.wikipedia.org/wiki/Object_slicing
Explanation for checker ctor-missing-parent-argument (level2)ctor-missing-parent-argument
Warns when QObject derived classes don't have at least one CTOR receiving a QObject.
This is an attempt to catch classes which are missing the parent argument. It doesn't have false-positives, but might miss true-positives. For example, if you have a CTOR which receives a `QObject* but then forget to pass it to the base CTOR.
Explanation for checker function-args-by-ref (level2)function-args-by-ref
Warns when you should be passing by const-ref. Types with sizeof > 16 bytes [1] or types which are not trivially-copyable [2] or not trivially-destructible [3] should be passed by ref. A rule of thumb is that if passing by value would trigger copy-ctor and/or dtor then pass by ref instead.
This check will ignore shared pointers, you're on your own. Most of the times passing shared pointers by const-ref is the best thing to do, but occasionally that will lead to crashes if you're in a method that calls something else that makes the shared pointer ref count go down to zero.
- [1] http://www.macieira.org/blog/2012/02/the-value-of-passing-by-value/
- [2] http://en.cppreference.com/w/cpp/concept/TriviallyCopyable
- [3] http://www.cplusplus.com/reference/type_traits/is_trivially_destructible/
function-args-by-value
Warns when you should be passing by value instead of by-ref. Types with sizeof <= 16 bytes [1] which are trivially-copyable [2] and trivially-destructible [3] should be passed by value.
Only fix these warnings if you're sure that the value would be passed in a CPU register instead on the stack.
- [1] http://www.macieira.org/blog/2012/02/the-value-of-passing-by-value/
- [2] http://en.cppreference.com/w/cpp/concept/TriviallyCopyable
- [3] http://www.cplusplus.com/reference/type_traits/is_trivially_destructible/
global-const-char-pointer
Finds where you're using const char *foo instead of const char *const foo or const char []foo.
The former case adds a pointer in .data, pointing to .rodata. The later cases only use .rodata.
implicit-casts
Finds places with unwanted implicit casts in function calls.
Supported cases
pointer->bool cast in functions accepting bool and pointers, example:
MyWidget(bool b, QObject *parent = nullptr) {} MyWidget(parent);bool->int
void func(int duration); func(someBool);
This last case is disabled due to false positives when calling C code.
You can enable it by with:
export CLAZY_EXTRA_OPTIONS=implicit-casts-bool-to-int
missing-qobject-macro
Finds QObject derived classes that don't have a Q_OBJECT macro.
Reasons to use Q_OBJECT
- Signals and slots
QObject::inheritsqobject_castmetaObject()->className()- Use your custom widget as a selector in Qt stylesheets
Reasons not to use Q_OBJECT
- Templated QObjects
- Compilation time
This check can't be used with pre-compiled headers support. This check doesn't have false positives, but it's not included in level <= 1 because the missing Q_OBJECT might be intentional.
Explanation for checker missing-typeinfo (level2)missing-typeinfo
Suggests usage of Q_PRIMITIVE_TYPE or Q_MOVABLE_TYPE in cases where you're using QList<T> and sizeof(T) > sizeof(void*)
or using QVector<T>, unless they already have a type info classification.
See Q_DECLARE_TYPEINFO in Qt documentation for more information.
old-style-connect
Finds usages of old style connects.
Connecting with old style syntax (SIGNAL/SLOT) is much slower than using pointer to member syntax (PMF).
Here's however a non-exhaustive list of caveats you should be aware of: - You can't disconnect with new-syntax if the connect was made with old-syntax (and vice-versa) - You can't disconnect from a static slot with new-syntax (although connecting works) - Difference in behaviour when calling slots of partially destroyed objects (https://codereview.qt-project.org/#/c/83800)
Fixits
You can convert the most simple cases with export CLAZY_FIXIT=fix-old-style-connect.
Be careful, as PMF is not a 100% drop-in replacement.
Pitfalls
Although this check doesn't have false-positives it's a level2 check, that's because some connects are tricky to convert to PMF syntax and might introduce bugs if you don't know what you're doing.
Explanation for checker qstring-allocations (level2)qstring-unneeded-heap-allocations
Finds places with unneeded memory allocations due to temporary QStrings.
Here's a summary of usages that allocate:
QString s = "foo"; // Allocates, use QStringLiteral("foo") insteadQString s = QLatin1String("foo"); // Allocates, use QStringLiteral("foo") instead2.1
QString s = QLatin1String(""); // No allocation. QString is optimized for this case, so it's safe for empty literalsQString s = QStringLiteral("foo"); // No allocationQString s = QString::fromLatin1("foo"); // Allocates, use QStringLiteralQString s = QString::fromUtf8("foo"); // Allocates, use QStringLiterals == "foo" // Allocates, use QLatin1Strings == QLatin1String("foo) // No allocations == QStringLiteral("foo") // No allocationQString {"append", "compare", "endsWith", "startsWith", "indexOf", "insert","lastIndexOf", "prepend", "replace", "contains" } // They all have QLatin1String overloads, so passing a QLatin1String is ok.QString::fromLatin1("foo %1").arg(bar) // Allocates twice, replacing with QStringLiteral makes it allocate only once.
Fixits
fix-qlatin1string-allocations // To replace QLatin1String with QStringLiteral only where it was allocating before
fix-fromLatin1_fromUtf8-allocations // To replace fromLatin1() and fromUtf8() so it doesn't allocate
fix-fromCharPtrAllocations // To replace raw string literals so it doesn't allocate
Example:
export CLAZY_FIXIT="fix-fromCharPtrAllocations"
Pitfalls
QStringLiteralmight make your app crash at exit if plugins are involved. See: https://blogs.kde.org/2015/11/05/qregexp-qstringliteral-crash-exit and http://lists.qt-project.org/pipermail/development/2015-November/023681.htmlAlso note that MSVC crashes when
QStringLiteralis used inside initializer lists. For that reason no warning or fixit is emitted for this case unless you set an env variable:export CLAZY_EXTRA_OPTIONS="qstring-allocations-no-msvc-compat"
returning-void-expression
Warns when returning a void expression.
Example: ``` void doStuff() { if (cond) return // Oops, forgot the ; but it still compiles since processStuff() returns void.
processStuff();
} ```
Explanation for checker rule-of-three (level2)rule-of-three
Implements the rule of three: https://en.wikipedia.org/wiki/Rule_of_three_%28C%2B%2B_programming%29
Exceptions
To reduce the amount of warnings, these cases won't emit warnings: - class has a QSharedDataPointer member - class inherits from QSharedData - if only the dtor is implemented and it's protected - class name ends with "Private" and is defined in a .cpp, .cxx or _p.h file
In some cases you're missing methods, in others you have too many methods. You'll have to judge what's the correct fix and beware of binary compatibility.
Explanation for checker static-pmf (level2)static-pmf
Warns when storing a pointer to QObject member function into a static variable. Passing such variable to a connect is known to fail when using MingW.
Example:
static auto pmf = &QObject::destroyed; QCOMPARE(pmf, &QObject::destroyed); // fails
Explanation for checker virtual-call-ctor (level2)virtual-call-ctor
Finds places where you're calling pure virtual functions inside a constructor or destructor. Compilers usually warn about this if there isn't any indirection, this check will catch cases like calling a non-pure virtual that calls a pure virtual.
This check only looks for pure virtuals, ignoring non-pure, which in theory you shouldn't call, but seems common practice.
Explanation for checker assert-with-side-effects (level3)assert-with-side-effects
Tries to find Q_ASSERTs with side-effects. Asserts are compiled-out in release mode so you shouldn't put any important code inside them.
Example
// The connect statement wouldn't run in release mode
Q_ASSERT(connect(buttonm, &QPushButton::clicked, this, &MainWindow::handleClick));
Pitfalls
As this is a level3 check, it will have many false positives and might be buggy. Patches accepted!
Explanation for checker detaching-member (level3)detaching-member
Finds places where member containers are potentially detached.
Example
QString MyClass::myMethod()
{
return m_container.first(); // Should be constFirst()
}
Pitfalls
This check is disabled by default as it reports too many false positives for now.
Explanation for checker reserve-candidates (level3)reserve-candidates
Finds places that could use a reserve() call.
Whenever you know how many elements a container will hold you should reserve
space in order to avoid repeated memory allocations.
Trivial example missing reserve()
QList<int> ages;
// list.reserve(people.size());
for (auto person : people)
list << person.age();
Example where reserve shouldn't be used:
QLost<int> list;
for (int i = 0; i < 1000; ++i) {
// reserve() will be called 1000 times, meaning 1000 allocations
// whilst without a reserve the internal exponential growth algorithm would do a better job
list.reserve(list.size() + 2);
for (int j = 0; j < 2; ++j) {
list << m;
}
}
Supported containers
QVector, std::vector, QList, QSet and QVarLengthArray
Pitfalls
Rate of false-positives is around 15%. Don't go blindly calling reserve() without proper analysis.
In doubt don't use it, all containers have a growth curve and usually only do log(N) allocations
when you append N items.
thread-with-slots
slots in a QThread derived class are usually a code smell, because
they'll run in the thread where the QThread QObject lives and not in
the thread itself.
Disabled by default since it's very hard to avoid for false-positives. You'll have to explicitly enable it and check case by case for races.
Explanation for checker unneeded-cast (level3)unneeded-cast
Finds unneeded qobject_cast, static_cast and dynamic_casts. Warns when you're casting to base or casting to the same type, which doesn't require any explicit cast.
Also warns when you're using dynamic_cast for QObjects. qobject_cast is prefered.
Example
Foo *a = ...;
Foo *b = qobject_cast<Foo*>(a);
To shut the warnings about using qobject_cast over dynamic cast you can set:
export CLAZY_EXTRA_OPTIONS="unneeded-cast-prefer-dynamic-cast-over-qobject"
NOTE: This check has many false-positives. For example, you might cast to base
class to call a non-virtual method, or qobject_cast to itself to check if the
QObject destructor is currently being run.