vsk added a comment.
@wallace thanks for winnowing the test objects. I left an inline suggestion
about simplifying the error-handling in IntelPTInstruction. Other than that,
mechanically this is looking good. Thanks!
================
Comment at: lldb/source/Plugins/Trace/intel-pt/DecodedThread.h:36
+
+ IntelPTInstruction(const pt_insn &pt_insn, int libipt_error_code = 0)
+ : m_pt_insn(pt_insn), m_libipt_error_code(libipt_error_code),
----------------
Does this IntelPTInstruction constructor ever get called with a non-zero libipt
error code?
================
Comment at: lldb/source/Plugins/Trace/intel-pt/DecodedThread.h:40
+
+ IntelPTInstruction(int libipt_error_code)
+ : m_pt_insn(), m_libipt_error_code(libipt_error_code), m_custom_error()
{}
----------------
It might be a bit cleaner to just have two IntelPTInstruction constructors: one
that accepts a `const pt_insn &` and another that accepts an `llvm::Error`. To
do that, you'd need to introduce a PT-specific ErrorInfo (`class IntelPTError :
public ErrorInfo<IntelPTError> { ... }`). This can wrap some sort of generic
error (as a std::string, perhaps), or a libipt-specific error.
It'd be a little more up front work, but the benefit is that it simplifies
error handling (there's just one type of error, only one error value to check
in IsError, etc).
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D89283/new/
https://reviews.llvm.org/D89283
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
