I totally agree with you, I didn't think about creating custom callbacks =P. I'll refactor the code accordingly. Thanks, man
Il giorno ven 2 ott 2020 alle ore 01:51 Pavel Labath <pa...@labath.sk> ha scritto: > On 01/10/2020 20:57, Walter wrote: > >> - 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. > > > > Very good remark. Probably we'll end up doing that when we start > > implementing reverse debugging. The tricky thing we'll need to solve in > > the future is seamlessly transition between a trace and a live process. > > For example, when reverse debugging a live process, you might be stopped > > at a breakpoint in the trace, then you do "continue", it reaches the end > > of the trace, and then it resumes the live process. I still haven't > > decided what we'll propose for this, but probably we'll have to change a > > lot of the current code to make that happen nicely. > > Yes, that will be interesting. Even more interesting will be the > question of how to communicate to the user the fact that even though we > have "unwound" the PC to the previous location, variables and memory > still contain the "live" values. Particularly, once we support rr-style > reverse debugging which will "unwind" memory as well.. > > > > >> - 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? > > > > Well, there's definitely coupling, but there are two reasons. One is > > that the Trace of a live process won't need any parsing. Parsing is only > > used when loading a trace from a json file. That makes parsing one of > > the two main ways to create a Trace. And besides, I think that the > > single responsibility principle is good to follow. The Parser class does > > the parsing, and the Trace class holds an actual trace. > > Yeah, I'm all for the single responsibility principle, but the way it is > implemented gives me pause. Like, if the Parser is supposed to "create" > the Trace, then why does the code do it the other way around > (Trace::CreateParser)? For one, that means that it will be possible to > "parse" (by calling CreateParser()->...) any trace that you can get your > hands on, even that of a live process. Ideally, the Parser would be the > one creating a Trace object (in it's Parse function, or something), and > after the Trace is created, it should no longer be possible to obtain > the parser. All of this could be encapsulated in the "create_callback" > that the plugin registers, and so the fact that there is a parser class > involved would be completely unknown to the rest of the code. And then > there could be a different create_callback which would create a Trace > object suitable for tracing a live process... > > pl > -- - Walter ErquÃnigo Pezo
_______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
