[lldb/Interpreter] Add requirements to Scripted Interface abstract methods (#109063)

This patch adds new requirements to the Scripted Interface abstract
method checker to check the minimum number of argument for abstract
methods.

This check is done when creating the interface object so the object is
not created if the user implementation doesn't match the abstract method
requirement.

Signed-off-by: Med Ismail Bennani <ismail@bennani.ma>

6 files changed, 142 insertions, 53 deletions

diff --git a/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/OperatingSystemPythonInterface.h b/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/OperatingSystemPythonInterface.h
index 92358ac..102c3c3 100644
--- a/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/OperatingSystemPythonInterface.h
+++ b/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/OperatingSystemPythonInterface.h
@@ -31,8 +31,9 @@ public:
                      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,
diff --git a/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedPlatformPythonInterface.h b/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedPlatformPythonInterface.h
index 36a219a..a0da1bb 100644
--- a/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedPlatformPythonInterface.h
+++ b/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedPlatformPythonInterface.h
@@ -29,10 +29,13 @@ public:
                      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;
diff --git a/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedProcessPythonInterface.h b/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedProcessPythonInterface.h
index 1535d57..703b942 100644
--- a/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedProcessPythonInterface.h
+++ b/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedProcessPythonInterface.h
@@ -31,9 +31,12 @@ public:
                      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;
diff --git a/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedPythonInterface.h b/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedPythonInterface.h
index cb6f6ec..c715295 100644
--- a/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedPythonInterface.h
+++ b/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedPythonInterface.h
@@ -36,37 +36,78 @@ public:
     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 @@ public:
     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 @@ public:
           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 @@ public:
     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)));
diff --git a/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedThreadPlanPythonInterface.h b/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedThreadPlanPythonInterface.h
index 5e78ae7..2d01739 100644
--- a/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedThreadPlanPythonInterface.h
+++ b/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedThreadPlanPythonInterface.h
@@ -30,7 +30,8 @@ public:
                      lldb::ThreadPlanSP thread_plan_sp,
                      const StructuredDataImpl &args_sp) override;
 
-  llvm::SmallVector<llvm::StringLiteral> GetAbstractMethods() const override {
+  llvm::SmallVector<AbstractMethodRequirement>
+  GetAbstractMethodRequirements() const override {
     return {};
   }
 
diff --git a/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedThreadPythonInterface.h b/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedThreadPythonInterface.h
index 5676f7f..1fb23b3 100644
--- a/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedThreadPythonInterface.h
+++ b/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedThreadPythonInterface.h
@@ -28,9 +28,10 @@ public:
                      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;