[llvm-cov] Allow commas in filenames passed to `-object` flag

[andrewjcg]

Summary

Currently, -object takes a comma separated list of objects as an
argument, which prevents it working with path names that contain a
comma. Drop comma-separated support, which requires to set pass the
-object flag multiple times to set multiple objects.

[vsk]

I don't think the intention was ever to require filename arguments to be comma separated (the expectation is that multiple -object arguments should be passed). This looks like a reasonable cleanup though.

It should be simple enough to add a test for this. There are a few examples of -object being passed multiple times in llvm/test/tools/llvm-cov; one of these can converted to test RUN: not llvm-cov ... -object=f1,f2 ... 2>&1 | FileCheck %s.

[andrewjcg]

add test

[vsk]

Thanks, lgtm with a minor change.

llvm/test/tools/llvm-cov/comma,in,path.test

3 ↗

(On Diff #291085)

It'd be nice to check for something more than exit code 0, e.g. using -dump-collected-paths.

[andrewjcg]

test for something more than exit code

[andrewjcg]

As I don't have commit access, is there someone on this diff that can push this? Or is there some process for this?

[vsk]

@andrewjcg I've committed this in 3c731ba5f1b. I found conflicting information online about whether Windows allows filenames with commas in them. To err on the safe side (and avoid potentially corrupting the git history for some llvm devs), I changed the test. Hope you don't mind -- if you prefer another way of doing it, let me know.

[andrewjcg]

@vsk: Looks good, thanks!

llvm/test/tools/llvm-cov/comma-in-coverage-object-filename.test

+RUN: llvm-cov show %t -instr-profile %t -dump-collected-objects -object a,b | FileCheck %s
+
+CHECK: a,b

llvm/tools/llvm-cov/CodeCoverage.cpp

void CodeCoverageTool::writeSourceFileView(StringRef SourceFile,
   Printer->closeViewFile(std::move(OS));
 }
 
 int CodeCoverageTool::run(Command Cmd, int argc, const char **argv) {
   cl::opt<std::string> CovFilename(
       cl::Positional, cl::desc("Covered executable or object file."));
 
   cl::list<std::string> CovFilenames(
-      "object", cl::desc("Coverage executable or object file"), cl::ZeroOrMore,
-      cl::CommaSeparated);
+      "object", cl::desc("Coverage executable or object file"), cl::ZeroOrMore);
+
+  cl::opt<bool> DebugDumpCollectedObjects(
+      "dump-collected-objects", cl::Optional, cl::Hidden,
+      cl::desc("Show the collected coverage object files"));
 
   cl::list<std::string> InputSourceFiles(
       cl::Positional, cl::desc("<Source files>"), cl::ZeroOrMore);
 
   cl::opt<bool> DebugDumpCollectedPaths(
       "dump-collected-paths", cl::Optional, cl::Hidden,
       cl::desc("Show the collected paths to source files"));
 
if (!CovFilename.empty())
       ObjectFilenames.emplace_back(CovFilename);
     for (const std::string &Filename : CovFilenames)
       ObjectFilenames.emplace_back(Filename);
     if (ObjectFilenames.empty()) {
       errs() << "No filenames specified!\n";
       ::exit(1);
     }
 
+    if (DebugDumpCollectedObjects) {
+      for (StringRef OF : ObjectFilenames)
+        outs() << OF << '\n';
+      ::exit(0);
+    }
+
     ViewOpts.Format = Format;
     switch (ViewOpts.Format) {
     case CoverageViewOptions::OutputFormat::Text:
       ViewOpts.Colors = UseColor == cl::BOU_UNSET
                             ? sys::Process::StandardOutHasColors()
                             : UseColor == cl::BOU_TRUE;
       break;
     case CoverageViewOptions::OutputFormat::HTML: