[Lldb-commits] [PATCH] D119963: [LLDB] Dump valid ranges of variables

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

I suppose one could ask the question whether this should be printing the entire set range of ranges, or just the one



================
Comment at: lldb/source/Commands/Options.td:960
     "target modules.">;
+  def target_modules_lookup_variables_ranges : Option<"show-variables-ranges", 
+    "R">, GroupRange<1, 6>, Desc<"Dump valid ranges of variables (must be used "
----------------



================
Comment at: lldb/source/Commands/Options.td:961
+  def target_modules_lookup_variables_ranges : Option<"show-variables-ranges", 
+    "R">, GroupRange<1, 6>, Desc<"Dump valid ranges of variables (must be used "
+    "in conjunction with --verbose">;
----------------
A not widely known fact: It is possible to produce an long option without a short version by using an non-printable character in place of the short option. You can see and example of that in `disassemble --force`, defined in this file.

If we are considering this a niche option, then maybe we don't need to use a random character just to have a short version.


================
Comment at: lldb/test/Shell/SymbolFile/DWARF/x86/debug_loc.s:16
+# CHECK-LABEL: image lookup -v -a 0 -R
+# CHECK: Variable: {{.*}}, name = "x0", type = "int", location = DW_OP_reg5 RDI, decl = , valid ranges =
+# CHECK: Variable: {{.*}}, name = "x1", type = "int", location = <empty>, decl = , valid ranges =
----------------
I don't think that the information itself is out of place here, but I think it can get confusing when one sees it like this, next to the `location` field. So, if we had an output like `location = RDI, valid ranges = [0, 20)`, I think one would/could read it as "the variable is in RDI from PC=0 to PC=20", but what it _really_ is telling you is "the variable (or its value -- I'm not sure) exists between PC=0 and PC=20, *and* for the exact PC that you are querying (0 in this case), it's value happens to be in RDI".

We could try to come up with a way to make this less confusing, but maybe we could just sidestep this problem and just print this information in the `image dump symfile` output.


================
Comment at: lldb/test/Shell/SymbolFile/DWARF/x86/debug_loc.s:27
 # CHECK:   Function{
 # CHECK:     Variable{{.*}}, name = "x0", {{.*}}, scope = parameter, location =
 # CHECK-NEXT:  [0x0000000000000000, 0x0000000000000001): DW_OP_reg5 RDI
----------------
If you printed the information here, then there would be no (or less) confusion as the location field contains explicit range information. Additionally, the `image dump symfile` output is so verbose, than an extra field does not make a difference.

(I'm sorry, I know it was I who suggested using `image lookup`, but I did not realize you would be adding an extra argument because of that.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119963