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



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

Reply via email to