diff --git a/lldb/include/lldb/Interpreter/Interfaces/ScriptedInterface.h b/lldb/include/lldb/Interpreter/Interfaces/ScriptedInterface.h index 89c0b36d6fc2a..a3dc52c435561 100644 --- a/lldb/include/lldb/Interpreter/Interfaces/ScriptedInterface.h +++ b/lldb/include/lldb/Interpreter/Interfaces/ScriptedInterface.h @@ -31,7 +31,22 @@ class ScriptedInterface { return m_object_instance_sp; } - virtual llvm::SmallVector GetAbstractMethods() const = 0; + struct AbstractMethodRequirement { + llvm::StringLiteral name; + size_t min_arg_count = 0; + }; + + virtual llvm::SmallVector + GetAbstractMethodRequirements() const = 0; + + llvm::SmallVector const GetAbstractMethods() const { + llvm::SmallVector abstract_methods; + llvm::transform(GetAbstractMethodRequirements(), abstract_methods.begin(), + [](const AbstractMethodRequirement &requirement) { + return requirement.name; + }); + return abstract_methods; + } template static Ret ErrorWithMessage(llvm::StringRef caller_name, diff --git a/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/OperatingSystemPythonInterface.h b/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/OperatingSystemPythonInterface.h index 92358ac6c34f7..102c3c3953768 100644 --- a/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/OperatingSystemPythonInterface.h +++ b/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 GetAbstractMethods() const override { - return llvm::SmallVector({"get_thread_info"}); + llvm::SmallVector + GetAbstractMethodRequirements() const override { + return llvm::SmallVector({{"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 36a219a656993..a0da1bbc31c70 100644 --- a/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedPlatformPythonInterface.h +++ b/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 GetAbstractMethods() const override { - return llvm::SmallVector( - {"list_processes", "attach_to_process", "launch_process", - "kill_process"}); + llvm::SmallVector + GetAbstractMethodRequirements() const override { + return llvm::SmallVector( + {{"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 1535d573e72f4..703b942fe5647 100644 --- a/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedProcessPythonInterface.h +++ b/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 GetAbstractMethods() const override { - return llvm::SmallVector( - {"read_memory_at_address", "is_alive", "get_scripted_thread_plugin"}); + llvm::SmallVector + GetAbstractMethodRequirements() const override { + return llvm::SmallVector( + {{"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 cb6f6ec398926..c715295eb9ff0 100644 --- a/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedPythonInterface.h +++ b/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> + 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 payload; + }; + + llvm::Expected> CheckAbstractMethodImplementation( const python::PythonDictionary &class_dict) const { using namespace python; - std::map checker; -#define SET_ERROR_AND_CONTINUE(method_name, error) \ + std::map 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(); + 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(ts)...) + .str()); }; bool has_class_name = !class_name.empty(); @@ -107,16 +151,15 @@ class ScriptedPythonInterface : virtual public ScriptedInterface { PythonModule::MainModule().ResolveName( 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( 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 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))); diff --git a/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedThreadPlanPythonInterface.h b/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedThreadPlanPythonInterface.h index 5e78ae764eeb5..2d017396df7e2 100644 --- a/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedThreadPlanPythonInterface.h +++ b/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 GetAbstractMethods() const override { + llvm::SmallVector + 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 5676f7f1d6752..1fb23b39c7076 100644 --- a/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedThreadPythonInterface.h +++ b/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 GetAbstractMethods() const override { - return llvm::SmallVector( - {"get_stop_reason", "get_register_context"}); + llvm::SmallVector + GetAbstractMethodRequirements() const override { + return llvm::SmallVector( + {{"get_stop_reason"}, {"get_register_context"}}); } lldb::tid_t GetThreadID() override;