[Lldb-commits] [lldb] [lldb] Remove raw access to PluginInstances vector (PR #132884)

[David Peixotto via lldb-commits]

dmpots wrote:

> Did you consider having something like `GetEnabledInstances` that returns a vector with just the enabled instances? It seems like that would be a lot less intrusive and I'd be surprised if this code is hot enough that the overhead of creating a new vector instead of returning a reference makes a difference.

We did consider this approach. There are some drawbacks beyond the performance issue, but the tradeoff may be worth it.

If we define the method like: `std::vector<PluginInstance> GetEnabledInstances()` then we are returning copies of the PluginInstance. Anything that modifies the internal state of the `PluginInstance` will be lost since it is operating on a copy (e.g. `GetEnabledInstances().front().DoSomethingToModifyInternalState()`).

We could also return pointers to the plugin instances like`std::vector<PluginInstance*> GetEnabledInstances()`. But these would be pointers to the locations in the vector stored in the `PluginInstance` class. So if a new plugin is added to the PluginInstances list the pointers could be invalidated. Probably not an issue with the current code, but could be unsafe to hand out those references in general.

I suppose another way would be to keep a `std::vector<shared_ptr<PluginInstance>>` and then creating a copy would not be an issue since any updates would also modify the original object.

Let me know what you think.

https://github.com/llvm/llvm-project/pull/132884