[Lldb-commits] [PATCH] D104422: [trace] Add a TraceCursor class

[Greg Clayton via Phabricator via lldb-commits]

clayborg added inline comments.


================
Comment at: lldb/include/lldb/Target/TraceCursor.h:39-40
+///
+///  The Trace initially points to a dummy invalid instruction error signaling
+///  the end of a trace, similar to a C++ collections' end iterator.
+///
----------------
Should the TraceCursor point to the last instruction more like a begin() STL call?


================
Comment at: lldb/include/lldb/Target/TraceCursor.h:87
+  /// Force the cursor to point to the end of the trace.
+  virtual void ResetToEnd() = 0;
+
----------------
SeekToEnd()?


================
Comment at: lldb/include/lldb/Target/TraceCursor.h:90
+  /// Force the cursor to point to the first (i.e. oldest) item of the trace.
+  virtual void ResetToBegin() = 0;
+
----------------
SeekToStart()? SeekToBegin()?


================
Comment at: lldb/include/lldb/Target/TraceCursor.h:116-117
+  /// \return
+  ///     The size in bytes of the instruction opcode the cursor is pointing at.
+  ///     If the cursor points to an error in the trace, return \b 0.
+  virtual size_t GetInstructionSize() = 0;
----------------
Do we even need the instruction size in this cursor class interface? Clients can easily grab an instruction from GetLoadAddress() and do that themselves?


================
Comment at: lldb/include/lldb/Target/TraceCursor.h:130
+  /// The stop ID when the cursor was created.
+  ssize_t m_stop_id = -1;
+  /// The trace that owns this cursor.
----------------
Zero is the invalid stop ID.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D104422/new/

https://reviews.llvm.org/D104422