bulbazord created this revision. bulbazord added reviewers: wallace, jj10306, persona0220. Herald added a project: All. bulbazord requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits.
The way it works now, it stores a `const char *` that it does not explicitly own. It's owned by the ConstString StringPool. This is purely to manage its lifetime, we don't really benefit from deduplication (nor should we try to, they are errors). We also don't really benefit from quick comparisons. This may make the size of TraceItemStorage larger, but you have to pay the cost of owning the data somewhere. The ConstString StringPool is an attractive choice but ultimately a poor one. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D152326 Files: lldb/include/lldb/Target/TraceCursor.h lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp lldb/source/Plugins/Trace/intel-pt/DecodedThread.h lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.h Index: lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.h =================================================================== --- lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.h +++ lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.h @@ -28,7 +28,7 @@ bool HasValue() const override; - const char *GetError() const override; + llvm::StringRef GetError() const override; lldb::addr_t GetLoadAddress() const override; Index: lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp =================================================================== --- lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp +++ lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp @@ -95,7 +95,7 @@ return m_decoded_thread_sp->GetItemKindByIndex(m_pos); } -const char *TraceCursorIntelPT::GetError() const { +llvm::StringRef TraceCursorIntelPT::GetError() const { return m_decoded_thread_sp->GetErrorByIndex(m_pos); } Index: lldb/source/Plugins/Trace/intel-pt/DecodedThread.h =================================================================== --- lldb/source/Plugins/Trace/intel-pt/DecodedThread.h +++ lldb/source/Plugins/Trace/intel-pt/DecodedThread.h @@ -159,7 +159,7 @@ /// \return /// The error associated with a given trace item. - const char *GetErrorByIndex(uint64_t item_index) const; + llvm::StringRef GetErrorByIndex(uint64_t item_index) const; /// \return /// The trace item kind given an item index. @@ -275,7 +275,7 @@ lldb::TraceEvent event; /// The string message of this item if it's an error - const char *error; + std::string error; }; /// Create a new trace item. Index: lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp =================================================================== --- lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp +++ lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp @@ -186,14 +186,12 @@ } void DecodedThread::AppendError(const IntelPTError &error) { - CreateNewTraceItem(lldb::eTraceItemKindError).error = - ConstString(error.message()).AsCString(); + CreateNewTraceItem(lldb::eTraceItemKindError).error = error.message(); m_error_stats.RecordError(/*fatal=*/false); } void DecodedThread::AppendCustomError(StringRef err, bool fatal) { - CreateNewTraceItem(lldb::eTraceItemKindError).error = - ConstString(err).AsCString(); + CreateNewTraceItem(lldb::eTraceItemKindError).error = err.str(); m_error_stats.RecordError(fatal); } @@ -238,7 +236,9 @@ return static_cast<lldb::TraceItemKind>(m_item_kinds[item_index]); } -const char *DecodedThread::GetErrorByIndex(uint64_t item_index) const { +llvm::StringRef DecodedThread::GetErrorByIndex(uint64_t item_index) const { + if (item_index >= m_item_data.size()) + return llvm::StringRef(); return m_item_data[item_index].error; } Index: lldb/include/lldb/Target/TraceCursor.h =================================================================== --- lldb/include/lldb/Target/TraceCursor.h +++ lldb/include/lldb/Target/TraceCursor.h @@ -217,7 +217,7 @@ /// \return /// The error message the cursor is pointing at. - virtual const char *GetError() const = 0; + virtual llvm::StringRef GetError() const = 0; /// \return /// Whether the cursor points to an event or not.
Index: lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.h =================================================================== --- lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.h +++ lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.h @@ -28,7 +28,7 @@ bool HasValue() const override; - const char *GetError() const override; + llvm::StringRef GetError() const override; lldb::addr_t GetLoadAddress() const override; Index: lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp =================================================================== --- lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp +++ lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp @@ -95,7 +95,7 @@ return m_decoded_thread_sp->GetItemKindByIndex(m_pos); } -const char *TraceCursorIntelPT::GetError() const { +llvm::StringRef TraceCursorIntelPT::GetError() const { return m_decoded_thread_sp->GetErrorByIndex(m_pos); } Index: lldb/source/Plugins/Trace/intel-pt/DecodedThread.h =================================================================== --- lldb/source/Plugins/Trace/intel-pt/DecodedThread.h +++ lldb/source/Plugins/Trace/intel-pt/DecodedThread.h @@ -159,7 +159,7 @@ /// \return /// The error associated with a given trace item. - const char *GetErrorByIndex(uint64_t item_index) const; + llvm::StringRef GetErrorByIndex(uint64_t item_index) const; /// \return /// The trace item kind given an item index. @@ -275,7 +275,7 @@ lldb::TraceEvent event; /// The string message of this item if it's an error - const char *error; + std::string error; }; /// Create a new trace item. Index: lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp =================================================================== --- lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp +++ lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp @@ -186,14 +186,12 @@ } void DecodedThread::AppendError(const IntelPTError &error) { - CreateNewTraceItem(lldb::eTraceItemKindError).error = - ConstString(error.message()).AsCString(); + CreateNewTraceItem(lldb::eTraceItemKindError).error = error.message(); m_error_stats.RecordError(/*fatal=*/false); } void DecodedThread::AppendCustomError(StringRef err, bool fatal) { - CreateNewTraceItem(lldb::eTraceItemKindError).error = - ConstString(err).AsCString(); + CreateNewTraceItem(lldb::eTraceItemKindError).error = err.str(); m_error_stats.RecordError(fatal); } @@ -238,7 +236,9 @@ return static_cast<lldb::TraceItemKind>(m_item_kinds[item_index]); } -const char *DecodedThread::GetErrorByIndex(uint64_t item_index) const { +llvm::StringRef DecodedThread::GetErrorByIndex(uint64_t item_index) const { + if (item_index >= m_item_data.size()) + return llvm::StringRef(); return m_item_data[item_index].error; } Index: lldb/include/lldb/Target/TraceCursor.h =================================================================== --- lldb/include/lldb/Target/TraceCursor.h +++ lldb/include/lldb/Target/TraceCursor.h @@ -217,7 +217,7 @@ /// \return /// The error message the cursor is pointing at. - virtual const char *GetError() const = 0; + virtual llvm::StringRef GetError() const = 0; /// \return /// Whether the cursor points to an event or not.
_______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits