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

[Peter Szecsi via cfe-commits]

szepet added inline comments.

================
Comment at: clang-tidy/misc/EnumMisuseCheck.cpp:75
@@ +74,3 @@
+// 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
----------------
aaron.ballman wrote:
> The wrapping for this comment is a bit strange, also should be using doxygen-style comments. Also, the grammar is a bit off for the comment. I would recommend:
> ```
> Check if there are two or more enumerators that are not a power of 2 in the enum type, and that the enumeration contains enough elements to reasonably act as a bitmask. Exclude the case where the last enumerator is the sum of the lesser values or when it could contain consecutive values.
> ```
> Also, I would call this `isPossiblyBitMask` instead of using "bit field" because a bit-field is a syntactic construct that is unrelated. Bitmask types are covered in the C++ standard under [bitmask.types] and are slightly different, but more closely related to what this check is looking for.
Thanks for the recommendations! As you can see my grammar and vocabulary is a "bit strange" so I really appreciated the correction!

================
Comment at: clang-tidy/misc/EnumMisuseCheck.cpp:209
@@ +208,3 @@
+    if (LhsVar) {
+      diag(EnumDec->getInnerLocStart(), "enum type seems like a bitfield but "
+                                        "possibly contains misspelled "
----------------
aaron.ballman wrote:
> I think this diagnostic text is a bit confusing. The enum type shouldn't seem like a bit-field (but more like a bitmask), and I'm not certain what a misspelled number would be. I think the diagnostic is effectively saying that we guess this might be a bitmask, but some of the enumerator values don't make sense for that guess, and so this may be suspicious code -- but I really worry about the false positive rate for such a diagnostic. Have you run this check over a large body of work (like LLVM, Firefox, Chrome, etc)? If so, how do the diagnostics look?
> 
> Perhaps a different way to word the diagnostic is: "enum type used as a bitmask with an enumerator value that is not a power of 2" and place the diagnostic on the enumerator(s) that cause the problem rather than the enumeration as a whole.
Here you can see the results on LLVM. (weak options, less false positive)

{F2319113}

Here I have to mention that the last 4 results could be combined into one, because it`s actually the usage of the same enum in different files. If you wish I could easily change it.


https://reviews.llvm.org/D22507