Commit 8d024d59 authored by Waqar Ahmed's avatar Waqar Ahmed Committed by Waqar Ahmed
Browse files

New check: Unexpected flag enumerator value

parent 800d0eb5
......@@ -4,6 +4,7 @@
- New Checks:
- use-arrow-operator-instead-of-data
- use-static-qregularexpression
- unexpected-flag-enumerator-value
* v1.10 (July 20th 2021)
- Requires C++17
......
......@@ -24,6 +24,7 @@ set(CLAZY_CHECKS_SRCS ${CLAZY_CHECKS_SRCS}
${CMAKE_CURRENT_LIST_DIR}/src/checks/manuallevel/signal-with-return-value.cpp
${CMAKE_CURRENT_LIST_DIR}/src/checks/manuallevel/thread-with-slots.cpp
${CMAKE_CURRENT_LIST_DIR}/src/checks/manuallevel/tr-non-literal.cpp
${CMAKE_CURRENT_LIST_DIR}/src/checks/manuallevel/unexpected-flag-enumerator-value.cpp
${CMAKE_CURRENT_LIST_DIR}/src/checks/manuallevel/unneeded-cast.cpp
${CMAKE_CURRENT_LIST_DIR}/src/checks/manuallevel/use-arrow-operator-instead-of-data.cpp
${CMAKE_CURRENT_LIST_DIR}/src/checks/manuallevel/use-chrono-in-qtimer.cpp
......
......@@ -25,6 +25,7 @@ add_test(NAME reserve-candidates COMMAND python3 run_tests.py reserve-candidates
add_test(NAME signal-with-return-value COMMAND python3 run_tests.py signal-with-return-value WORKING_DIRECTORY ${CMAKE_SOURCE_DIR}/tests/)
add_test(NAME thread-with-slots COMMAND python3 run_tests.py thread-with-slots WORKING_DIRECTORY ${CMAKE_SOURCE_DIR}/tests/)
add_test(NAME tr-non-literal COMMAND python3 run_tests.py tr-non-literal WORKING_DIRECTORY ${CMAKE_SOURCE_DIR}/tests/)
add_test(NAME unexpected-flag-enumerator-value COMMAND python3 run_tests.py unexpected-flag-enumerator-value WORKING_DIRECTORY ${CMAKE_SOURCE_DIR}/tests/)
add_test(NAME unneeded-cast COMMAND python3 run_tests.py unneeded-cast WORKING_DIRECTORY ${CMAKE_SOURCE_DIR}/tests/)
add_test(NAME use-arrow-operator-instead-of-data COMMAND python3 run_tests.py use-arrow-operator-instead-of-data WORKING_DIRECTORY ${CMAKE_SOURCE_DIR}/tests/)
add_test(NAME use-chrono-in-qtimer COMMAND python3 run_tests.py use-chrono-in-qtimer WORKING_DIRECTORY ${CMAKE_SOURCE_DIR}/tests/)
......
......@@ -247,6 +247,7 @@ clazy runs all checks from level1 by default.
- [signal-with-return-value](docs/checks/README-signal-with-return-value.md)
- [thread-with-slots](docs/checks/README-thread-with-slots.md)
- [tr-non-literal](docs/checks/README-tr-non-literal.md)
- [unexpected-flag-enumerator-value](docs/checks/README-unexpected-flag-enumerator-value.md)
- [unneeded-cast](docs/checks/README-unneeded-cast.md)
- [use-arrow-operator-instead-of-data](docs/checks/README-use-arrow-operator-instead-of-data.md)
- [use-chrono-in-qtimer](docs/checks/README-use-chrono-in-qtimer.md)
......
......@@ -725,6 +725,13 @@
"level" : 0,
"categories" : ["performance"],
"visits_stmts" : true
},
{
"name" : "unexpected-flag-enumerator-value",
"class_name" : "UnexpectedFlagEnumeratorValue",
"level" : -1,
"categories" : ["bug"],
"visits_decls" : true
}
]
}
# unexpected-flag-enumerator-value
Checks `enum`s that are used as flag, for example:
```cpp
enum Foo {
A = 0x1,
B = 0x2,
C = 0x4,
D = 0x9 // Oops, typo
};
```
and checks whether all enumeration values are power of 2 or not.
......@@ -24,6 +24,7 @@ SET(README_manuallevel_FILES
${CMAKE_CURRENT_LIST_DIR}/docs/checks/README-signal-with-return-value.md
${CMAKE_CURRENT_LIST_DIR}/docs/checks/README-thread-with-slots.md
${CMAKE_CURRENT_LIST_DIR}/docs/checks/README-tr-non-literal.md
${CMAKE_CURRENT_LIST_DIR}/docs/checks/README-unexpected-flag-enumerator-value.md
${CMAKE_CURRENT_LIST_DIR}/docs/checks/README-unneeded-cast.md
${CMAKE_CURRENT_LIST_DIR}/docs/checks/README-use-arrow-operator-instead-of-data.md
${CMAKE_CURRENT_LIST_DIR}/docs/checks/README-use-chrono-in-qtimer.md
......
......@@ -52,6 +52,7 @@
#include "checks/manuallevel/signal-with-return-value.h"
#include "checks/manuallevel/thread-with-slots.h"
#include "checks/manuallevel/tr-non-literal.h"
#include "checks/manuallevel/unexpected-flag-enumerator-value.h"
#include "checks/manuallevel/unneeded-cast.h"
#include "checks/manuallevel/use-arrow-operator-instead-of-data.h"
#include "checks/manuallevel/use-chrono-in-qtimer.h"
......@@ -164,6 +165,7 @@ void CheckManager::registerChecks()
registerCheck(check<SignalWithReturnValue>("signal-with-return-value", ManualCheckLevel, RegisteredCheck::Option_VisitsDecls));
registerCheck(check<ThreadWithSlots>("thread-with-slots", ManualCheckLevel, RegisteredCheck::Option_VisitsStmts | RegisteredCheck::Option_VisitsDecls));
registerCheck(check<TrNonLiteral>("tr-non-literal", ManualCheckLevel, RegisteredCheck::Option_VisitsStmts));
registerCheck(check<UnexpectedFlagEnumeratorValue>("unexpected-flag-enumerator-value", ManualCheckLevel, RegisteredCheck::Option_VisitsDecls));
registerCheck(check<UnneededCast>("unneeded-cast", ManualCheckLevel, RegisteredCheck::Option_VisitsStmts));
registerCheck(check<UseArrowOperatorInsteadOfData>("use-arrow-operator-instead-of-data", ManualCheckLevel, RegisteredCheck::Option_VisitsStmts));
registerCheck(check<UseChronoInQTimer>("use-chrono-in-qtimer", ManualCheckLevel, RegisteredCheck::Option_VisitsStmts));
......
/*
This file is part of the clazy static checker.
Copyright (C) 2021 Klarälvdalens Datakonsult AB, a KDAB Group company, info@kdab.com
Author: Waqar Ahmed <waqar.ahmed@kdab.com>
This library is free software; you can redistribute it and/or
modify it under the terms of the GNU Library General Public
License as published by the Free Software Foundation; either
version 2 of the License, or (at your option) any later version.
This library is distributed in the hope that it will be useful,
but WITHOUT ANY WARRANTY; without even the implied warranty of
MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
Library General Public License for more details.
You should have received a copy of the GNU Library General Public License
along with this library; see the file COPYING.LIB. If not, write to
the Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor,
Boston, MA 02110-1301, USA.
*/
#include "unexpected-flag-enumerator-value.h"
#include "Utils.h"
#include "HierarchyUtils.h"
#include "QtUtils.h"
#include "TypeUtils.h"
#include <clang/AST/AST.h>
using namespace clang;
using namespace std;
UnexpectedFlagEnumeratorValue::UnexpectedFlagEnumeratorValue(const std::string &name, ClazyContext *context)
: CheckBase(name, context)
{
}
static bool isIntentionallyNotPowerOf2(EnumConstantDecl *en) {
ConstantExpr *cexpr = dyn_cast_or_null<ConstantExpr>(clazy::getFirstChild(en->getInitExpr()));
if (!cexpr) {
return false;
}
if (auto binaryOp = dyn_cast_or_null<BinaryOperator>(cexpr->getSubExpr())) {
binaryOp->dump();
return true;
}
return false;
}
struct IsFlagEnumResult {
bool isFlagEnum;
int numFalseValues;
};
static SmallVector<EnumConstantDecl*, 16> getEnumerators(EnumDecl *enDecl)
{
SmallVector<EnumConstantDecl*, 16> ret;
for (auto *enumerator : enDecl->enumerators()) {
ret.push_back(enumerator);
}
return ret;
}
static uint64_t getIntegerValue(EnumConstantDecl* e)
{
return e->getInitVal().getLimitedValue();
}
static bool hasConsecutiveValues(const SmallVector<EnumConstantDecl*, 16>& enumerators)
{
auto val = getIntegerValue(enumerators.front());
bool consecutive = true;
for (size_t i = 1; i < enumerators.size(); ++i) {
val++;
consecutive = getIntegerValue(enumerators[i]) == val;
}
return consecutive;
}
static IsFlagEnumResult isFlagEnum(const SmallVector<EnumConstantDecl*, 16>& enumerators)
{
if (enumerators.size() < 4) {
return {false, 0};
}
if (hasConsecutiveValues(enumerators)) {
return {false, 0};
}
llvm::SmallVector<bool, 16> enumValues;
for (auto *enumerator : enumerators) {
enumValues.push_back(enumerator->getInitVal().isPowerOf2());
}
const size_t count = std::count(enumValues.begin(), enumValues.end(), false);
// If half of our values were power-of-2, this is probably a flag enum
IsFlagEnumResult res;
res.isFlagEnum = count <= (enumerators.size() / 2);
res.numFalseValues = count;
return res;
}
void UnexpectedFlagEnumeratorValue::VisitDecl(clang::Decl *decl)
{
EnumDecl* enDecl = dyn_cast_or_null<EnumDecl>(decl);
if (!enDecl) {
return;
}
const SmallVector<EnumConstantDecl*, 16> enumerators = getEnumerators(enDecl);
auto flagEnum = isFlagEnum(enumerators);
if (!flagEnum.isFlagEnum) {
return;
}
for (EnumConstantDecl* enumerator : enumerators) {
if (!enumerator->getInitVal().isPowerOf2()) {
if (isIntentionallyNotPowerOf2(enumerator)) {
continue;
}
const auto value = enumerator->getInitVal().getLimitedValue();
emitWarning(enumerator->getInitExpr()->getBeginLoc(), "Unexpected non power-of-2 enumerator value: " + std::to_string(value));
}
}
}
/*
This file is part of the clazy static checker.
Copyright (C) 2021 Klarälvdalens Datakonsult AB, a KDAB Group company, info@kdab.com
Author: Waqar Ahmed <waqar.ahmed@kdab.com>
This library is free software; you can redistribute it and/or
modify it under the terms of the GNU Library General Public
License as published by the Free Software Foundation; either
version 2 of the License, or (at your option) any later version.
This library is distributed in the hope that it will be useful,
but WITHOUT ANY WARRANTY; without even the implied warranty of
MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
Library General Public License for more details.
You should have received a copy of the GNU Library General Public License
along with this library; see the file COPYING.LIB. If not, write to
the Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor,
Boston, MA 02110-1301, USA.
*/
#ifndef CLAZY_UNEXPECTED_FLAG_ENUMERATOR_VALUE_H
#define CLAZY_UNEXPECTED_FLAG_ENUMERATOR_VALUE_H
#include "checkbase.h"
/**
* See README-unexpected-flag-enumerator-value.md for more info.
*/
class UnexpectedFlagEnumeratorValue : public CheckBase
{
public:
explicit UnexpectedFlagEnumeratorValue(const std::string &name, ClazyContext *context);
void VisitDecl(clang::Decl *) override;
private:
};
#endif
{
"tests" : [
{
"filename" : "main.cpp"
}
]
}
// Enum is too small, won't be checked
enum TooSmall {
FooTooSmall = 1, // Ok
BarTooSmall = 2, // Ok
LalaTooSmall = 4, // Ok
};
enum TEST_FLAGS {
Foo = 1, // Ok
Bar = 2, // Ok
Both = Foo | Bar, // OK
Something = 3 // Warn
};
// Copied from Qt for testing purposes
enum AlignmentFlag {
AlignLeft = 0x0001,
AlignLeading = AlignLeft,
AlignRight = 0x0002,
AlignTrailing = AlignRight,
AlignHCenter = 0x0004,
AlignJustify = 0x0008,
AlignAbsolute = 0x0011, // Warn
AlignHorizontal_Mask = AlignLeft | AlignRight | AlignHCenter | AlignJustify | AlignAbsolute,
AlignTop = 0x0020,
AlignBottom = 0x0040,
AlignVCenter = 0x0080,
AlignBaseline = 0x0100,
// Note that 0x100 will clash with Qt::TextSingleLine = 0x100 due to what the comment above
// this enum declaration states. However, since Qt::AlignBaseline is only used by layouts,
// it doesn't make sense to pass Qt::AlignBaseline to QPainter::drawText(), so there
// shouldn't really be any ambiguity between the two overlapping enum values.
AlignVertical_Mask = AlignTop | AlignBottom | AlignVCenter | AlignBaseline,
AlignCenter = AlignVCenter | AlignHCenter
};
// No Warning here, all consecutive values
enum {
RED = 1,
GREEN = 2,
BLUE = 3,
BLACK = 4,
WHITE = 5,
};
unexpected-flag-enumerator-value/main.cpp:14:17: warning: Unexpected non power-of-2 enumerator value: 3 [-Wclazy-unexpected-flag-enumerator-value]
unexpected-flag-enumerator-value/main.cpp:25:21: warning: Unexpected non power-of-2 enumerator value: 17 [-Wclazy-unexpected-flag-enumerator-value]
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment