[Lldb-commits] [PATCH] D89842: [lldb/DWARF] Add support for DW_OP_implicit_value

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added a comment.

+1 on everything Pavel said.



================
Comment at: lldb/source/Expression/DWARFExpression.cpp:856-867
+  const uint32_t len = opcodes.GetULEB128(&opcode_offset);
+  const void *data = opcodes.GetData(&opcode_offset, len);
+
+  if (!data) {
+    LLDB_LOG(log, "Evaluate_DW_OP_implicit_value: could not be read data");
+    return false;
+  }
----------------
labath wrote:
> I'm not sure this function is really complex enough to justify its existence. The actual code is pretty short and most of it is just argument lists and logging. I don't think the logging is very useful as the caller logs already, and the argument lists would go away if this were inlined.
Agreed, plus half of the arguments are unused. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89842