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

Alexander Kornienko via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Sep 8 08:37:59 PDT 2017


alexfh accepted this revision.
alexfh added a comment.
This revision is now accepted and ready to land.

LG modulo comments. Thank you for the fix!



================
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
 
----------------
aaron.ballman wrote:
> I thought we built in C++11 mode by default these days?
I'm not sure, but if we actually do, we could later remove `-std=c++11` from the test script and all tests.


================
Comment at: test/clang-tidy/misc-suspicious-enum-usage.cpp:92
+
+// Bug #34400
+template <typename>
----------------
I'd suggest enclosing the test case in a `namespace PR34400 {}` for clarity and to avoid possible name conflicts.


================
Comment at: test/clang-tidy/misc-suspicious-enum-usage.cpp:122
+struct a<f<ad, ae, af>> {
+  enum { ah = ad::m,
+         ai = ae::m,
----------------
aaron.ballman wrote:
> This seems like a lot of complicated code for the test case -- can this be reduced further?
This is the best creduce could do. It should be possible to make this much shorter, but I wouldn't spend too much time on that.


https://reviews.llvm.org/D37572





More information about the cfe-commits mailing list