[PATCH] D37808: [clang-tidy] Add new hicpp-multiway-paths-covered check for missing branches

[Jonas Devlieghere via Phabricator via cfe-commits]

JDevlieghere added inline comments.


================
Comment at: clang-tidy/hicpp/MultiwayPathsCoveredCheck.cpp:80
+
+  // Unsigned overflow occured. Returning max() is sufficient, since noone
+  // writes so many case labels in source code.
----------------
Maybe merge this with the function comment, no point in having both imho.


================
Comment at: clang-tidy/hicpp/MultiwayPathsCoveredCheck.cpp:82
+  // writes so many case labels in source code.
+  if (Bits > 0 && DiscreteValues <= 1) {
+    return std::numeric_limits<std::size_t>::max();
----------------
LLVM style guide says no braces with single line if/else statements. No need for the else after a return. Alternatively you could use the ternary operator. 


================
Comment at: clang-tidy/hicpp/MultiwayPathsCoveredCheck.cpp:100
+    return twoPow(Context.getTypeSize(T));
+  else
+    return twoPow(0ul);
----------------
No need for the else here. See previous comment. 


================
Comment at: clang-tidy/hicpp/MultiwayPathsCoveredCheck.cpp:146
+
+  if (CaseCount == 1 || CaseCount == 2) {
+    diag(Switch->getLocStart(),
----------------
Braces


================
Comment at: clang-tidy/hicpp/MultiwayPathsCoveredCheck.cpp:180
+    if (const auto *IntegerCondition =
+            Result.Nodes.getNodeAs<DeclRefExpr>("non-enum-condition")) {
+      return getNumberOfPossibleValues(IntegerCondition->getType(),
----------------
Braces


================
Comment at: clang-tidy/hicpp/MultiwayPathsCoveredCheck.cpp:183
+                                       *Result.Context);
+    } else if (const auto *BitfieldDecl =
+                   Result.Nodes.getNodeAs<FieldDecl>("bitfield")) {
----------------
You can just use if because of the return.


================
Comment at: clang-tidy/hicpp/MultiwayPathsCoveredCheck.cpp:186
+      return twoPow(BitfieldDecl->getBitWidthValue(*Result.Context));
+    } else {
+      llvm_unreachable("either bitfield or non-enum must be condition");
----------------
Drop the else


================
Comment at: clang-tidy/hicpp/MultiwayPathsCoveredCheck.cpp:198
+  // missed some value explicity, therefore add a default branch.
+  if (CaseCount < MaxPathsPossible) {
+    diag(Switch->getLocStart(), CoverMsgs[CaseCount == 1 ? 0 : 1]);
----------------
Braces


https://reviews.llvm.org/D37808