[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

diff --git a/lldb/include/lldb/Target/Trace.h b/lldb/include/lldb/Target/Trace.h
index beae9e2..917e669 100644 (file)
--- a/lldb/include/lldb/Target/Trace.h
+++ b/lldb/include/lldb/Target/Trace.h
@@ -169,9 +169,9 @@ public:
   /// 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 95b0223..4e405ae 100644 (file)
--- 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 ada7799..ab3f779 100644 (file)
--- a/lldb/include/lldb/Target/TraceDumper.h
+++ b/lldb/include/lldb/Target/TraceDumper.h
@@ -93,7 +93,7 @@ public:
   ///
   /// \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 @@ private:
   /// 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 c51e185..fd88a45 100644 (file)
--- 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 fe0cb09..9aa128a 100644 (file)
--- a/lldb/source/Commands/CommandObjectThread.cpp
+++ b/lldb/source/Commands/CommandObjectThread.cpp
@@ -2269,17 +2269,17 @@ protected:
       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 @@ protected:
       // 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 f3f0a51..18b15f2 100644 (file)
--- 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 7f2c3f8..b104a8a 100644 (file)
--- a/lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h
+++ b/lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h
@@ -71,7 +71,7 @@ public:
 
   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 384abd2..df8da39 100644 (file)
--- 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 872530b..74f6ea0 100644 (file)
--- 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;
 }