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

2021-06-24 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added a comment. In D104422#2839895 , @vsk wrote: > FTR, I'm quite happy with the new direction. Thanks! Thanks for your input. It helped us come up with something we thought made sense given all the input! Repository: rG LLVM Github Monore

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

2021-06-24 Thread Vedant Kumar via Phabricator via lldb-commits
vsk added a comment. FTR, I'm quite happy with the new direction. Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D104422/new/ https://reviews.llvm.org/D104422 ___ lldb-commits mailing list lldb-com

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

2021-06-23 Thread Phabricator via lldb-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG2aa1dd1c66dc: [trace] Add a TraceCursor class (authored by Walter Erquinigo ). Changed prior to commit: https://reviews.llvm.org/D

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

2021-06-23 Thread Greg Clayton via Phabricator via lldb-commits
clayborg accepted this revision. clayborg added a comment. This revision is now accepted and ready to land. Just need to initialize the m_stop_id in the cursor to the invalid value and this is good to go. Comment at: lldb/include/lldb/Target/TraceCursor.h:132 + /// The stop I

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

2021-06-18 Thread walter erquinigo via Phabricator via lldb-commits
wallace added inline comments. Comment at: lldb/include/lldb/Target/Trace.h:367 + /// We use 1 as a default stop id for post portem processes. + uint32_t m_stop_id = 1; clayborg wrote: > We really shouldn't have to set this manually. Probably best to start it

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

2021-06-18 Thread walter erquinigo via Phabricator via lldb-commits
wallace updated this revision to Diff 353135. wallace marked 3 inline comments as done. wallace added a comment. per comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D104422/new/ https://reviews.llvm.org/D104422 Files: lldb/include/lldb/Tar

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

2021-06-18 Thread Greg Clayton via Phabricator via lldb-commits
clayborg requested changes to this revision. clayborg added a comment. This revision now requires changes to proceed. Just a few nits! Comment at: lldb/include/lldb/Target/Trace.h:292-293 + /// \return + /// The stop ID of the live process being traced, or \b 1 if this is

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

2021-06-18 Thread walter erquinigo via Phabricator via lldb-commits
wallace updated this revision to Diff 353117. wallace marked an inline comment as done. wallace added a comment. address comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D104422/new/ https://reviews.llvm.org/D104422 Files: lldb/include/lldb

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

2021-06-18 Thread walter erquinigo via Phabricator via lldb-commits
wallace marked 2 inline comments as done. wallace added inline comments. Comment at: lldb/include/lldb/Target/Trace.h:292 + /// \return + /// The stop ID of the live process being traced, or \b 0 if this is a + /// post-mortem trace session. clayborg w

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

2021-06-18 Thread Greg Clayton via Phabricator via lldb-commits
clayborg requested changes to this revision. clayborg added a comment. This revision now requires changes to proceed. Just one issue regarding the stop ID for post mortem. Once this is resolved, we will be good to go! Comment at: lldb/include/lldb/Target/Trace.h:292 + /// \re

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

2021-06-17 Thread walter erquinigo via Phabricator via lldb-commits
wallace updated this revision to Diff 352872. wallace marked 2 inline comments as done. wallace added a comment. Herald added a subscriber: mgorny. address comments: - now simply pointing to the last instruction of the trace by default - removed the GetInstructionSize, as it's really not needed -

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

2021-06-17 Thread walter erquinigo via Phabricator via lldb-commits
wallace marked 11 inline comments as done. wallace 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 iter

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

2021-06-16 Thread 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 Tra

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

2021-06-16 Thread walter erquinigo via Phabricator via lldb-commits
wallace updated this revision to Diff 352605. wallace added a comment. - Simplified Trace.h by simply having lldb::TraceCursorUP GetCursor, as suggested. - Improved the documentation including a sample code. - Added methods to quickly move the trace to the beginning or the end. - Renamed TraceIns

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

2021-06-16 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added inline comments. Comment at: lldb/include/lldb/Target/Trace.h:224 + /// failed to load, an \a llvm::Error is returned. + virtual llvm::Expected GetThreadEndCursor( + Thread &thread, If the trace cursor contains an error, do we need llvm:

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

2021-06-16 Thread walter erquinigo via Phabricator via lldb-commits
wallace created this revision. wallace added reviewers: clayborg, vsk. wallace requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. As a follow up of D103588 , I'm reinitiating the discussion with a new proposa