[PATCH] D66526: WIP: [utils] Add the llvm-locstats tool

[Vedant Kumar via Phabricator via llvm-commits]

vsk added inline comments.


================
Comment at: llvm/utils/llvm-locstats/llvm-locstats.py:167
+  # Get the llvm-dwarfdump --statistics and write it into the temproary file.
+    tmp_stats_file = TemporaryFile(mode='w+b')
+    call([llvm_dwarfdump_bin, "--statistics", binary], stdout=tmp_stats_file)
----------------
Still not sure why this needs to touch the filesystem. Why not use a pipe? Like:

subproc = Popen(cmd, stdin=PIPE, stdout=PIPE, stderr=PIPE, universal_newlines = True)
cmd_stdout, cmd_stderr = subproc.communicate()


================
Comment at: llvm/utils/llvm-locstats/llvm-locstats.py:213
+            variables_100_cov = \
+                json_parsed['vars with 100% of its scope covered']
+        else:
----------------
I think it'd be great to simplify this. Note that "1-9%" is covered, as is "11-19%", but that "10%" is missed. It should be possible to write a helper to get the buckets, like:

```
def coverage_buckets():
  yield '0%'
  for start in range(1, 92, 10):
    yield '{0}%-{1}%'.format(start, start + 9)
  yield '100%'
```

You can use a helper like that to use a for-loop to extract all of the "vars with $bucket of its scope covered" fields, etc.


================
Comment at: llvm/utils/llvm-locstats/llvm-locstats.py:254
+              json_parsed['vars (excluding the debug entry values) '
+                          'with 100% of its scope covered']
+    elif results.only_formal_parameters:
----------------
Ditto here and everywhere else we need to look up coverage buckets within json_parsed.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66526/new/

https://reviews.llvm.org/D66526