szepet removed rL LLVM as the repository for this revision.
szepet updated this revision to Diff 64897.
szepet added a comment.
updating patch based on review comments
https://reviews.llvm.org/D22507
Files:
clang-tidy/misc/EnumMisuseCheck.cpp
clang-tidy/misc/EnumMisuseCheck.h
docs/clang-tidy/checks/misc-enum-misuse.rst
test/clang-tidy/misc-enum-misuse-weak.cpp
test/clang-tidy/misc-enum-misuse.cpp
Index: test/clang-tidy/misc-enum-misuse.cpp
===================================================================
--- test/clang-tidy/misc-enum-misuse.cpp
+++ test/clang-tidy/misc-enum-misuse.cpp
@@ -1,6 +1,7 @@
// RUN: %check_clang_tidy %s misc-enum-misuse %t -- -config="{CheckOptions: [{key: misc-enum-misuse.IsStrict, value: 0}]}" --
-enum Empty {};
+enum Empty {
+};
enum A { A = 1,
B = 2,
@@ -8,23 +9,27 @@
D = 8,
E = 16,
F = 32,
- G = 63 };
+ G = 63
+};
enum X { X = 8,
Y = 16,
- Z = 4 };
+ Z = 4
+};
-enum {P = 2,
- Q = 3,
- R = 4,
- S = 8,
- T = 16 };
+enum { P = 2,
+ Q = 3,
+ R = 4,
+ S = 8,
+ T = 16
+};
enum { H,
I,
J,
K,
- L };
+ L
+};
enum Days { Monday,
Tuesday,
@@ -32,14 +37,14 @@
Thursday,
Friday,
Saturday,
- Sunday };
+ Sunday
+};
Days bestDay() {
return Friday;
}
int trigger() {
-
if (bestDay() | A)
return 1;
// CHECK-MESSAGES: :[[@LINE-2]]:17: warning: enum values are from different enum types [misc-enum-misuse]
@@ -47,9 +52,8 @@
return 1;
// CHECK-MESSAGES: :[[@LINE-2]]:9: warning: enum values are from different enum types [misc-enum-misuse]
unsigned p;
- p = Q | P;
+ p = Q | P;
// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: left hand side value is not power-of-2 unlike most other values in the enum type [misc-enum-misuse]
-
}
int dont_trigger() {
@@ -74,4 +78,4 @@
if (H + I + L == 42)
return 1;
return 42;
-};
+}
Index: test/clang-tidy/misc-enum-misuse-weak.cpp
===================================================================
--- test/clang-tidy/misc-enum-misuse-weak.cpp
+++ test/clang-tidy/misc-enum-misuse-weak.cpp
@@ -6,24 +6,28 @@
D = 8,
E = 16,
F = 32,
- G = 63 };
+ G = 63
+};
enum X { X = 8,
Y = 16,
- Z = 4 };
+ Z = 4
+};
// CHECK-MESSAGES: :[[@LINE+1]]:1: warning: enum type seems like a bitfield but possibly contains misspelled number(s) [misc-enum-misuse]
enum PP { P = 2,
Q = 3,
R = 4,
S = 8,
T = 16,
- U = 31};
+ U = 31
+};
enum { H,
I,
J,
K,
- L };
+ L
+};
enum Days { Monday,
Tuesday,
@@ -31,7 +35,8 @@
Thursday,
Friday,
Saturday,
- Sunday };
+ Sunday
+};
Days bestDay() {
return Friday;
@@ -59,7 +64,7 @@
// Line 60 triggers the LINE:18 warning
p = A | G;
//p = G | X;
- return 0;
+ return 0;
}
int dont_trigger() {
@@ -68,7 +73,7 @@
int d = c | H, e = b * a;
a = B | C;
b = X | Z;
-
+
unsigned bitflag;
enum A aa = B;
bitflag = aa | C;
@@ -80,5 +85,4 @@
if (H + I + L == 42)
return 1;
return 42;
-};
-
+}
Index: docs/clang-tidy/checks/misc-enum-misuse.rst
===================================================================
--- docs/clang-tidy/checks/misc-enum-misuse.rst
+++ docs/clang-tidy/checks/misc-enum-misuse.rst
@@ -23,3 +23,44 @@
enum val. (only in "Strict")
4. Check both side of | or + operator where the enum values are from the same
enum type.
+
+Examples:
+
+.. code:: c++
+
+//1.
+Enum {A, B, C}
+Enum {D, E, F}
+Enum {G = 10, H = 11, I = 12};
+
+unsigned flag;
+flag = A | H; //OK, disjoint value intervalls in the enum types > probably good use
+flag = B | F; //warning, have common values so they are probably misused
+
+
+
+//2.
+
+Enum Bitfield { A = 0;
+ B = 1;
+ C = 2;
+ D = 4;
+ E = 8;
+ F = 16;
+ G = 31; //OK, real bitfield
+}
+
+Enum AlmostBitfield { AA = 0;
+ BB = 1;
+ CC = 2;
+ DD = 4;
+ EE = 8;
+ FF = 16;
+ GG; //Problem, forgot to initialize
+}
+
+ unsigned flag = 0;
+ flag |= E; //ok
+ flag |= EE; //warning at the decl, and note that it was used here as a bitfield
+
+void f(const string&); // Good: const is not top level.
Index: clang-tidy/misc/EnumMisuseCheck.h
===================================================================
--- clang-tidy/misc/EnumMisuseCheck.h
+++ clang-tidy/misc/EnumMisuseCheck.h
@@ -16,18 +16,20 @@
namespace tidy {
namespace misc {
-/// FIXME: Write a short description.
-///
+/// The checker detects various cases when an enum is probably misused (as a
+/// bitfield).
/// For the user-facing documentation see:
/// http://clang.llvm.org/extra/clang-tidy/checks/misc-enum-misuse.html
class EnumMisuseCheck : public ClangTidyCheck {
- const bool IsStrict;
+private:
+ const bool IsStrict;
- public:
- EnumMisuseCheck(StringRef Name, ClangTidyContext *Context)
- : ClangTidyCheck(Name, Context), IsStrict(Options.get("IsStrict", 1)) {}
- void registerMatchers(ast_matchers::MatchFinder *Finder) override;
- void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+public:
+ EnumMisuseCheck(StringRef Name, ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context), IsStrict(Options.get("IsStrict", 1)) {}
+ void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+ void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+ void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
};
} // namespace misc
Index: clang-tidy/misc/EnumMisuseCheck.cpp
===================================================================
--- clang-tidy/misc/EnumMisuseCheck.cpp
+++ clang-tidy/misc/EnumMisuseCheck.cpp
@@ -16,23 +16,12 @@
namespace tidy {
namespace misc {
-// Return the number of EnumConstantDecls in an EnumDecl.
-static int enumLen(const EnumDecl *EnumDec) {
- int Counter = 0;
-
- for (auto E : EnumDec->enumerators()) {
- (void)E;
- Counter++;
- }
- return Counter;
-}
-
// Stores a min and a max value which describe an interval.
struct ValueRange {
- llvm::APSInt MinVal, MaxVal;
+ llvm::APSInt MinVal;
+ llvm::APSInt MaxVal;
ValueRange(const EnumDecl *EnumDec) {
-
llvm::APSInt BeginVal = EnumDec->enumerator_begin()->getInitVal();
MinVal = BeginVal;
MaxVal = BeginVal;
@@ -46,19 +35,24 @@
}
};
+// Return the number of EnumConstantDecls in an EnumDecl.
+static int enumLength(const EnumDecl *EnumDec) {
+ return std::distance(EnumDec->enumerator_begin(), EnumDec->enumerator_end());
+}
+
static bool hasDisjointValueRange(const EnumDecl *Enum1,
const EnumDecl *Enum2) {
ValueRange Range1(Enum1), Range2(Enum2);
return ((Range1.MaxVal < Range2.MinVal) || (Range2.MaxVal < Range1.MinVal));
}
-bool hasCommonBit(const llvm::APSInt &Val1, const llvm::APSInt &Val2) {
+static bool hasCommonBit(const llvm::APSInt &Val1, const llvm::APSInt &Val2) {
return (Val1 & Val2).getExtValue();
}
bool isMaxValAllBitSet(const EnumDecl *EnumDec) {
for (auto I = EnumDec->enumerator_begin(), E = EnumDec->enumerator_end();
- I != E; I++) {
+ I != E; ++I) {
auto It = I;
if (++It == E)
return I->getInitVal().countTrailingOnes() ==
@@ -77,8 +71,23 @@
return Counter;
}
-void EnumMisuseCheck::registerMatchers(MatchFinder *Finder) {
+// We check if there is at most 2 not power-of-2 value in the enum type and the
+// it contains enough element to make sure it could be a biftield, but we
+// exclude the cases when the last number is the sum of the lesser values or
+// when it could contain consecutive numbers.
+static bool isPossiblyBitField(int NonPowOfTwoCounter, int EnumLen,
+ const ValueRange &VR, const EnumDecl *EnumDec) {
+ return NonPowOfTwoCounter >= 1 && NonPowOfTwoCounter <= 2 &&
+ NonPowOfTwoCounter < enumLength(EnumDec) / 2 &&
+ (VR.MaxVal - VR.MinVal != EnumLen - 1) &&
+ !(NonPowOfTwoCounter == 1 && isMaxValAllBitSet(EnumDec));
+}
+void EnumMisuseCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
+ Options.store(Opts, "IsStrict", IsStrict);
+}
+
+void EnumMisuseCheck::registerMatchers(MatchFinder *Finder) {
const auto enumExpr = [](StringRef RefName, StringRef DeclName) {
return allOf(ignoringImpCasts(expr().bind(RefName)),
ignoringImpCasts(hasType(enumDecl().bind(DeclName))));
@@ -113,19 +122,12 @@
hasRHS(enumExpr("enumExpr", "enumDecl"))),
this);
}
-// if there is only one not power-of-2 value in the enum unless it is
-static bool isPossiblyBitField(const int NonPowOfTwoCounter, const int EnumLen,
- const ValueRange &VR, const EnumDecl *EnumDec) {
- return NonPowOfTwoCounter >= 1 && NonPowOfTwoCounter <= 2 && NonPowOfTwoCounter < enumLen(EnumDec) / 2 &&
- (VR.MaxVal - VR.MinVal != EnumLen - 1) && !(NonPowOfTwoCounter == 1 && isMaxValAllBitSet(EnumDec));
-}
void EnumMisuseCheck::check(const MatchFinder::MatchResult &Result) {
- const auto *DiffEnumOp = Result.Nodes.getNodeAs<BinaryOperator>("diffEnumOp");
// 1. case: The two enum values come from different types.
- if (DiffEnumOp) {
-
+ if (const auto *DiffEnumOp =
+ Result.Nodes.getNodeAs<BinaryOperator>("diffEnumOp")) {
const auto *EnumDec = Result.Nodes.getNodeAs<EnumDecl>("enumDecl");
const auto *OtherEnumDec =
Result.Nodes.getNodeAs<EnumDecl>("otherEnumDecl");
@@ -137,7 +139,6 @@
if (!hasDisjointValueRange(EnumDec, OtherEnumDec))
diag(DiffEnumOp->getOperatorLoc(),
"enum values are from different enum types");
-
return;
}
@@ -145,14 +146,13 @@
// a, Investigating the right hand side of += or |= operator.
// b, When the operator is | or + but only one of them is an
// EnumExpr
- const auto *EnumExpr = Result.Nodes.getNodeAs<Expr>("enumExpr");
- if (EnumExpr) {
+ if (const auto *EnumExpr = Result.Nodes.getNodeAs<Expr>("enumExpr")) {
if (!IsStrict)
return;
const auto *EnumDec = Result.Nodes.getNodeAs<EnumDecl>("enumDecl");
ValueRange VR(EnumDec);
- int EnumLen = enumLen(EnumDec);
+ int EnumLen = enumLength(EnumDec);
int NonPowOfTwoCounter = countNonPowOfTwoNum(EnumDec);
if (isPossiblyBitField(NonPowOfTwoCounter, EnumLen, VR, EnumDec)) {
const auto *EnumDecExpr = dyn_cast<DeclRefExpr>(EnumExpr);
@@ -167,13 +167,14 @@
else if (!EnumConstDecl) {
diag(EnumDec->getInnerLocStart(), "enum type seems like a bitfield but "
"possibly contains misspelled "
- "number(s) ");
+ "number(s)");
diag(EnumExpr->getExprLoc(), "Used here as a bitfield.",
DiagnosticIDs::Note);
}
}
return;
}
+
// 3. case
// | or + operator where both argument comes from the same enum type
@@ -198,7 +199,7 @@
int NonPowOfTwoCounter = countNonPowOfTwoNum(EnumDec);
ValueRange VR(EnumDec);
- int EnumLen = enumLen(EnumDec);
+ int EnumLen = enumLength(EnumDec);
if (isPossiblyBitField(NonPowOfTwoCounter, EnumLen, VR, EnumDec) &&
(IsStrict ||
(EnumBinOp->isBitwiseOp() && RhsConstant && LhsConstant &&
@@ -207,19 +208,17 @@
diag(EnumDec->getInnerLocStart(), "enum type seems like a bitfield but "
"possibly contains misspelled "
"number(s)");
- diag(EnumExpr->getExprLoc(), "Used here as a bitfield.",
+ diag(LhsExpr->getExprLoc(), "Used here as a bitfield.",
DiagnosticIDs::Note);
} else if (!(LhsConstant->getInitVal()).isPowerOf2())
diag(LhsExpr->getExprLoc(), "left hand side value is not power-of-2 "
"unlike most other values in the enum type");
if (RhsVar) {
- // if (LhsVar)
- // return;
diag(EnumDec->getInnerLocStart(), "enum type seems like a bitfield but "
"possibly contains misspelled "
"number(s)");
- diag(EnumExpr->getExprLoc(), "Used here as a bitfield.",
+ diag(RhsExpr->getExprLoc(), "Used here as a bitfield.",
DiagnosticIDs::Note);
} else if (!(RhsConstant->getInitVal()).isPowerOf2())
diag(RhsExpr->getExprLoc(), "right hand side value is not power-of-2 "
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
