[PATCH] D22507: Clang-tidy - Enum misuse check

[Alexander Kornienko via cfe-commits]

alexfh requested changes to this revision.
alexfh added a comment.
This revision now requires changes to proceed.

Close, but still a bunch of comments in the docs and a suggestion to fix a class of false positives.


================
Comment at: clang-tidy/misc/EnumMisuseCheck.cpp:210
@@ +209,3 @@
+           DiagnosticIDs::Note);
+    } else if (!LhsConstant->getInitVal().isPowerOf2())
+      diag(LhsExpr->getExprLoc(), "left hand side value is not power-of-2 "
----------------
# llvm/lib/MC/ELFObjectWriter.cpp:903 - the warning looks reasonable.
# llvm/lib/Target/X86/Disassembler/X86DisassemblerDecoderCommon.h:66 - the warning looks reasonable (ATTR_max doesn't seem to be useful for the bitmask enum).
# llvm/tools/clang/lib/Basic/IdentifierTable.cpp:95 - two issues here:
  1. the "possibly contains misspelled number(s) " message could be more useful, if it specified which member corresponds to the possibly misspelled number
  2. I suppose, the check considers `KEYALL = (0x1fffff & ~KEYNOMS18 & ~KEYNOOPENCL)` to be incorrect. I think, it should exclude enum members initialized with a bit arithmetic expressions, since it's rather common to define aliases for a certain combination of flags.
# llvm/tools/clang/lib/Frontend/Rewrite/RewriteModernObjC.cpp:5083 and friends - the warning looks reasonable, since it's hard to understand the motivation for the `BLOCK_FIELD_IS_OBJECT   =  3`. If it's a combination of flags, it should be written as such, and the check should ignore enum members initialized with a bit arithmetic expression.

================
Comment at: docs/clang-tidy/checks/misc-enum-misuse.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.
+
----------------
Too much indentation here.

================
Comment at: docs/clang-tidy/checks/misc-enum-misuse.rst:38
@@ +37,3 @@
+ // Case 1 (strict and weak mode):
+ Enum {A, B, C};
+ Enum {D, E, F = 5};
----------------
1. `Enum` should not start with a capital letter, since it's a keyword.
2. Please indent the code block contents by 2 spaces (currently, it's indented by 1).
3. Please clang-format all code samples.

================
Comment at: docs/clang-tidy/checks/misc-enum-misuse.rst:47
@@ +46,3 @@
+ // Case 2 (only in strict mode):
+ Enum bitmask { A = 0;
+                B = 1;
----------------
Commas should be used instead of semicolons.

================
Comment at: docs/clang-tidy/checks/misc-enum-misuse.rst:54
@@ +53,3 @@
+                G = 31; // OK, real bitmask.
+ }
+
----------------
Missing semicolon.

In general, please make sure code snippets are valid code. Otherwise, it creates unneeded obstacles in reading the code.


https://reviews.llvm.org/D22507