[PATCH] D40854: [clang-tidy] WIP implement cppcoreguidelines check for mixed integer arithmetic

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

The "enforcement" listed on the C++ Core Guidelines is very unhelpful, so it might be worth bringing it up as an issue on their bug tracker. ES.100 basically says "you know what we mean, wink wink" as enforcement and doesn't give any guidance as to what is safe or unsafe. It gives no exceptions, which I think is unlikely to be useful to most developers. For instance: `void f(unsigned i) { if (i > 0) {} }`, this is a mixed signed and unsigned comparison (literals are signed by default) -- should this check diagnose? How about `unsigned int i = 12; i += 1;`? ES.102 is equally as strange with "Flag unsigned literals (e.g. -2) used as container subscripts." That's a signed literal (2) with a negation operator -- do they mean "Flag container subscripts whose value is known to be negative", or something else?



================
Comment at: clang-tidy/cppcoreguidelines/MixedIntArithmeticCheck.cpp:31
+      binaryOperator(allOf(anyOf(hasOperatorName("+"), hasOperatorName("-"),
+                                 hasOperatorName("*"), hasOperatorName("/")),
+                           hasEitherOperand(UnsignedIntegerOperand),
----------------
What about `%` or comparisons operators?


================
Comment at: test/clang-tidy/cppcoreguidelines-mixed-int-arithmetic.cpp:167-168
+void pure_signed() {
+  int SInt1 = 42u;
+  signed char SChar1 = 42u;
+  SignedEnum SE1 = SEnum1;
----------------
I think these are intended to be flagged under ES.102. "Flag results of unsigned arithmetic assigned to or printed as signed."


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D40854