[lldb/Interpreter] Add requirements to Scripted Interface abstract methods

lldb/include/lldb/Interpreter/Interfaces/ScriptedInterface.h

@@ -31,7 +31,22 @@ class ScriptedInterface {
     return m_object_instance_sp;
   }
 
-  virtual llvm::SmallVector<llvm::StringLiteral> GetAbstractMethods() const = 0;
+  struct AbstractMethodRequirement {
+    llvm::StringLiteral name;
+    size_t min_arg_count = 0;
+  };
+
+  virtual llvm::SmallVector<AbstractMethodRequirement>
+  GetAbstractMethodRequirements() const = 0;
+
+  llvm::SmallVector<llvm::StringLiteral> const GetAbstractMethods() const {
+    llvm::SmallVector<llvm::StringLiteral> abstract_methods;
+    llvm::transform(GetAbstractMethodRequirements(), abstract_methods.begin(),
+                    [](const AbstractMethodRequirement &requirement) {
+                      return requirement.name;
+                    });
+    return abstract_methods;
+  }
 
   template <typename Ret>
   static Ret ErrorWithMessage(llvm::StringRef caller_name,

lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/OperatingSystemPythonInterface.h

@@ -31,8 +31,9 @@ class OperatingSystemPythonInterface
                      StructuredData::DictionarySP args_sp,
                      StructuredData::Generic *script_obj = nullptr) override;
 
-  llvm::SmallVector<llvm::StringLiteral> GetAbstractMethods() const override {
-    return llvm::SmallVector<llvm::StringLiteral>({"get_thread_info"});
+  llvm::SmallVector<AbstractMethodRequirement>
+  GetAbstractMethodRequirements() const override {
+    return llvm::SmallVector<AbstractMethodRequirement>({{"get_thread_info"}});
   }
 
   StructuredData::DictionarySP CreateThread(lldb::tid_t tid,

lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedPlatformPythonInterface.h

@@ -29,10 +29,13 @@ class ScriptedPlatformPythonInterface : public ScriptedPlatformInterface,
                      StructuredData::DictionarySP args_sp,
                      StructuredData::Generic *script_obj = nullptr) override;
 
-  llvm::SmallVector<llvm::StringLiteral> GetAbstractMethods() const override {
-    return llvm::SmallVector<llvm::StringLiteral>(
-        {"list_processes", "attach_to_process", "launch_process",
-         "kill_process"});
+  llvm::SmallVector<AbstractMethodRequirement>
+  GetAbstractMethodRequirements() const override {
+    return llvm::SmallVector<AbstractMethodRequirement>(
+        {{"list_processes"},
+         {"attach_to_process", 2},
+         {"launch_process", 2},
+         {"kill_process", 2}});
   }
 
   StructuredData::DictionarySP ListProcesses() override;

lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedProcessPythonInterface.h

@@ -31,9 +31,12 @@ class ScriptedProcessPythonInterface : public ScriptedProcessInterface,
                      StructuredData::DictionarySP args_sp,
                      StructuredData::Generic *script_obj = nullptr) override;
 
-  llvm::SmallVector<llvm::StringLiteral> GetAbstractMethods() const override {
-    return llvm::SmallVector<llvm::StringLiteral>(
-        {"read_memory_at_address", "is_alive", "get_scripted_thread_plugin"});
+  llvm::SmallVector<AbstractMethodRequirement>
+  GetAbstractMethodRequirements() const override {
+    return llvm::SmallVector<AbstractMethodRequirement>(
+        {{"read_memory_at_address", 4},
+         {"is_alive"},
+         {"get_scripted_thread_plugin"}});
   }
 
   StructuredData::DictionarySP GetCapabilities() override;

lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedPythonInterface.h

@@ -36,37 +36,78 @@ class ScriptedPythonInterface : virtual public ScriptedInterface {
     eNotImplemented,
     eNotAllocated,
     eNotCallable,
+    eUnknownArgumentCount,
+    eInvalidArgumentCount,
     eValid
   };
 
-  llvm::Expected<std::map<llvm::StringLiteral, AbstractMethodCheckerCases>>
+  struct AbstrackMethodCheckerPayload {
+
+    struct InvalidArgumentCountPayload {
+      InvalidArgumentCountPayload(size_t required, size_t actual)
+          : required_argument_count(required), actual_argument_count(actual) {}
+
+      size_t required_argument_count;
+      size_t actual_argument_count;
+    };
+
+    AbstractMethodCheckerCases checker_case;
+    std::variant<std::monostate, InvalidArgumentCountPayload> payload;
+  };
+
+  llvm::Expected<std::map<llvm::StringLiteral, AbstrackMethodCheckerPayload>>
   CheckAbstractMethodImplementation(
       const python::PythonDictionary &class_dict) const {
 
     using namespace python;
 
-    std::map<llvm::StringLiteral, AbstractMethodCheckerCases> checker;
-#define SET_ERROR_AND_CONTINUE(method_name, error)                             \
+    std::map<llvm::StringLiteral, AbstrackMethodCheckerPayload> checker;
+#define SET_CASE_AND_CONTINUE(method_name, case)                               \
   {                                                                            \
-    checker[method_name] = error;                                              \
+    checker[method_name] = {case, {}};                                         \
     continue;                                                                  \
   }
 
-    for (const llvm::StringLiteral &method_name : GetAbstractMethods()) {
+    for (const AbstractMethodRequirement &requirement :
+         GetAbstractMethodRequirements()) {
+      llvm::StringLiteral method_name = requirement.name;
       if (!class_dict.HasKey(method_name))
-        SET_ERROR_AND_CONTINUE(method_name,
-                               AbstractMethodCheckerCases::eNotImplemented)
+        SET_CASE_AND_CONTINUE(method_name,
+                              AbstractMethodCheckerCases::eNotImplemented)
       auto callable_or_err = class_dict.GetItem(method_name);
-      if (!callable_or_err)
-        SET_ERROR_AND_CONTINUE(method_name,
-                               AbstractMethodCheckerCases::eNotAllocated)
-      if (!PythonCallable::Check(callable_or_err.get().get()))
-        SET_ERROR_AND_CONTINUE(method_name,
-                               AbstractMethodCheckerCases::eNotCallable)
-      checker[method_name] = AbstractMethodCheckerCases::eValid;
+      if (!callable_or_err) {
+        llvm::consumeError(callable_or_err.takeError());
+        SET_CASE_AND_CONTINUE(method_name,
+                              AbstractMethodCheckerCases::eNotAllocated)
+      }
+
+      PythonCallable callable = callable_or_err->AsType<PythonCallable>();
+      if (!callable)
+        SET_CASE_AND_CONTINUE(method_name,
+                              AbstractMethodCheckerCases::eNotCallable)
+
+      if (!requirement.min_arg_count)
+        SET_CASE_AND_CONTINUE(method_name, AbstractMethodCheckerCases::eValid)
+
+      auto arg_info_or_err = callable.GetArgInfo();
+      if (!arg_info_or_err) {
+        llvm::consumeError(arg_info_or_err.takeError());
+        SET_CASE_AND_CONTINUE(method_name,
+                              AbstractMethodCheckerCases::eUnknownArgumentCount)
+      }
+
+      PythonCallable::ArgInfo arg_info = *arg_info_or_err;
+      if (requirement.min_arg_count <= arg_info.max_positional_args) {
+        SET_CASE_AND_CONTINUE(method_name, AbstractMethodCheckerCases::eValid)
+      } else {
+        checker[method_name] = {
+            AbstractMethodCheckerCases::eInvalidArgumentCount,
+            AbstrackMethodCheckerPayload::InvalidArgumentCountPayload(
+                requirement.min_arg_count, arg_info.max_positional_args)};
+      }
     }
 
-#undef HANDLE_ERROR
+#undef SET_CASE_AND_CONTINUE
 
     return checker;
   }
@@ -78,8 +119,11 @@ class ScriptedPythonInterface : virtual public ScriptedInterface {
     using namespace python;
     using Locker = ScriptInterpreterPythonImpl::Locker;
 
-    auto create_error = [](std::string message) {
-      return llvm::createStringError(llvm::inconvertibleErrorCode(), message);
+    Log *log = GetLog(LLDBLog::Script);
+    auto create_error = [](llvm::StringLiteral format, auto &&...ts) {
+      return llvm::createStringError(
+          llvm::formatv(format.data(), std::forward<decltype(ts)>(ts)...)
+              .str());
     };
 
     bool has_class_name = !class_name.empty();
@@ -107,16 +151,15 @@ class ScriptedPythonInterface : virtual public ScriptedInterface {
           PythonModule::MainModule().ResolveName<python::PythonDictionary>(
               m_interpreter.GetDictionaryName());
       if (!dict.IsAllocated())
-        return create_error(
-            llvm::formatv("Could not find interpreter dictionary: %s",
-                          m_interpreter.GetDictionaryName()));
+        return create_error("Could not find interpreter dictionary: {0}",
+                            m_interpreter.GetDictionaryName());
 
       auto init =
           PythonObject::ResolveNameWithDictionary<python::PythonCallable>(
               class_name, dict);
       if (!init.IsAllocated())
-        return create_error(llvm::formatv("Could not find script class: {0}",
-                                          class_name.data()));
+        return create_error("Could not find script class: {0}",
+                            class_name.data());
 
       std::tuple<Args...> original_args = std::forward_as_tuple(args...);
       auto transformed_args = TransformArgs(original_args);
@@ -186,36 +229,73 @@ class ScriptedPythonInterface : virtual public ScriptedInterface {
     if (!checker_or_err)
       return checker_or_err.takeError();
 
+    llvm::Error abstract_method_errors = llvm::Error::success();
     for (const auto &method_checker : *checker_or_err)
-      switch (method_checker.second) {
+      switch (method_checker.second.checker_case) {
       case AbstractMethodCheckerCases::eNotImplemented:
-        LLDB_LOG(GetLog(LLDBLog::Script),
-                 "Abstract method {0}.{1} not implemented.",
-                 obj_class_name.GetString(), method_checker.first);
+        abstract_method_errors = llvm::joinErrors(
+            std::move(abstract_method_errors),
+            std::move(create_error("Abstract method {0}.{1} not implemented.",
+                                   obj_class_name.GetString(),
+                                   method_checker.first)));
         break;
       case AbstractMethodCheckerCases::eNotAllocated:
-        LLDB_LOG(GetLog(LLDBLog::Script),
-                 "Abstract method {0}.{1} not allocated.",
-                 obj_class_name.GetString(), method_checker.first);
+        abstract_method_errors = llvm::joinErrors(
+            std::move(abstract_method_errors),
+            std::move(create_error("Abstract method {0}.{1} not allocated.",
+                                   obj_class_name.GetString(),
+                                   method_checker.first)));
         break;
       case AbstractMethodCheckerCases::eNotCallable:
-        LLDB_LOG(GetLog(LLDBLog::Script),
-                 "Abstract method {0}.{1} not callable.",
-                 obj_class_name.GetString(), method_checker.first);
+        abstract_method_errors = llvm::joinErrors(
+            std::move(abstract_method_errors),
+            std::move(create_error("Abstract method {0}.{1} not callable.",
+                                   obj_class_name.GetString(),
+                                   method_checker.first)));
+        break;
+      case AbstractMethodCheckerCases::eUnknownArgumentCount:
+        abstract_method_errors = llvm::joinErrors(
+            std::move(abstract_method_errors),
+            std::move(create_error(
+                "Abstract method {0}.{1} has unknown argument count.",
+                obj_class_name.GetString(), method_checker.first)));
         break;
+      case AbstractMethodCheckerCases::eInvalidArgumentCount: {
+        auto &payload_variant = method_checker.second.payload;
+        if (!std::holds_alternative<
+                AbstrackMethodCheckerPayload::InvalidArgumentCountPayload>(
+                payload_variant)) {
+          abstract_method_errors = llvm::joinErrors(
+              std::move(abstract_method_errors),
+              std::move(create_error(
+                  "Abstract method {0}.{1} has unexpected argument count.",
+                  obj_class_name.GetString(), method_checker.first)));
+        } else {
+          auto payload = std::get<
+              AbstrackMethodCheckerPayload::InvalidArgumentCountPayload>(
+              payload_variant);
+          abstract_method_errors = llvm::joinErrors(
+              std::move(abstract_method_errors),
+              std::move(
+                  create_error("Abstract method {0}.{1} has unexpected "
+                               "argument count (expected {2} but has {3}).",
+                               obj_class_name.GetString(), method_checker.first,
+                               payload.required_argument_count,
+                               payload.actual_argument_count)));
+        }
+      } break;
       case AbstractMethodCheckerCases::eValid:
-        LLDB_LOG(GetLog(LLDBLog::Script),
-                 "Abstract method {0}.{1} implemented & valid.",
+        LLDB_LOG(log, "Abstract method {0}.{1} implemented & valid.",
                  obj_class_name.GetString(), method_checker.first);
         break;
       }
 
-    for (const auto &method_checker : *checker_or_err)
-      if (method_checker.second != AbstractMethodCheckerCases::eValid)
-        return create_error(
-            llvm::formatv("Abstract method {0}.{1} missing. Enable lldb "
-                          "script log for more details.",
-                          obj_class_name.GetString(), method_checker.first));
+    if (abstract_method_errors) {
+      Status error = Status::FromError(std::move(abstract_method_errors));
+      LLDB_LOG(log, "Abstract method error in {0}:\n{1}", class_name,
+               error.AsCString());
+      return error.ToError();
+    }
 
     m_object_instance_sp = StructuredData::GenericSP(
         new StructuredPythonObject(std::move(result)));

lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedThreadPlanPythonInterface.h

@@ -30,7 +30,8 @@ class ScriptedThreadPlanPythonInterface : public ScriptedThreadPlanInterface,
                      lldb::ThreadPlanSP thread_plan_sp,
                      const StructuredDataImpl &args_sp) override;
 
-  llvm::SmallVector<llvm::StringLiteral> GetAbstractMethods() const override {
+  llvm::SmallVector<AbstractMethodRequirement>
+  GetAbstractMethodRequirements() const override {
     return {};
   }
 

lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedThreadPythonInterface.h

@@ -28,9 +28,10 @@ class ScriptedThreadPythonInterface : public ScriptedThreadInterface,
                      StructuredData::DictionarySP args_sp,
                      StructuredData::Generic *script_obj = nullptr) override;
 
-  llvm::SmallVector<llvm::StringLiteral> GetAbstractMethods() const override {
-    return llvm::SmallVector<llvm::StringLiteral>(
-        {"get_stop_reason", "get_register_context"});
+  llvm::SmallVector<AbstractMethodRequirement>
+  GetAbstractMethodRequirements() const override {
+    return llvm::SmallVector<AbstractMethodRequirement>(
+        {{"get_stop_reason"}, {"get_register_context"}});
   }
 
   lldb::tid_t GetThreadID() override;