[PATCH] D37572: [clang-tidy] SuspiciousEnumUsageCheck bugfix

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.


================
Comment at: clang-tidy/misc/SuspiciousEnumUsageCheck.cpp:46
+          return llvm::APSInt::compareValues(E1->getInitVal(),
+                                             E2->getInitVal()) == -1;
         });
----------------
I would test for `< 0` rather than a direct equality test (compare functions generally follow the C idioms).


================
Comment at: clang-tidy/misc/SuspiciousEnumUsageCheck.cpp:61-62
   ValueRange Range1(Enum1), Range2(Enum2);
-  return (Range1.MaxVal < Range2.MinVal) || (Range2.MaxVal < Range1.MinVal);
+  bool Less1 = llvm::APSInt::compareValues(Range1.MaxVal, Range2.MinVal) == -1;
+  bool Less2 = llvm::APSInt::compareValues(Range2.MaxVal, Range1.MinVal) == -1;
+  return Less1 || Less2;
----------------
Same here, though I would also get rid of the local variables and just perform the comparison in the return statement.


================
Comment at: test/clang-tidy/misc-suspicious-enum-usage.cpp:1
-// RUN: %check_clang_tidy %s misc-suspicious-enum-usage %t -- -config="{CheckOptions: [{key: misc-suspicious-enum-usage.StrictMode, value: 0}]}" --
+// RUN: %check_clang_tidy %s misc-suspicious-enum-usage %t -- -config="{CheckOptions: [{key: misc-suspicious-enum-usage.StrictMode, value: 0}]}" -- -std=c++11
 
----------------
I thought we built in C++11 mode by default these days?


================
Comment at: test/clang-tidy/misc-suspicious-enum-usage.cpp:122
+struct a<f<ad, ae, af>> {
+  enum { ah = ad::m,
+         ai = ae::m,
----------------
This seems like a lot of complicated code for the test case -- can this be reduced further?


https://reviews.llvm.org/D37572