[Lldb-commits] [PATCH] D52689: [LLDB] - Add support for DW_FORM_implicit_const.

[Greg Clayton via Phabricator via lldb-commits]

clayborg requested changes to this revision.
clayborg added a comment.
This revision now requires changes to proceed.

A few things inlined. Very close. DumpAttribute will need to take a DWARFFormValue in order to dump the value correctly.



================
Comment at: source/Plugins/SymbolFile/DWARF/DWARFAbbreviationDeclaration.h:43-44
+    dw_form_t form;
+    DWARFFormValue::ValueType val;
+    m_attributes[idx].get(attr, form, val);
+    form_value.SetForm(form);
----------------
It would be nice to be able to access the dw_form_t and DWARFFormValue::ValueType within "form_value" without having to create a temp variables here for both "form" and "val". Fine to add accessors that return references for the form_t and DWARFFormValue::ValueType to DWARFFormValue.


================
Comment at: source/Plugins/SymbolFile/DWARF/DWARFAbbreviationDeclaration.h:45-46
+    m_attributes[idx].get(attr, form, val);
+    form_value.SetForm(form);
+    form_value.SetValue(val);
   }
----------------
If we are able to follow my last inline comment, then these go away.


================
Comment at: source/Plugins/SymbolFile/DWARF/DWARFAttribute.h:23
+  DWARFAttribute(dw_attr_t attr, dw_form_t form,
+                 DWARFFormValue::ValueType value)
+      : m_attr(attr), m_form(form), m_value(value) {}
----------------
Do we need to use a "DWARFFormValue::ValueType" here? Right now we only need a int64_t and DWARFFormValue::ValueType is larger than that. It does future proof us a bit and there aren't all that many abbreviations, even in a large DWARF file. Just thinking out loud here


================
Comment at: source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp:630-631
 
           DumpAttribute(dwarf2Data, cu, debug_info_data, &offset, s, attr,
-                        form);
+                        form_value.Form());
         }
----------------
DumpAttribute will need to take the full form_value in order to dump DW_FORM_implicit_const forms correctly. Change DumpAttribute to take a "form_value"


https://reviews.llvm.org/D52689