[Lldb-commits] [PATCH] D88841: [intel pt] Refactor parsing

This revision was not accepted when it landed; it landed in state "Needs Review". This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGea1f49741ec4: [intel pt] Refactor parsing (authored by Walter

[Lldb-commits] [PATCH] D88841: [intel pt] Refactor parsing

labath accepted this revision. labath added a comment. Looks good. In D88841#2320410 , @wallace wrote: > - Cannot delete Trace() {}, as there’s a compilation error. Could be because > some constructors have been marked as deleted, so the compiler wants

[Lldb-commits] [PATCH] D88841: [intel pt] Refactor parsing

wallace updated this revision to Diff 297075. wallace added a comment. - Added “trace schema all”, following the pattern from "trace all", and added a test for this. - Created a non-static version of GetSchema in Trace.h - Cannot delete Trace() {}, as there’s a compilation error. Could be

[Lldb-commits] [PATCH] D88841: [intel pt] Refactor parsing

clayborg requested changes to this revision. clayborg added inline comments. This revision now requires changes to proceed. Comment at: lldb/include/lldb/Core/PluginManager.h:342 + static const char *GetTraceSchema(ConstString name); + This is fine if we have

[Lldb-commits] [PATCH] D88841: [intel pt] Refactor parsing

labath added a comment. I like this. A lot of comments, but they're all cosmetic. Comment at: lldb/include/lldb/Target/Trace.h:96-98 protected: Trace() {} delete completely. As this is an abstract class, making the constructor protected does not do

[Lldb-commits] [PATCH] D88841: [intel pt] Refactor parsing

wallace updated this revision to Diff 296562. wallace marked 10 inline comments as done. wallace added a comment. Address issues based on this diff's comments and from conversations with Greg. - I'm still keeping the TraceSessionFile name instead of TraceSettings, as later there'll be actual

[Lldb-commits] [PATCH] D88841: [intel pt] Refactor parsing

labath added inline comments. Comment at: lldb/source/Plugins/Trace/intel-pt/ThreadIntelPT.cpp:20 + +ThreadIntelPT::~ThreadIntelPT() {} + just `=default`, or even don't declare it all (as explained in the other thread, these declarations are completely

[Lldb-commits] [PATCH] D88841: [intel pt] Refactor parsing

clayborg requested changes to this revision. clayborg added inline comments. This revision now requires changes to proceed. Comment at: lldb/include/lldb/Target/Trace.h:77 /// - /// \param[in] settings - /// JSON object describing a trace. + /// \param[in]

[Lldb-commits] [PATCH] D88841: [intel pt] Refactor parsing

wallace created this revision. wallace added reviewers: clayborg, labath. Herald added subscribers: lldb-commits, mgorny. Herald added a reviewer: JDevlieghere. Herald added a project: LLDB. wallace requested review of this revision. With the feedback I was getting in different diffs, I realized