<div dir="ltr">I totally agree with you, I didn't think about creating custom callbacks =P. I'll refactor the code accordingly. Thanks, man</div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">Il giorno ven 2 ott 2020 alle ore 01:51 Pavel Labath <<a href="mailto:pavel@labath.sk">pavel@labath.sk</a>> ha scritto:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On 01/10/2020 20:57, Walter wrote:<br>
>> - I am surprised that it was not necessary to create a special process<br>
> plugin for this purpose. I have a feeling one will be necessary sooner<br>
> or later because of the need to customize the step/continue/etc. flows.<br>
> Currently, this will probably produce very bizarre if one tries to<br>
> execute those commands. The reason I'm bringing this up is because of<br>
> the `Target::GetTrace` method being added in the next patch. If there is<br>
> a trace-specific process class, then it might make sense for this class<br>
> to hold the trace object instead of adding the GetTrace method on every<br>
> Target object out there even though most targets will have that null. I<br>
> don't know if that will really be the case, but I think it's something<br>
> worth keeping in mind as you work on the subsequent patches.<br>
> <br>
> Very good remark. Probably we'll end up doing that when we start<br>
> implementing reverse debugging. The tricky thing we'll need to solve in<br>
> the future is seamlessly transition between a trace and a live process.<br>
> For example, when reverse debugging a live process, you might be stopped<br>
> at a breakpoint in the trace, then you do "continue", it reaches the end<br>
> of the trace, and then it resumes the live process. I still haven't<br>
> decided what we'll propose for this, but probably we'll have to change a<br>
> lot of the current code to make that happen nicely.<br>
<br>
Yes, that will be interesting. Even more interesting will be the<br>
question of how to communicate to the user the fact that even though we<br>
have "unwound" the PC to the previous location, variables and memory<br>
still contain the "live" values. Particularly, once we support rr-style<br>
reverse debugging which will "unwind" memory as well..<br>
<br>
> <br>
>> - I am puzzled by the TraceXXX vs. TraceXXXSettingsParser duality. The<br>
> two classes are very tightly coupled, so it's not clear to me what is<br>
> the advantage of separating it out this way (one could just move all the<br>
> relevant methods directly into the Trace class. What's the reason for<br>
> this design?<br>
> <br>
> Well, there's definitely coupling, but there are two reasons. One is<br>
> that the Trace of a live process won't need any parsing. Parsing is only<br>
> used when loading a trace from a json file. That makes parsing one of<br>
> the two main ways to create a Trace. And besides, I think that the<br>
> single responsibility principle is good to follow. The Parser class does<br>
> the parsing, and the Trace class holds an actual trace.<br>
<br>
Yeah, I'm all for the single responsibility principle, but the way it is<br>
implemented gives me pause. Like, if the Parser is supposed to "create"<br>
the Trace, then why does the code do it the other way around<br>
(Trace::CreateParser)? For one, that means that it will be possible to<br>
"parse" (by calling CreateParser()->...) any trace that you can get your<br>
hands on, even that of a live process. Ideally, the Parser would be the<br>
one creating a Trace object (in it's Parse function, or something), and<br>
after the Trace is created, it should no longer be possible to obtain<br>
the parser. All of this could be encapsulated in the "create_callback"<br>
that the plugin registers, and so the fact that there is a parser class<br>
involved would be completely unknown to the rest of the code. And then<br>
there could be a different create_callback which would create a Trace<br>
object suitable for tracing a live process...<br>
<br>
pl<br>
</blockquote></div><br clear="all"><div><br></div>-- <br><div dir="ltr" class="gmail_signature"><div dir="ltr"><div>- Walter Erquínigo Pezo<br><br></div></div></div>