[PATCH] D14889: sancov -not_covered_functions.

[Ivan Krasin via llvm-commits]

krasin added a comment.

First bunch of comments from Friday. Will add more later.


================
Comment at: tools/sancov/sancov.cc:63
@@ -44,1 +62,3 @@
                           "Print all covered funcions."),
+               clEnumValN(NotCoveredFunctionsAction, "not_covered_functions",
+                          "Print all not covered funcions."),
----------------
LLVM tools use flags (with a notable exception of llvm-dwarfdump) with a dash instead of an underscore.
Please rename the flag to --not-covered-functions, and also do the same with --covered-functions.

================
Comment at: tools/sancov/sancov.cc:95
@@ -76,2 +94,3 @@
   errs() << "Error: " << Error.message() << "(" << Error.value() << ")\n";
-  exit(-2);
+  exit(-1);
+}
----------------
Ditto, exit(1)

================
Comment at: tools/sancov/sancov.cc:442
@@ +441,3 @@
+  // Line format: <file_name>:<line> <function_name>
+  void printNotCoveredFunctions(raw_ostream &out) {
+    std::set<FunctionLoc> AllFns =
----------------
Per LLVM style guide: http://llvm.org/docs/CodingStandards.html#name-types-functions-variables-and-enumerators-properly

> Variable names should be nouns (as they represent state). The name should be camel case, and start with an upper case letter (e.g. Leader or Boats).

Please, change everywhere (except main)

================
Comment at: tools/sancov/sancov.cc:95
@@ -76,2 +94,3 @@
   errs() << "Error: " << Error.message() << "(" << Error.value() << ")\n";
-  exit(-2);
+  exit(-1);
+}
----------------
Why not exit(1)? LLVM tools use 1 as the exit code in case of an error:

ag "exit[(]1[)]" tools | wc -l
109

There's 3 python scripts which violate that, but otherwise the rule is followed pretty much everywhere.

================
Comment at: tools/sancov/sancov.cc:439
@@ -232,4 +438,3 @@
 
-        out << FileName << ":" << Line << " " << FunctionName << "\n";
-      }
-    }
+  // Print list of covered functions.
+  // Line format: <file_name>:<line> <function_name>
----------------
Please fix the comment: "Print list of not covered functions".


http://reviews.llvm.org/D14889