grimar added inline comments.

================
Comment at: source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp:1072-1079
+  if (at_ranges_val != DW_INVALID_OFFSET) {
+    if (DWARFDebugRangesBase *debug_ranges = dwarf2Data->DebugRanges()) {
+
+      dw_offset_t debug_ranges_offset;
+      if (form_value.Form() == DW_FORM_rnglistx)
+        debug_ranges_offset = debug_ranges->GetOffset(at_ranges_val);
+      else
----------------
clayborg wrote:
> Can/should we do all this work when we extract the form value so that 
> "form_value.Unsigned()" just returns the right thing? If not, every place 
> that gets DW_AT_ranges attribute would need to do this.
Doing everything inside `DWARFFormValue::ExtractValue` would make the callers 
code simpler indeed,
but my concern is that it would mean that instead of the attribute value 
requested it would return the offset value read from `.debug_rnglists` section. 
I am not sure it is good idea to read any debug section content at that low 
level. 

I think 'ExtractValue` ideally should not know about the debug sections. And 
also that would not be consistent with the other forms it reads. I am not sure 
if we might want to know the raw value of the `DW_FORM_rnglistx` one day, but 
with such change, we will lose such possibility.

I would suggest making a helper function instead, so all the callers can use 
it. I did it in this patch, what do you think?


https://reviews.llvm.org/D53929



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

Reply via email to