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

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

Reply via email to