clayborg requested changes to this revision.
clayborg added a comment.
This revision now requires changes to proceed.
Very close! Just a few nits.
================
Comment at: lldb/include/lldb/Target/TraceCursor.h:101
+ /// Set the direction to use in the \a TraceCursor::Move() method.
+ void SetDirection(Direction direction);
+
----------------
If there is no other direction other than forward to backward, should this just
be a bool? Then we won't have to add the enum to the public API when we expose
this.
================
Comment at: lldb/include/lldb/Target/TraceCursor.h:105
+ /// The current direction of the cursor.
+ TraceCursor::Direction GetDirection();
+
----------------
bool? Fine if you think the enum should be used.
================
Comment at: lldb/include/lldb/Target/TraceCursor.h:134-135
+ /// The number of matching instructions visited before stopping the
+ /// cursor. If stopped at an error, this count is increased by 1 to
+ /// account for the error.
+ virtual size_t Move(size_t count = 1) = 0;
----------------
If we get an error, we should return less than the requested amount. If you ask
for 10, and only get 7, but get an error, you don't want to return 8. So I
would vote for documentation and implementation like:
```
The number of successful moves that occurred. If the returned value is less
than \a count, then check for an error condition by calling XXX. If there is no
error and less than \a count was returned, then the cursor is at the end or
beginnning of the data.
```
================
Comment at: lldb/source/Target/TraceCursor.cpp:20-35
+ExecutionContextRef &TraceCursor::GetExecutionContextRef() {
+ return m_exe_ctx_ref;
+}
+
+void TraceCursor::SetGranularity(
+ lldb::TraceInstructionControlFlowType granularity) {
+ m_granularity = granularity;
----------------
All these can be moved to the header file. We don't do this in the public API,
but we can do this in private classes.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D105531/new/
https://reviews.llvm.org/D105531
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
