[Lldb-commits] [PATCH] D88769: [trace] Scaffold "thread trace dump instructions"

Tue Oct 6 06:01:01 PDT 2020

labath added a comment.

In D88769#2312482 <https://reviews.llvm.org/D88769#2312482>, @wallace wrote:

> - Comments on the architecture of classes are in that diff

Where would that be? I couldn't find anything that would answer my question from the previous comment?

The reason I want to know that is that if each `TraceXXX` "plugin" is supposed to have/use/need a `ProcessTraceXXX` plugin, then I'd rather organize them such that they are closer together. OTOH, if ProcessTrace is supposed to work with all kinds of traces, then the current design makes perfect sense (though I am having trouble imagining how would that work).

> - I was able to get rid of cross-plug-in dependencies except for RegisterContextHistory. The class is very simple, so I could move it to lldb/Core. What do you think?

IIUC, the only place which uses it the PT trace plugin. That is ok. Some plugin-to-plugin dependencies (though not cycles, ideally) are fine, particularly for something called "utility". That said, I don't think that moving this class to the core libraries would be a particularly bad choice either...

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88769