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

2017-09-12 Thread Peter Szecsi via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL313016: [clang-tidy] SuspiciousEnumUsageCheck bugfix (authored by szepet). Changed prior to commit: https://reviews.llvm.org/D37572?vs=114567&id=114788#toc Repository: rL LLVM https://reviews.llvm.o

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

2017-09-11 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh accepted this revision. alexfh added a comment. Still LG. Fine to commit. Thanks for the fix! If you find time to construct the test case that triggers both affected code paths (my test seems to only inspect one of them, IIUC), it would be nice to do as a follow up. https://reviews.ll

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

2017-09-11 Thread Peter Szecsi via Phabricator via cfe-commits
szepet updated this revision to Diff 114567. szepet marked 3 inline comments as done. szepet added a comment. Updates based on the comments. Thanks for the reduced test case and the fast review! Not sure if in these cases am I allowed to commit (since it was accepted) or since it was accepted a

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

2017-09-08 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: test/clang-tidy/misc-suspicious-enum-usage.cpp:122 +struct a> { + enum { ah = ad::m, + ai = ae::m, alexfh wrote: > alexfh wrote: > > aaron.ballman wrote: > > > alexfh wrote: > > > > aaron.ballman wrote: >

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

2017-09-08 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh added inline comments. Comment at: test/clang-tidy/misc-suspicious-enum-usage.cpp:122 +struct a> { + enum { ah = ad::m, + ai = ae::m, alexfh wrote: > aaron.ballman wrote: > > alexfh wrote: > > > aaron.ballman wrote: > > > > This seems like a lot o

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

2017-09-08 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh added inline comments. Comment at: test/clang-tidy/misc-suspicious-enum-usage.cpp:122 +struct a> { + enum { ah = ad::m, + ai = ae::m, aaron.ballman wrote: > alexfh wrote: > > aaron.ballman wrote: > > > This seems like a lot of complicated code for

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

2017-09-08 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: test/clang-tidy/misc-suspicious-enum-usage.cpp:122 +struct a> { + enum { ah = ad::m, + ai = ae::m, alexfh wrote: > aaron.ballman wrote: > > This seems like a lot of complicated code for the test case -- ca

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

2017-09-08 Thread Alexander Kornienko via Phabricator via cfe-commits
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 -- -confi

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

2017-09-08 Thread 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 `

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

2017-09-07 Thread Peter Szecsi via Phabricator via cfe-commits
szepet created this revision. Herald added subscribers: baloghadamsoftware, whisperity, JDevlieghere. There is a reported bug on the checker not handling the some APSInt values correctly: https://bugs.llvm.org/show_bug.cgi?id=34400 This patch fixes it, however, it shows a false positive. (Added