labath added a comment.

In D88769#2314164 <https://reviews.llvm.org/D88769#2314164>, @labath wrote:

> 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've now read the description of D88841 <https://reviews.llvm.org/D88841>, 
which kind of answers that. The reason I am not sure of this design is that I 
fear this will require a fairly big API surface between `ProcessTrace` and 
`Trace` classes in order to implement the more complicated commands like 
reverse-step. It also adds a bunch of dependency restrictions:

- `Trace` class (being a core class) should not know anything about 
ProcessTrace (a plugin) -- this bit could be removed if we make ProcessTrace a 
non-plugin
- `ProcessTrace` (being a generic plugin serving multiple traces) should not 
know anything about any specific trace plugin

OTOH, if ProcessTraceXXX is an internal affair of the TraceXXX plugin, then the 
two classes can freely cooperate (within reason), and can use any interfaces 
they see fit. And the TraceYYY plugin can use a completely different interface 
to communicate with its process class.

However, I don't think your design is bad, if you can pull it off, so feel free 
to carry on like you were planning. I'd just encourage you to, as you're 
working on it, to think about whether things can be made simpler with an 
different organization of things.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88769

_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to