[Lldb-commits] [lldb] 3bec33b - [trace] Replace TraceCursorUP with TraceCursorSP
Jakob Johnson via lldb-commits
lldb-commits at lists.llvm.org
Mon Aug 1 13:54:28 PDT 2022
Author: Jakob Johnson
Date: 2022-08-01T13:53:53-07:00
New Revision: 3bec33b16db11c67d43bda134520a2132ff606c9
URL: https://github.com/llvm/llvm-project/commit/3bec33b16db11c67d43bda134520a2132ff606c9
DIFF: https://github.com/llvm/llvm-project/commit/3bec33b16db11c67d43bda134520a2132ff606c9.diff
LOG: [trace] Replace TraceCursorUP with TraceCursorSP
The use of `std::unique_ptr` with `TraceCursor` adds unnecessary complexity to adding `SBTraceCursor` bindings
Specifically, since `TraceCursor` is an abstract class there's no clean
way to provide "deep clone" semantics for `TraceCursorUP` short of
creating a pure virtual `clone()` method (afaict).
After discussing with @wallace, we decided there is no strong reason to
favor wrapping `TraceCursor` with `std::unique_ptr` over `std::shared_ptr`, thus this diff
replaces all usages of `std::unique_ptr<TraceCursor>` with `std::shared_ptr<TraceCursor>`.
This sets the stage for future diffs to introduce `SBTraceCursor`
bindings in a more clean fashion.
Test Plan:
Differential Revision: https://reviews.llvm.org/D130925
Added:
Modified:
lldb/include/lldb/Target/Trace.h
lldb/include/lldb/Target/TraceCursor.h
lldb/include/lldb/Target/TraceDumper.h
lldb/include/lldb/lldb-forward.h
lldb/source/Commands/CommandObjectThread.cpp
lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h
lldb/source/Plugins/TraceExporter/ctf/CommandObjectThreadTraceExportCTF.cpp
lldb/source/Target/TraceDumper.cpp
Removed:
################################################################################
diff --git a/lldb/include/lldb/Target/Trace.h b/lldb/include/lldb/Target/Trace.h
index beae9e28417d..917e66992ad1 100644
--- a/lldb/include/lldb/Target/Trace.h
+++ b/lldb/include/lldb/Target/Trace.h
@@ -169,9 +169,9 @@ class Trace : public PluginInterface,
/// Get a \a TraceCursor for the given thread's trace.
///
/// \return
- /// A \a TraceCursorUP. If the thread is not traced or its trace
+ /// A \a TraceCursorSP. If the thread is not traced or its trace
/// information failed to load, an \a llvm::Error is returned.
- virtual llvm::Expected<lldb::TraceCursorUP>
+ virtual llvm::Expected<lldb::TraceCursorSP>
CreateNewCursor(Thread &thread) = 0;
/// Dump general info about a given thread's trace. Each Trace plug-in
diff --git a/lldb/include/lldb/Target/TraceCursor.h b/lldb/include/lldb/Target/TraceCursor.h
index 95b022331634..4e405aeaab7c 100644
--- a/lldb/include/lldb/Target/TraceCursor.h
+++ b/lldb/include/lldb/Target/TraceCursor.h
@@ -52,7 +52,7 @@ namespace lldb_private {
///
/// Sample usage:
///
-/// TraceCursorUP cursor = trace.GetTrace(thread);
+/// TraceCursorSP cursor = trace.GetTrace(thread);
///
/// for (; cursor->HasValue(); cursor->Next()) {
/// TraceItemKind kind = cursor->GetItemKind();
diff --git a/lldb/include/lldb/Target/TraceDumper.h b/lldb/include/lldb/Target/TraceDumper.h
index ada779990e07..ab3f77916751 100644
--- a/lldb/include/lldb/Target/TraceDumper.h
+++ b/lldb/include/lldb/Target/TraceDumper.h
@@ -93,7 +93,7 @@ class TraceDumper {
///
/// \param[in] options
/// Additional options for configuring the dumping.
- TraceDumper(lldb::TraceCursorUP &&cursor_up, Stream &s,
+ TraceDumper(lldb::TraceCursorSP cursor_sp, Stream &s,
const TraceDumperOptions &options);
/// Dump \a count instructions of the thread trace starting at the current
@@ -114,7 +114,7 @@ class TraceDumper {
/// Create a trace item for the current position without symbol information.
TraceItem CreatRawTraceItem();
- lldb::TraceCursorUP m_cursor_up;
+ lldb::TraceCursorSP m_cursor_sp;
TraceDumperOptions m_options;
std::unique_ptr<OutputWriter> m_writer_up;
};
diff --git a/lldb/include/lldb/lldb-forward.h b/lldb/include/lldb/lldb-forward.h
index c51e1850338f..fd88a45ba06f 100644
--- a/lldb/include/lldb/lldb-forward.h
+++ b/lldb/include/lldb/lldb-forward.h
@@ -420,7 +420,7 @@ typedef std::weak_ptr<lldb_private::ThreadPlan> ThreadPlanWP;
typedef std::shared_ptr<lldb_private::ThreadPlanTracer> ThreadPlanTracerSP;
typedef std::shared_ptr<lldb_private::Trace> TraceSP;
typedef std::unique_ptr<lldb_private::TraceExporter> TraceExporterUP;
-typedef std::unique_ptr<lldb_private::TraceCursor> TraceCursorUP;
+typedef std::shared_ptr<lldb_private::TraceCursor> TraceCursorSP;
typedef std::shared_ptr<lldb_private::Type> TypeSP;
typedef std::weak_ptr<lldb_private::Type> TypeWP;
typedef std::shared_ptr<lldb_private::TypeCategoryImpl> TypeCategoryImplSP;
diff --git a/lldb/source/Commands/CommandObjectThread.cpp b/lldb/source/Commands/CommandObjectThread.cpp
index fe0cb0945cde..9aa128aaa288 100644
--- a/lldb/source/Commands/CommandObjectThread.cpp
+++ b/lldb/source/Commands/CommandObjectThread.cpp
@@ -2269,17 +2269,17 @@ class CommandObjectTraceDumpInstructions : public CommandObjectParsed {
m_options.m_dumper_options.id = m_last_id;
}
- llvm::Expected<TraceCursorUP> cursor_or_error =
+ llvm::Expected<TraceCursorSP> cursor_or_error =
m_exe_ctx.GetTargetSP()->GetTrace()->CreateNewCursor(*thread_sp);
if (!cursor_or_error) {
result.AppendError(llvm::toString(cursor_or_error.takeError()));
return false;
}
- TraceCursorUP &cursor_up = *cursor_or_error;
+ TraceCursorSP &cursor_sp = *cursor_or_error;
if (m_options.m_dumper_options.id &&
- !cursor_up->HasId(*m_options.m_dumper_options.id)) {
+ !cursor_sp->HasId(*m_options.m_dumper_options.id)) {
result.AppendError("invalid instruction id\n");
return false;
}
@@ -2295,10 +2295,10 @@ class CommandObjectTraceDumpInstructions : public CommandObjectParsed {
// We need to stop processing data when we already ran out of instructions
// in a previous command. We can fake this by setting the cursor past the
// end of the trace.
- cursor_up->Seek(1, TraceCursor::SeekType::End);
+ cursor_sp->Seek(1, TraceCursor::SeekType::End);
}
- TraceDumper dumper(std::move(cursor_up),
+ TraceDumper dumper(std::move(cursor_sp),
out_file ? *out_file : result.GetOutputStream(),
m_options.m_dumper_options);
diff --git a/lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp b/lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
index f3f0a513e3fa..18b15f25b1ab 100644
--- a/lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
+++ b/lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
@@ -176,12 +176,12 @@ Expected<Optional<uint64_t>> TraceIntelPT::FindBeginningOfTimeNanos() {
return storage.beginning_of_time_nanos;
}
-llvm::Expected<lldb::TraceCursorUP>
+llvm::Expected<lldb::TraceCursorSP>
TraceIntelPT::CreateNewCursor(Thread &thread) {
if (Expected<DecodedThreadSP> decoded_thread = Decode(thread)) {
if (Expected<Optional<uint64_t>> beginning_of_time =
FindBeginningOfTimeNanos())
- return std::make_unique<TraceCursorIntelPT>(
+ return std::make_shared<TraceCursorIntelPT>(
thread.shared_from_this(), *decoded_thread, m_storage.tsc_conversion,
*beginning_of_time);
else
diff --git a/lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h b/lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h
index 7f2c3f8dda5d..b104a8a2581e 100644
--- a/lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h
+++ b/lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h
@@ -71,7 +71,7 @@ class TraceIntelPT : public Trace {
llvm::StringRef GetSchema() override;
- llvm::Expected<lldb::TraceCursorUP> CreateNewCursor(Thread &thread) override;
+ llvm::Expected<lldb::TraceCursorSP> CreateNewCursor(Thread &thread) override;
void DumpTraceInfo(Thread &thread, Stream &s, bool verbose,
bool json) override;
diff --git a/lldb/source/Plugins/TraceExporter/ctf/CommandObjectThreadTraceExportCTF.cpp b/lldb/source/Plugins/TraceExporter/ctf/CommandObjectThreadTraceExportCTF.cpp
index 384abd2166df..df8da3967fc6 100644
--- a/lldb/source/Plugins/TraceExporter/ctf/CommandObjectThreadTraceExportCTF.cpp
+++ b/lldb/source/Plugins/TraceExporter/ctf/CommandObjectThreadTraceExportCTF.cpp
@@ -81,7 +81,7 @@ bool CommandObjectThreadTraceExportCTF::DoExecute(Args &command,
return false;
} else {
auto do_work = [&]() -> Error {
- Expected<TraceCursorUP> cursor = trace_sp->CreateNewCursor(*thread);
+ Expected<TraceCursorSP> cursor = trace_sp->CreateNewCursor(*thread);
if (!cursor)
return cursor.takeError();
TraceHTR htr(*thread, **cursor);
diff --git a/lldb/source/Target/TraceDumper.cpp b/lldb/source/Target/TraceDumper.cpp
index 872530b657a1..74f6ea0ffa52 100644
--- a/lldb/source/Target/TraceDumper.cpp
+++ b/lldb/source/Target/TraceDumper.cpp
@@ -295,32 +295,32 @@ CreateWriter(Stream &s, const TraceDumperOptions &options, Thread &thread) {
new OutputWriterCLI(s, options, thread));
}
-TraceDumper::TraceDumper(lldb::TraceCursorUP &&cursor_up, Stream &s,
+TraceDumper::TraceDumper(lldb::TraceCursorSP cursor_sp, Stream &s,
const TraceDumperOptions &options)
- : m_cursor_up(std::move(cursor_up)), m_options(options),
+ : m_cursor_sp(std::move(cursor_sp)), m_options(options),
m_writer_up(CreateWriter(
- s, m_options, *m_cursor_up->GetExecutionContextRef().GetThreadSP())) {
+ s, m_options, *m_cursor_sp->GetExecutionContextRef().GetThreadSP())) {
if (m_options.id)
- m_cursor_up->GoToId(*m_options.id);
+ m_cursor_sp->GoToId(*m_options.id);
else if (m_options.forwards)
- m_cursor_up->Seek(0, TraceCursor::SeekType::Beginning);
+ m_cursor_sp->Seek(0, TraceCursor::SeekType::Beginning);
else
- m_cursor_up->Seek(0, TraceCursor::SeekType::End);
+ m_cursor_sp->Seek(0, TraceCursor::SeekType::End);
- m_cursor_up->SetForwards(m_options.forwards);
+ m_cursor_sp->SetForwards(m_options.forwards);
if (m_options.skip) {
- m_cursor_up->Seek((m_options.forwards ? 1 : -1) * *m_options.skip,
+ m_cursor_sp->Seek((m_options.forwards ? 1 : -1) * *m_options.skip,
TraceCursor::SeekType::Current);
}
}
TraceDumper::TraceItem TraceDumper::CreatRawTraceItem() {
TraceItem item = {};
- item.id = m_cursor_up->GetId();
+ item.id = m_cursor_sp->GetId();
if (m_options.show_timestamps)
- item.timestamp = m_cursor_up->GetWallClockTime();
+ item.timestamp = m_cursor_sp->GetWallClockTime();
return item;
}
@@ -378,7 +378,7 @@ CalculateDisass(const TraceDumper::SymbolInfo &symbol_info,
}
Optional<lldb::user_id_t> TraceDumper::DumpInstructions(size_t count) {
- ThreadSP thread_sp = m_cursor_up->GetExecutionContextRef().GetThreadSP();
+ ThreadSP thread_sp = m_cursor_sp->GetExecutionContextRef().GetThreadSP();
SymbolInfo prev_symbol_info;
Optional<lldb::user_id_t> last_id;
@@ -386,32 +386,32 @@ Optional<lldb::user_id_t> TraceDumper::DumpInstructions(size_t count) {
ExecutionContext exe_ctx;
thread_sp->GetProcess()->GetTarget().CalculateExecutionContext(exe_ctx);
- for (size_t insn_seen = 0; insn_seen < count && m_cursor_up->HasValue();
- m_cursor_up->Next()) {
+ for (size_t insn_seen = 0; insn_seen < count && m_cursor_sp->HasValue();
+ m_cursor_sp->Next()) {
- last_id = m_cursor_up->GetId();
+ last_id = m_cursor_sp->GetId();
TraceItem item = CreatRawTraceItem();
- if (m_cursor_up->IsEvent()) {
+ if (m_cursor_sp->IsEvent()) {
if (!m_options.show_events)
continue;
- item.event = m_cursor_up->GetEventType();
+ item.event = m_cursor_sp->GetEventType();
switch (*item.event) {
case eTraceEventCPUChanged:
- item.cpu_id = m_cursor_up->GetCPU();
+ item.cpu_id = m_cursor_sp->GetCPU();
break;
case eTraceEventHWClockTick:
- item.hw_clock = m_cursor_up->GetHWClock();
+ item.hw_clock = m_cursor_sp->GetHWClock();
break;
case eTraceEventDisabledHW:
case eTraceEventDisabledSW:
break;
}
- } else if (m_cursor_up->IsError()) {
- item.error = m_cursor_up->GetError();
+ } else if (m_cursor_sp->IsError()) {
+ item.error = m_cursor_sp->GetError();
} else {
insn_seen++;
- item.load_address = m_cursor_up->GetLoadAddress();
+ item.load_address = m_cursor_sp->GetLoadAddress();
if (!m_options.raw) {
SymbolInfo symbol_info;
@@ -429,7 +429,7 @@ Optional<lldb::user_id_t> TraceDumper::DumpInstructions(size_t count) {
}
m_writer_up->TraceItem(item);
}
- if (!m_cursor_up->HasValue())
+ if (!m_cursor_sp->HasValue())
m_writer_up->NoMoreData();
return last_id;
}
More information about the lldb-commits
mailing list