alexfh requested changes to this revision.
alexfh added a comment.
This revision now requires changes to proceed.
Thank you for the updates!
Please re-run the check on LLVM to see what has changed.
================
Comment at: clang-tidy/misc/SuspiciousEnumUsageCheck.cpp:53
@@ +52,3 @@
+}
+static bool isNonPowerOf2NorNullLiteral(const EnumConstantDecl *EnumConst) {
+ if (const Expr *InitExpr = EnumConst->getInitExpr()) {
----------------
nit: Add an empty line above.
================
Comment at: clang-tidy/misc/SuspiciousEnumUsageCheck.cpp:61
@@ +60,3 @@
+}
+static bool isMaxValAllBitSetLiteral(const EnumDecl *EnumDec) {
+ auto EnumConst = std::max_element(
----------------
nit: Add an empty line above.
================
Comment at: clang-tidy/misc/SuspiciousEnumUsageCheck.cpp:67
@@ +66,3 @@
+ });
+ if (const Expr *InitExpr = EnumConst->getInitExpr())
+ return EnumConst->getInitVal().countTrailingOnes() ==
----------------
nit: Please add braces, since the body doesn't fit on a line.
================
Comment at: clang-tidy/misc/SuspiciousEnumUsageCheck.cpp:155
@@ +154,3 @@
+
+ if (EnumDec->enumerator_begin() == EnumDec->enumerator_end() ||
+ OtherEnumDec->enumerator_begin() == OtherEnumDec->enumerator_end())
----------------
Why?
================
Comment at: clang-tidy/misc/SuspiciousEnumUsageCheck.cpp:175
@@ +174,3 @@
+ int NonPowOfTwoCounter = countNonPowOfTwoLiteralNum(EnumDec);
+ if (isPossiblyBitMask(NonPowOfTwoCounter, EnumLen, VR, EnumDec)) {
+ const auto *EnumDecExpr = dyn_cast<DeclRefExpr>(EnumExpr);
----------------
Use early return here.
================
Comment at: clang-tidy/misc/SuspiciousEnumUsageCheck.cpp:208
@@ +207,3 @@
+ RhsDecExpr ? dyn_cast<EnumConstantDecl>(RhsDecExpr->getDecl()) : nullptr;
+ bool LhsVar = !LhsConstant, RhsVar = !RhsConstant;
+
----------------
One variable definition at a time, please.
================
Comment at: clang-tidy/misc/SuspiciousEnumUsageCheck.cpp:216
@@ +215,3 @@
+ int EnumLen = enumLength(EnumDec);
+ if (isPossiblyBitMask(NonPowOfTwoCounter, EnumLen, VR, EnumDec) &&
+ (StrictMode ||
----------------
Use early return here.
================
Comment at: clang-tidy/misc/SuspiciousEnumUsageCheck.h:32-35
@@ +31,6 @@
+ const bool StrictMode;
+ const char* DifferentEnumErrorMessage;
+ const char* BitmaskErrorMessage;
+ const char* BitmaskVarErrorMessage;
+ const char* BitmaskNoteMessage;
+};
----------------
These can be just static constants in the .cpp file. Apart from that, `const
char X[] = ...;` is a better way to define string constants, otherwise you
would have to go with `const char * const X = ...;` to make the pointer const
as well.
================
Comment at: docs/clang-tidy/checks/misc-suspicious-enum-usage.rst:4
@@ +3,3 @@
+misc-suspicious-enum-usage
+================
+
----------------
This line should be aligned with the line above.
================
Comment at: docs/clang-tidy/checks/misc-suspicious-enum-usage.rst:9
@@ +8,3 @@
+1. When "ADD" or "bitwise OR" is used between two enum which come from
different
+ types and these types value ranges are not disjoint.
+
----------------
Remove two spaces at the start of the line.
================
Comment at: docs/clang-tidy/checks/misc-suspicious-enum-usage.rst:11
@@ +10,3 @@
+
+In the following cases you can choose either "Strict" or "Weak" option.
+In "Strict" mode we check if the used EnumConstantDecl type contains almost
----------------
There's no "Weak" option, it's the "Strict" option set to false / 0.
================
Comment at: docs/clang-tidy/checks/misc-suspicious-enum-usage.rst:12
@@ +11,3 @@
+In the following cases you can choose either "Strict" or "Weak" option.
+In "Strict" mode we check if the used EnumConstantDecl type contains almost
+only pow-of-2 numbers.
----------------
Please use the `:option:` notation and add the option description (`.. option::
...`). See, for example, modernize-use-emplace.rst.
================
Comment at: docs/clang-tidy/checks/misc-suspicious-enum-usage.rst:31
@@ +30,3 @@
+
+===============
+
----------------
This notation is used to make the previous line a heading. Doesn't seem to be
the case here. See http://llvm.org/docs/SphinxQuickstartTemplate.html for some
examples. Please also try to build your docs to check for obvious issues.
================
Comment at: test/clang-tidy/misc-suspicious-enum-usage-strict.cpp:18
@@ +17,3 @@
+ ZZ = 3
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: enum type seems like a bitmask
(contains mostly power-of-2 literals), but this literal is not a power-of-2
[misc-suspicious-enum-usage]
+};
----------------
Add check lines for notes as well (I don't think you'll be able to use
[[@LINE]] for most of them, but you can probably just skip the line.
https://reviews.llvm.org/D22507
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
