[lldb/Interpreter] Add requirements to Scripted Interface abstract methods #109063
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@llvm/pr-subscribers-lldb Author: Med Ismail Bennani (medismailben) ChangesThis 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. Full diff: https://github.com/llvm/llvm-project/pull/109063.diff 7 Files Affected:
diff --git a/lldb/include/lldb/Interpreter/Interfaces/ScriptedInterface.h b/lldb/include/lldb/Interpreter/Interfaces/ScriptedInterface.h
index 89c0b36d6fc2a1..a3dc52c435561f 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<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,
diff --git a/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/OperatingSystemPythonInterface.h b/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/OperatingSystemPythonInterface.h
index 92358ac6c34f7e..102c3c39537686 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<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 36a219a656993b..3031508f892322 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<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", 1},
+ {"launch_process", 1},
+ {"kill_process", 1}});
}
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 1535d573e72f43..fc645981b61b31 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<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", 3},
+ {"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 cb6f6ec3989260..aa214f4c141b58 100644
--- a/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedPythonInterface.h
+++ b/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedPythonInterface.h
@@ -36,6 +36,7 @@ class ScriptedPythonInterface : virtual public ScriptedInterface {
eNotImplemented,
eNotAllocated,
eNotCallable,
+ eInvalidArgumentCount,
eValid
};
@@ -46,27 +47,48 @@ class ScriptedPythonInterface : virtual public ScriptedInterface {
using namespace python;
std::map<llvm::StringLiteral, AbstractMethodCheckerCases> checker;
-#define SET_ERROR_AND_CONTINUE(method_name, error) \
+#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::eInvalidArgumentCount)
+ }
+
+ PythonCallable::ArgInfo arg_info = *arg_info_or_err;
+ checker[method_name] =
+ (requirement.min_arg_count <= arg_info.max_positional_args)
+ ? AbstractMethodCheckerCases::eValid
+ : AbstractMethodCheckerCases::eInvalidArgumentCount;
}
-#undef HANDLE_ERROR
+#undef SET_CASE_AND_CONTINUE
return checker;
}
@@ -78,8 +100,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::StringRef 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 +132,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 +210,45 @@ 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) {
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::eInvalidArgumentCount:
+ 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)));
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)
+ return abstract_method_errors;
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 5e78ae764eeb51..2d017396df7e27 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<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 5676f7f1d67528..1fb23b39c70761 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<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;
|
bulbazord
reviewed
Sep 18, 2024
lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedPythonInterface.h
Outdated
Show resolved
Hide resolved
medismailben
force-pushed
the
scripted-abstract-method-requirements
branch
from
91d993c to
e815d5f
Compare
bulbazord
reviewed
Sep 19, 2024
lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedPythonInterface.h
Outdated
Show resolved
Hide resolved
…thods 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 <[email protected]>
medismailben
force-pushed
the
scripted-abstract-method-requirements
branch
from
e815d5f to
897dd35
Compare
bulbazord
approved these changes
Sep 19, 2024
tmsri
pushed a commit
to tmsri/llvm-project
that referenced
this pull request
Sep 19, 2024
…thods (llvm#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 <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.