[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