Add enable/disable api for SystemRuntime plugins

This commit adds support for enabling and disabling plugins by name.
The changes are made generically in the `PluginInstances` class, but
currently we only expose the ability to SystemRuntime plugins. Other
plugins types can be added easily.

We had a few design goals for how disabled plugins should work

  1. Plugins that are disabled should still be visible to the system.
     This allows us to dynamically enable and disable plugins
     and report their state to the user.
  2. Plugin order should be stable across disable and enable changes.
     We want avoid changing the order of plugin lookup. When a plugin
     is re-enabled it should return to its original slot in the
     creation order.
  3. Disabled plugins should not appear in PluginManager operations.
     Clients should be able to assume that only enabled plugins will
     be returned from the PluginManager.

We explored two other ways to implement this change:

  1. Keep two separate lists and move the instances between them when
     enabling and disabling plugins. This requires maintaining the
     original insert order to satisfy the goals above and thus adds
     some complexity to the code.
  2. Return a copy of the instances list with disabled plugins removed
     when clients iterate over them. This requires extra copies and
     would potentially lead to the copies getting out of sync with
     the actual stored instances because any modifications to the
     instances would be lost.

Instead, we modify the plugin instance to maintain a bool of its enabled
state.  Clients external to Instances class expect to iterate over only
enabled instance so we skip over disabed instances in the query and
snapshot apis. This way the client does not have to manually check which
instances are enabled.

Author: dmpots

lldb/include/lldb/Core/PluginManager.h

@@ -22,6 +22,7 @@
 
 #include <cstddef>
 #include <cstdint>
+#include <vector>
 
 #define LLDB_PLUGIN_DEFINE_ADV(ClassName, PluginName)                          \
   namespace lldb_private {                                                     \
@@ -47,6 +48,12 @@ class CommandInterpreter;
 class Debugger;
 class StringList;
 
+struct RegisteredPluginInfo {
+  llvm::StringRef name = "";
+  llvm::StringRef description = "";
+  bool enabled = false;
+};
+
 class PluginManager {
 public:
   static void Initialize();
@@ -168,6 +175,12 @@ class PluginManager {
   static SystemRuntimeCreateInstance
   GetSystemRuntimeCreateCallbackAtIndex(uint32_t idx);
 
+  static std::vector<RegisteredPluginInfo> GetSystemRuntimePluginInfo();
+
+  // Modify the enabled state of a SystemRuntime plugin.
+  // Returns false if the plugin name is not found.
+  static bool SetSystemRuntimePluginEnabled(llvm::StringRef name, bool enabled);
+
   // ObjectFile
   static bool
   RegisterPlugin(llvm::StringRef name, llvm::StringRef description,

lldb/source/Core/PluginManager.cpp

@@ -188,11 +188,13 @@ template <typename Callback> struct PluginInstance {
   PluginInstance(llvm::StringRef name, llvm::StringRef description,
                  Callback create_callback,
                  DebuggerInitializeCallback debugger_init_callback = nullptr)
-      : name(name), description(description), create_callback(create_callback),
+      : name(name), description(description), enabled(true),
+        create_callback(create_callback),
         debugger_init_callback(debugger_init_callback) {}
 
   llvm::StringRef name;
   llvm::StringRef description;
+  bool enabled;
   Callback create_callback;
   DebuggerInitializeCallback debugger_init_callback;
 };
@@ -250,7 +252,9 @@ template <typename Instance> class PluginInstances {
   }
 
   void PerformDebuggerCallback(Debugger &debugger) {
-    for (auto &instance : m_instances) {
+    for (const auto &instance : m_instances) {
+      if (!instance.enabled)
+        continue;
       if (instance.debugger_init_callback)
         instance.debugger_init_callback(debugger);
     }
@@ -260,7 +264,14 @@ template <typename Instance> class PluginInstances {
   // Note that this is a copy of the internal state so modifications
   // to the returned instances will not be reflected back to instances
   // stored by the PluginInstances object.
-  std::vector<Instance> GetSnapshot() { return m_instances; }
+  std::vector<Instance> GetSnapshot() {
+    std::vector<Instance> enabled_instances;
+    for (const auto &instance : m_instances) {
+      if (instance.enabled)
+        enabled_instances.push_back(instance);
+    }
+    return enabled_instances;
+  }
 
   const Instance *GetInstanceAtIndex(uint32_t idx) {
     uint32_t count = 0;
@@ -280,12 +291,41 @@ template <typename Instance> class PluginInstances {
   const Instance *
   FindEnabledInstance(std::function<bool(const Instance &)> predicate) const {
     for (const auto &instance : m_instances) {
+      if (!instance.enabled)
+        continue;
       if (predicate(instance))
         return &instance;
     }
     return nullptr;
   }
 
+  // Return a list of all the registered plugin instances. This includes both
+  // enabled and disabled instances. The instances are listed in the order they
+  // were registered which is the order they would be queried if they were all
+  // enabled.
+  std::vector<RegisteredPluginInfo> GetPluginInfoForAllInstances() {
+    // Lookup the plugin info for each instance in the sorted order.
+    std::vector<RegisteredPluginInfo> plugin_infos;
+    plugin_infos.reserve(m_instances.size());
+    for (const Instance &instance : m_instances)
+      plugin_infos.push_back(
+          {instance.name, instance.description, instance.enabled});
+
+    return plugin_infos;
+  }
+
+  bool SetInstanceEnabled(llvm::StringRef name, bool enable) {
+    auto it = std::find_if(
+        m_instances.begin(), m_instances.end(),
+        [&](const Instance &instance) { return instance.name == name; });
+
+    if (it == m_instances.end())
+      return false;
+
+    it->enabled = enable;
+    return true;
+  }
+
 private:
   std::vector<Instance> m_instances;
 };
@@ -627,6 +667,15 @@ PluginManager::GetSystemRuntimeCreateCallbackAtIndex(uint32_t idx) {
   return GetSystemRuntimeInstances().GetCallbackAtIndex(idx);
 }
 
+std::vector<RegisteredPluginInfo> PluginManager::GetSystemRuntimePluginInfo() {
+  return GetSystemRuntimeInstances().GetPluginInfoForAllInstances();
+}
+
+bool PluginManager::SetSystemRuntimePluginEnabled(llvm::StringRef name,
+                                                  bool enable) {
+  return GetSystemRuntimeInstances().SetInstanceEnabled(name, enable);
+}
+
 #pragma mark ObjectFile
 
 struct ObjectFileInstance : public PluginInstance<ObjectFileCreateInstance> {

lldb/unittests/Core/CMakeLists.txt

@@ -7,6 +7,7 @@ add_lldb_unittest(LLDBCoreTests
   FormatEntityTest.cpp
   MangledTest.cpp
   ModuleSpecTest.cpp
+  PluginManagerTest.cpp
   ProgressReportTest.cpp
   RichManglingContextTest.cpp
   SourceLocationSpecTest.cpp