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

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In https://reviews.llvm.org/D31172#808177, @clayborg wrote:

> In https://reviews.llvm.org/D31172#808105, @labath wrote:
>
> > - 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.


I've tried that, and eventually decided against it, as it would create an uncomfortable mutual recursion between SetArchitecture and SetExecutableModule.

> 
> 
>> - (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.

We can't have ArchSpec vend the plugin, as that would cause ArchSpec to depend on the whole world again (which is what we are trying to remove by this). I don't see why the plugin could not hold the ArchSpec -- for architectures which don't need special processing, we could just have a default dumb "plugin" that does nothing but vend the ArchSpec.

The alternative would be to have a third object which holds both ArchSpec and the plugin, and whose only job is to make sure these two are in sync -- it would basically be a glorified std::pair with a custom assignment operator. I think this would actually be a fairly clean solution -- the only problem I have is that I don't know how to name the new class.  I'll upload it so you can see how it looks like.


https://reviews.llvm.org/D31172