[Lldb-commits] [PATCH] D98996: Teach DWARFExpression about DWARF 4+ Location Descriptions

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added inline comments.


================
Comment at: lldb/source/Expression/DWARFExpression.cpp:992
+    case DW_OP_regx:
+      dwarf4_location_description_kind = Register;
+      break;
----------------
Could you return the kind and do `LocationDescriptionKind dwarf4_location_description_kind = classifyLocationDescription`? That way you can make it a static instead of a lambda in a function that's already a hundreds of lines long. 


================
Comment at: lldb/source/Expression/DWARFExpression.cpp:1007
     const uint8_t op = opcodes.GetU8(&offset);
+    classifyLocationDescription(op);
 
----------------
That also makes it more explicit that this is updating the `dwarf4_location_description_kind`


================
Comment at: lldb/source/Expression/DWARFExpression.cpp:2636
+    switch (dwarf4_location_description_kind) {
+    case Empty: // FIXME: This would be suspicious. Add some error handling.
+      LLDB_LOGF(log, log_msg, "Empty");
----------------
lldb_assert?


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

https://reviews.llvm.org/D98996