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

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

In https://reviews.llvm.org/D31172#808105, @labath wrote:

> In https://reviews.llvm.org/D31172#808038, @clayborg wrote:
>
> > Added a suggestion to make Target::GetArchitecturePlugin lazy and to ask if the arch is supported. Most of the time we will set this once and never need to change it, even when we attach, change arch, etc. The new suggestion will allow us to use the same arch plug-in without re-fetching it. Let me know what you think as the changes are thrown out there to see what people's opinions are.
>
>
> Well... I don't feel strongly about it, but it's not my preferred solution (the IsSupported() call feels hacky). But since we're already throwing ideas out there, here are two of mine:
>
> - confine all the changes to m_arch to the SetArchitecture method. Right now only one of the assignments to m_arch is done outside of this method, and that one can be easily converted to a call to SetArchitecture(). This should make sure the two values never get out of sync.


That would be fine too.

> - (a more extreme version of the first idea) store the ArchSpec inside the architecture plugin (then use m_architecture_up->GetArchSpec() instead of m_arch). This will make it impossible for the values to go out of sync, as we will have a single source of truth. (We will need a default arch plugin for cases where we don't have a specific plugin)

It would need to be the other way around. Store the Architecture plug-in inside of the ArchSpec since we only have the "arm" Architecture plug-in and architectures don't need to make one of these unless the arch has special things it needs to do.

I am fine with either approach and fine not doing that approach I suggested. I just don't want this to ever get out of sync. So maybe the easiest way it to ask the ArchSpec object for it's architecture plug-in? If there is no state maintained in these plug-ins, we might just have a single instance of each Architecture plug-in that everyone uses (across targets)? Then everyone doesn't need to hold onto a shared/unique_ptr and can just fetch it from the Plugin manager each time?


https://reviews.llvm.org/D31172