wallace requested changes to this revision. wallace added a comment. This revision now requires changes to proceed.
there are many comments from the previous versions of this diff that you didn't apply. Go through all of them first :) ================ Comment at: lldb/source/Plugins/Trace/intel-pt/DecodedThread.h:88-91 + // /// \return + // /// An \a llvm::Error object if this class corresponds to an Error, or an + // /// \a llvm::Error::success otherwise. + // llvm::Error ToError() const; ---------------- delete it. We don't want to leave old code as comments ================ Comment at: lldb/source/Plugins/Trace/intel-pt/DecodedThread.h:122 llvm::Optional<uint64_t> m_timestamp; - std::unique_ptr<llvm::ErrorInfoBase> m_error; + bool is_error; }; ---------------- now that you changed this, could show share in the description of this diff the difference in byte size between the old and new code when tracing the same number of instructions? ================ Comment at: lldb/source/Plugins/Trace/intel-pt/DecodedThread.h:152 + /// The error of the trace. + llvm::Error GetError(uint64_t ins_idx); + ---------------- or GetErrorByInstructionIndex ================ Comment at: lldb/source/Plugins/Trace/intel-pt/DecodedThread.h:148-151 + /// Get the error at some instruction index from the decoded trace. + /// + /// \return + /// The error of the trace. ---------------- wallace wrote: > we can make the documentation clearer you didn't apply these changes ================ Comment at: lldb/source/Plugins/Trace/intel-pt/DecodedThread.h:157 + + /// Append an error of instruction decoding. + void AppendError(llvm::Error err); ---------------- wallace wrote: > same here Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122293/new/ https://reviews.llvm.org/D122293 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
