[llvm-locstats][NFC] Support OOP concept

[djtodoro]

Summary

Making these changes, the code becomes more robust and easier for adding the new features.

1. Introduce the LocationStats class representing the statistics
2. Add the pretty_print() method in the LocationStats class
3. Add additional '-' for the program options
4. Add the verify_program_inputs() function
5. Add the parse_locstats() function
6. Rename 'results' => 'opts'
7. Add more comments

[dstenb]

Seems like a good refactoring. Two minor inline comments.

llvm/utils/llvm-locstats/llvm-locstats.py

33

Should this be a return instead now that this code is moved to a utility function?

76–78

an -> a

[djtodoro]

Thanks for the comments! Sorry for the late response, I was on the vacation.

[djtodoro]

-Addressing comments

llvm/docs/CommandGuide/llvm-locstats.rst

 location information in all code section bytes (where the variable or parameter
 is in the scope). The line [50%,60%) shows the number and the percentage of DIEs
 where the location information is between 50 and 60 percentage of its scope
 covered.
 
 OPTIONS
 -------
 
-.. option:: -only-variables
+.. option:: --only-variables
 
-            Calculate the location statistics only for local variables.
+            calculate the location statistics only for local variables
 
-.. option:: -only-formal-parameters
+.. option:: --only-formal-parameters
 
-            Calculate the location statistics only for formal parameters.
+            calculate the location statistics only for formal parameters
 
-.. option:: -ignore-debug-entry-values
+.. option:: --ignore-debug-entry-values
 
-            Ignore the location statistics on locations containing the
-            debug entry values DWARF operation.
+            ignore the location statistics on locations containing the
+            debug entry values DWARF operation
 
 EXIT STATUS
 -----------
 
 :program:`llvm-locstats` returns 0 if the input file were parsed
 successfully. Otherwise, it returns 1.
 
 OUTPUT EXAMPLE

llvm/utils/llvm-locstats/llvm-locstats.py

 #!/usr/bin/env python
 #
 # This is a tool that works like debug location coverage calculator.
 # It parses the llvm-dwarfdump --statistics output by reporting it
 # in a more human readable way.
 #
 
 from __future__ import print_function
 import argparse
 import os
 import sys
 from json import loads
 from math import ceil
 from subprocess import Popen, PIPE
 
-def coverage_buckets():
-  yield '0%'
-  yield '(0%,10%)'
-  for start in range(10, 91, 10):
-    yield '[{0}%,{1}%)'.format(start, start + 10)
-  yield '100%'
-
-def locstats_output(
-  variables_total,
-  variables_total_locstats,
-  variables_with_loc,
-  scope_bytes_covered,
-  scope_bytes,
-  variables_coverage_map
-  ):
-
-  if scope_bytes == 0:
+# Holds the debug location statistics.
+class LocationStats:
+  def __init__(self, file_name, variables_total, variables_total_locstats,
+    variables_with_loc, variables_scope_bytes_covered, variables_scope_bytes,
+    variables_coverage_map):
+    self.file_name = file_name
+    self.variables_total = variables_total
+    self.variables_total_locstats = variables_total_locstats
+    self.variables_with_loc = variables_with_loc
+    self.scope_bytes_covered = variables_scope_bytes_covered
+    self.scope_bytes = variables_scope_bytes
+    self.variables_coverage_map = variables_coverage_map
+
+  # Pretty print the debug location buckets.
+  def pretty_print(self):
+    if self.scope_bytes == 0:
       print ('No scope bytes found.')
-    sys.exit(0)
+      return -1
 
-  pc_ranges_covered = int(ceil(scope_bytes_covered * 100.0)
-              / scope_bytes)
+    pc_ranges_covered = int(ceil(self.scope_bytes_covered * 100.0) \
+                / self.scope_bytes)
     variables_coverage_per_map = {}
     for cov_bucket in coverage_buckets():
       variables_coverage_per_map[cov_bucket] = \
-      int(ceil(variables_coverage_map[cov_bucket] * 100.0) \
-               / variables_total_locstats)
+        int(ceil(self.variables_coverage_map[cov_bucket] * 100.0) \
+                 / self.variables_total_locstats)
 
     print (' =================================================')
     print ('            Debug Location Statistics       ')
     print (' =================================================')
     print ('     cov%           samples         percentage(~)  ')
     print (' -------------------------------------------------')
     for cov_bucket in coverage_buckets():
       print ('   {0:10}     {1:8d}              {2:3d}%'. \
-      format(cov_bucket, variables_coverage_map[cov_bucket], \
+        format(cov_bucket, self.variables_coverage_map[cov_bucket], \
                variables_coverage_per_map[cov_bucket]))
     print (' =================================================')
     print (' -the number of debug variables processed: ' \
-    + str(variables_total_locstats))
+      + str(self.variables_total_locstats))
     print (' -PC ranges covered: ' + str(pc_ranges_covered) + '%')
 
     # Only if we are processing all the variables output the total
     # availability.
-  if variables_total and variables_with_loc:
-    total_availability = int(ceil(variables_with_loc * 100.0) \
-                                  / variables_total)
+    if self.variables_total and self.variables_with_loc:
+      total_availability = int(ceil(self.variables_with_loc * 100.0) \
+                                    / self.variables_total)
       print (' -------------------------------------------------')
       print (' -total availability: ' + str(total_availability) + '%')
     print (' =================================================')
 
-def parse_program_args(parser):
-  parser.add_argument('-only-variables', action='store_true',
-            default=False,
-            help='calculate the location statistics only for '
-               'local variables'
-            )
-  parser.add_argument('-only-formal-parameters', action='store_true',
-            default=False,
-            help='calculate the location statistics only for '
-               'formal parameters'
-            )
-  parser.add_argument('-ignore-debug-entry-values', action='store_true',
-            default=False,
-            help='ignore the location statistics on locations with '
-               'entry values'
-            )
-  parser.add_argument('file_name', type=str, help='file to process')
-  return parser.parse_args()
-
-
-def Main():
-  parser = argparse.ArgumentParser()
-  results = parse_program_args(parser)
-
-  if len(sys.argv) < 2:
-    print ('error: Too few arguments.')
-    parser.print_help()
-    sys.exit(1)
+    return 0
 
-  if results.only_variables and results.only_formal_parameters:
-    print ('error: Please use just one only* option.')
-    parser.print_help()
-    sys.exit(1)
+# Define the location buckets.
+def coverage_buckets():
+  yield '0%'
+  yield '(0%,10%)'
+  for start in range(10, 91, 10):
+    yield '[{0}%,{1}%)'.format(start, start + 10)
+  yield '100%'
 
+# Parse the JSON representing the debug statistics, and create a
+# LocationStats object.
+def parse_locstats(opts, binary):
   # These will be different due to different options enabled.
   variables_total = None
   variables_total_locstats = None
   variables_with_loc = None
   variables_scope_bytes_covered = None
   variables_scope_bytes = None
   variables_scope_bytes_entry_values = None
   variables_coverage_map = {}
-  binary = results.file_name
 
   # Get the directory of the LLVM tools.
   llvm_dwarfdump_cmd = os.path.join(os.path.dirname(__file__), \
                                     "llvm-dwarfdump")
   # The statistics llvm-dwarfdump option.
   llvm_dwarfdump_stats_opt = "--statistics"
 
+  # Generate the stats with the llvm-dwarfdump.
   subproc = Popen([llvm_dwarfdump_cmd, llvm_dwarfdump_stats_opt, binary], \
                   stdin=PIPE, stdout=PIPE, stderr=PIPE, \
                   universal_newlines = True)
   cmd_stdout, cmd_stderr = subproc.communicate()
 
   # Get the JSON and parse it.
   json_parsed = None
 
   try:
     json_parsed = loads(cmd_stdout)
   except:
     print ('error: No valid llvm-dwarfdump statistics found.')
     sys.exit(1)
 
-  if results.only_variables:
+  if opts.only_variables:
     # Read the JSON only for local variables.
     variables_total_locstats = \
       json_parsed['total vars procesed by location statistics']
     variables_scope_bytes_covered = \
       json_parsed['vars scope bytes covered']
     variables_scope_bytes = \
       json_parsed['vars scope bytes total']
-    if not results.ignore_debug_entry_values:
+    if not opts.ignore_debug_entry_values:
       for cov_bucket in coverage_buckets():
         cov_category = "vars with {} of its scope covered".format(cov_bucket)
         variables_coverage_map[cov_bucket] = json_parsed[cov_category]
     else:
       variables_scope_bytes_entry_values = \
         json_parsed['vars entry value scope bytes covered']
       variables_scope_bytes_covered = variables_scope_bytes_covered \
          - variables_scope_bytes_entry_values
       for cov_bucket in coverage_buckets():
         cov_category = \
           "vars (excluding the debug entry values) " \
           "with {} of its scope covered".format(cov_bucket)
         variables_coverage_map[cov_bucket] = json_parsed[cov_category]
-  elif results.only_formal_parameters:
+  elif opts.only_formal_parameters:
     # Read the JSON only for formal parameters.
     variables_total_locstats = \
       json_parsed['total params procesed by location statistics']
     variables_scope_bytes_covered = \
       json_parsed['formal params scope bytes covered']
     variables_scope_bytes = \
       json_parsed['formal params scope bytes total']
-    if not results.ignore_debug_entry_values:
+    if not opts.ignore_debug_entry_values:
       for cov_bucket in coverage_buckets():
         cov_category = "params with {} of its scope covered".format(cov_bucket)
         variables_coverage_map[cov_bucket] = json_parsed[cov_category]
     else:
       variables_scope_bytes_entry_values = \
         json_parsed['formal params entry value scope bytes covered']
       variables_scope_bytes_covered = variables_scope_bytes_covered \
         - variables_scope_bytes_entry_values
       for cov_bucket in coverage_buckets():
         cov_category = \
           "params (excluding the debug entry values) " \
           "with {} of its scope covered".format(cov_bucket)
         variables_coverage_map[cov_bucket] = json_parsed[cov_category]
   else:
     # Read the JSON for both local variables and formal parameters.
     variables_total = \
       json_parsed['source variables']
     variables_with_loc = json_parsed['variables with location']
     variables_total_locstats = \
       json_parsed['total variables procesed by location statistics']
     variables_scope_bytes_covered = \
       json_parsed['scope bytes covered']
     variables_scope_bytes = \
       json_parsed['scope bytes total']
-    if not results.ignore_debug_entry_values:
+    if not opts.ignore_debug_entry_values:
       for cov_bucket in coverage_buckets():
         cov_category = "variables with {} of its scope covered". \
                        format(cov_bucket)
         variables_coverage_map[cov_bucket] = json_parsed[cov_category]
     else:
       variables_scope_bytes_entry_values = \
         json_parsed['entry value scope bytes covered']
       variables_scope_bytes_covered = variables_scope_bytes_covered \
         - variables_scope_bytes_entry_values
       for cov_bucket in coverage_buckets():
         cov_category = "variables (excluding the debug entry values) " \
                        "with {} of its scope covered". format(cov_bucket)
         variables_coverage_map[cov_bucket] = json_parsed[cov_category]
 
+  return LocationStats(binary, variables_total, variables_total_locstats,
+                       variables_with_loc, variables_scope_bytes_covered,
+                       variables_scope_bytes, variables_coverage_map)
+
+# Parse the program arguments.
+def parse_program_args(parser):
+  parser.add_argument('--only-variables', action='store_true', default=False,
+            help='calculate the location statistics only for local variables')
+  parser.add_argument('--only-formal-parameters', action='store_true',
+            default=False,
+            help='calculate the location statistics only for formal parameters')
+  parser.add_argument('--ignore-debug-entry-values', action='store_true',
+            default=False,
+            help='ignore the location statistics on locations with '
+                 'entry values')
+  parser.add_argument('file_name', type=str, help='file to process')
+
+  return parser.parse_args()
+
+# Verify that the program inputs meet the requirements.
+def verify_program_inputs(opts):
+  if len(sys.argv) < 2:
+    print ('error: Too few arguments.')
+    return False
+
+  if opts.only_variables and opts.only_formal_parameters:
+    print ('error: Please use just one --only* option.')
+    return False
+
+  return True
+
+def Main():
+  parser = argparse.ArgumentParser()
+  opts = parse_program_args(parser)
+
+  if not verify_program_inputs(opts):
+    parser.print_help()
+    sys.exit(1)
+
+  binary = opts.file_name
+  locstats = parse_locstats(opts, binary)
+
   # Pretty print collected info.
-  locstats_output(
-    variables_total,
-    variables_total_locstats,
-    variables_with_loc,
-    variables_scope_bytes_covered,
-    variables_scope_bytes,
-    variables_coverage_map
-    )
+  if locstats.pretty_print() == -1:
+    sys.exit(0)
 
 if __name__ == '__main__':
   Main()
   sys.exit(0)