JDevlieghere wrote: > 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()`).
Is that a theoretical concern or actually something that happens (or will happen with your patches)? Looking at the existing code, it doesn't look like anyone is modifying the instance and I can remove the non-const overload and we still compile. If anything, we probably don't want anyone to change these things as we're iterating over them. > 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. While definitely unsafe an something I think we shouldn't do, that's a problem today too if someone were to modify the instance list from another thread or take the address of one of its elements. > 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. I'd like to avoid shared_pointers if possible, but we could achieve the same thing by storing unique pointers in the vector and handing out raw points like you suggested above. https://github.com/llvm/llvm-project/pull/132884 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
