[PATCH] D85705: Add a "Trace" plug-in to LLDB to add process trace support in stages.

Pavel Labath via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Oct 1 07:11:58 PDT 2020


labath added a comment.

I have two high-level remarks/questions about this:

- I am surprised that it was not necessary to create a special process plugin for this purpose. I have a feeling one will be necessary sooner or later because of the need to customize the step/continue/etc. flows. Currently, this will probably produce very bizarre if one tries to execute those commands. The reason I'm bringing this up is because of the `Target::GetTrace` method being added in the next patch. If there is a trace-specific process class, then it might make sense for this class to hold the trace object instead of adding the GetTrace method on every Target object out there even though most targets will have that null. I don't know if that will really be the case, but I think it's something worth keeping in mind as you work on the subsequent patches.
- I am puzzled by the TraceXXX vs. TraceXXXSettingsParser duality. The two classes are very tightly coupled, so it's not clear to me what is the advantage of separating it out this way (one could just move all the relevant methods directly into the Trace class. What's the reason for this design?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85705



More information about the llvm-commits mailing list