[Lldb-commits] [PATCH] D31172: Move stop info override callback code from ArchSpec into Process

[Zachary Turner via lldb-commits]

Thanks for the suggestions!  Not sure if I'll be able to make those changes
before tomorrow EOD, and after that I will be out for ~2 weeks, so if time
doesn't permit, I may not follow up with another patch until after I get
back.

On Tue, Mar 21, 2017 at 11:19 AM Jim Ingham via Phabricator <
reviews at reviews.llvm.org> wrote:

> jingham added a comment.
>
> None of this isn't modeling the situation particularly clearly, since we
> have "architecture specific modifications to general behaviors" that aren't
> coming in through plugins so that it would be easy to modify the behavior
> for new architectures. Instead, we're requiring new architectures to go in
> and edit supposedly generic code.  We put off that abstraction because it
> was unclear what we would need it to do, and because of that for the most
> part we just put these architecture specific behaviors inline in various
> places.  Then this one callback had nowhere to go but ArchSpec which is the
> only logical place for architecture specific code to accumulate.   It
> doesn't seem like we should gate cleaning up ArchSpec on that larger issue,
> however.
>
> Rather than introducing another file, it seems simpler to make this part
> of StopInfo, since it is logically modifying the StopInfo for a thread.  It
> isn't right there, but it's any worse, and StopInfo is closer to the
> machine (though so far mostly closer to the Platform) than Process should
> be.  And StopInfo's already have to know about pretty much all the details
> of Process/Thread/etc...  so you wouldn't be adding knowledge to them that
> isn't appropriate.
>
> I'd also suggest removing the "callback" name from it since it really
> isn't a general purpose callback, it is absolutely determined by the
> ArchSpec.  For instance if you were to use anything but the ARM one for ARM
> debugging you would break debugging.  It's not optional...  Instead I'd
> call it something like InvokeStopInfoArchOverride.  That will also make it
> clear that this is a target for gathering up into some "ArchSpec specific
> behaviors" if/when we get around to that.
>
>
> https://reviews.llvm.org/D31172
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20170321/dd5e2c52/attachment.html>