[PATCH] D37808: [clang-tidy] Add new hicpp-multiway-paths-covered check for missing branches
Jonas Devlieghere via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Mon Oct 16 08:18:58 PDT 2017
JDevlieghere added inline comments.
================
Comment at: clang-tidy/hicpp/MultiwayPathsCoveredCheck.cpp:76
+
+std::size_t twoPow(std::size_t Bits) {
+ const std::size_t DiscreteValues = 1ul << Bits;
----------------
Add a comment describing what this function does. I'd move and rephrase the comment below.
================
Comment at: clang-tidy/hicpp/MultiwayPathsCoveredCheck.cpp:98
+ // hold.
+ const std::size_t BitCount = [&T, &Context]() {
+ if (T->isIntegralType(Context))
----------------
Unless you expect a whole bunch of logic to be added here, I'd un-const and initialize BitCount to zero, then just have if-clause reassign it and get rid of the lambda. This will save you a few lines of code and complexity.
================
Comment at: clang-tidy/hicpp/MultiwayPathsCoveredCheck.cpp:149
+
+ llvm::SmallVector<std::string, 2> DegenerateMsgs = {
+ "degenerated switch with default label only",
----------------
If there's only going to be two messages, you could use the ternary operator and save an instantiation of the `SmallVector`.
================
Comment at: clang-tidy/hicpp/MultiwayPathsCoveredCheck.cpp:195
+ // matcher used for here does not match on degenerate 'switch'
+ assert(CaseCount > 0 && "Switch stmt without any case found. This case "
+ "should be excluded by the matcher and is handled "
----------------
Let's move this to right after where you define CaseCount
https://reviews.llvm.org/D37808
More information about the cfe-commits
mailing list