labath added a comment. It seems to me that the DataEncoder class is a lot more complicated than what is necessary for our existing use cases. All of them involve creating a new buffer (either empty, or with some pre-existing content), modifying it, and passing it on. A lot of the DataEncoder complexity (`IsDynamic`, `UpdateMemberVariables`, ...) stems from the fact that it wants to support writing into unowned buffers, functionality that we don't even use.
If we removed that, we could make this code a lot simpler. See inline comments for one way to achieve that. ================ Comment at: lldb/include/lldb/Utility/DataEncoder.h:79-80 + /// A size of an address in bytes. \see PutAddress DataEncoder(void *data, uint32_t data_length, lldb::ByteOrder byte_order, uint8_t addr_size); ---------------- Take `const void*data` here and make a copy for internal use inside the constructor. ================ Comment at: lldb/include/lldb/Utility/DataEncoder.h:283-291 + /// Return if this object has a dynamic buffer that can be appended to. + /// + /// The buffer is dynamic if the "m_data_sp" is valid. "m_data_sp" is only set + /// by constructors mentioned below. + /// + /// \see DataEncoder(lldb::ByteOrder byte_order, uint8_t addr_size); + bool IsDynamic() const { ---------------- delete ================ Comment at: lldb/include/lldb/Utility/DataEncoder.h:309-314 + /// Update the m_start and m_end after appending data. /// - /// \param[in] offset - /// The offset into \a data_sp at which the subset starts. - /// - /// \param[in] length - /// The length in bytes of the subset of \a data_sp. - /// - /// \return - /// The number of bytes that this object now contains. - uint32_t SetData(const lldb::DataBufferSP &data_sp, uint32_t offset = 0, - uint32_t length = UINT32_MAX); + /// Any of the member functions that append data to the end of the owned + /// data should call this function after appending data to update the required + /// member variables. + void UpdateMemberVariables(); ---------------- delete, along with the member variables it updates. ================ Comment at: lldb/source/Expression/DWARFExpression.cpp:463-465 // So first we copy the data into a heap buffer std::unique_ptr<DataBufferHeap> head_data_up( new DataBufferHeap(m_data.GetDataStart(), m_data.GetByteSize())); ---------------- delete, and create data encoder from `m_data.GetDataStart()` directly Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D115073/new/ https://reviews.llvm.org/D115073 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits