wallace marked 9 inline comments as done.
wallace added inline comments.
================
Comment at: lldb/source/Commands/CommandObjectThread.cpp:2111
bool m_create_repeat_command_just_invoked;
- size_t m_consecutive_repetitions = 0;
+ std::map<const Thread *, std::unique_ptr<TraceInstructionDumper>> m_dumpers;
};
----------------
clayborg wrote:
> "Thread *" isn't a stable thing to use, I would suggest using the "tid" of
> the thread. Any reason you are making a map of thread to unique pointers?
> Can't this just be:
> ```
> std::map<lldb::tid_t, TraceInstructionDumper> m_dumpers;
> ```
I would suggest using the "tid" of the thread -> makes sense
The cursor is a Unique pointer, which renders the TraceInstructionDumper not
copyable.
================
Comment at: lldb/source/Target/TraceInstructionDumper.cpp:22
+TraceInstructionDumper::TraceInstructionDumper(lldb::TraceCursorUP &&cursor_up,
+ bool forwards, size_t skip,
+ bool raw)
----------------
clayborg wrote:
> Seems like the "forwards" and "skip" parameter shouldn't be in this class at
> all? Just setup the cursor before giving it to this class?
the direction is useful to knowing whether to call Next or Prev in the
iterations
the skip is also useful to have inside, because that way it's easier to know if
all the data has been consumed by the skip. That's stored in the m_no_more_data
flag. When a trace has been fully consumed, it's still pointing to the last
instruction, so another variable is needed to prevent more iterations
================
Comment at: lldb/source/Target/TraceInstructionDumper.cpp:35
+ if (m_forwards) {
+ if (!m_cursor_up->Next()) {
+ m_no_more_data = true;
----------------
clayborg wrote:
> Should we modify the TraceCursor API for Next(...) to be able to take a
> count? Seems like that might be much more efficient than recursively calling
> this function?
that's a pretty good idea
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D105531/new/
https://reviews.llvm.org/D105531
_______________________________________________
lldb-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
