clayborg added inline comments.

================
Comment at: lldb/include/lldb/Target/StackFrame.h:264
   ///     A pointer to a list of variables.
-  VariableList *GetVariableList(bool get_file_globals);
+  VariableList *GetVariableList(bool get_file_globals, Status *error_ptr);
 
----------------
labath wrote:
> Could this return `Expected<VariableList*>` ?
We don't have the equivalent of Expected in LLDB that uses a Status, so I opted 
for this as it was already being used. I started with Expected<VariableList*> 
but the code using it was messy and had conversions to llvm::Error and back to 
Status again. Also using an Expected with a pointer didn't look good and caused 
some bad looking code.


================
Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:4161
+  if (command) {
+    if (command->contains(" -gline-tables-only"))
+      return Status("-gline-tables-only enabled, no variable information is "
----------------
labath wrote:
> This isn't a particularly reliable way of detecting whether variable 
> information was emitted. For example a command line `clang -gline-tables-only 
> -g2` will in fact produce full debug info and `clang -g1` will not. Could we 
> make that determination based on the presence of actual variable DIEs in the 
> debug info? Perhaps query the index whether it knows of any (global) variable 
> or any type defined within the compile unit?
This function only gets called when there are no variables in a stack frame at 
all and checks for reasons why. So if "-gline-tables-only -g2" is used, this 
function won't get called if there were variables.

I planned to make a follow up patch that detected no variables in a compile 
uint by checking the compile unit's abbreviation set to see if it had any 
DW_TAG_variable or DW_TAG_formal_parameter definitions, and if there weren't 
any respond with "-gline-tables-only might be enabled....". 

If we see this option for sure and have the side affect of there being no 
variables, I would like to user the users know what they can do to fix things 
if at all possible. 


================
Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:4162-4163
+    if (command->contains(" -gline-tables-only"))
+      return Status("-gline-tables-only enabled, no variable information is "
+                    "available in debug info for this compile unit");
+  }
----------------
I could rephrase this as "-gline-tables-only detected in compiler flags, 
variable information might be been removed for this compile unit"?


================
Comment at: lldb/test/API/commands/frame/var/TestFrameVar.py:174-175
+        '''
+        self.build(dictionary={'CFLAGS_EXTRAS': '-gline-tables-only'},
+                   env={"RC_DEBUG_OPTIONS": "1"})
+        exe = self.getBuildArtifact("a.out")
----------------
labath wrote:
> Why not just pass `-grecord-command-line` in CFLAGS_EXTRAS? I think then you 
> should be able to remove @skipUnlessDarwin from this test...
I thought I tried that, but I was able to get this to work as suggested. I will 
change this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133164

_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to