-
Notifications
You must be signed in to change notification settings - Fork 15.1k
Switch the ScriptedBreakpointResolver over to the ScriptedInterface form #150720
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
form. This is NFC, I'm modernizing the interface before I add to it in a subsequent commit.
@llvm/pr-subscribers-lldb Author: None (jimingham) ChangesThis is NFC, I'm modernizing the interface before I add to it in a subsequent commit. Patch is 37.46 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/150720.diff 22 Files Affected:
diff --git a/lldb/bindings/python/python-wrapper.swig b/lldb/bindings/python/python-wrapper.swig
index 3d1d04e47e70b..2c30d536a753d 100644
--- a/lldb/bindings/python/python-wrapper.swig
+++ b/lldb/bindings/python/python-wrapper.swig
@@ -229,78 +229,6 @@ PythonObject lldb_private::python::SWIGBridge::LLDBSwigPythonCreateCommandObject
return pfunc(SWIGBridge::ToSWIGWrapper(std::move(debugger_sp)), dict);
}
-PythonObject lldb_private::python::SWIGBridge::LLDBSwigPythonCreateScriptedBreakpointResolver(
- const char *python_class_name, const char *session_dictionary_name,
- const StructuredDataImpl &args_impl,
- const lldb::BreakpointSP &breakpoint_sp) {
-
- if (python_class_name == NULL || python_class_name[0] == '\0' ||
- !session_dictionary_name)
- return PythonObject();
-
- PyErr_Cleaner py_err_cleaner(true);
-
- auto dict = PythonModule::MainModule().ResolveName<PythonDictionary>(
- session_dictionary_name);
- auto pfunc = PythonObject::ResolveNameWithDictionary<PythonCallable>(
- python_class_name, dict);
-
- if (!pfunc.IsAllocated())
- return PythonObject();
-
- PythonObject result =
- pfunc(SWIGBridge::ToSWIGWrapper(breakpoint_sp), SWIGBridge::ToSWIGWrapper(args_impl), dict);
- // FIXME: At this point we should check that the class we found supports all
- // the methods that we need.
-
- if (result.IsAllocated()) {
- // Check that __callback__ is defined:
- auto callback_func = result.ResolveName<PythonCallable>("__callback__");
- if (callback_func.IsAllocated())
- return result;
- }
- return PythonObject();
-}
-
-unsigned int lldb_private::python::SWIGBridge::LLDBSwigPythonCallBreakpointResolver(
- void *implementor, const char *method_name,
- lldb_private::SymbolContext *sym_ctx) {
- PyErr_Cleaner py_err_cleaner(false);
- PythonObject self(PyRefType::Borrowed, static_cast<PyObject *>(implementor));
- auto pfunc = self.ResolveName<PythonCallable>(method_name);
-
- if (!pfunc.IsAllocated())
- return 0;
-
- PythonObject result = sym_ctx ? pfunc(SWIGBridge::ToSWIGWrapper(*sym_ctx)) : pfunc();
-
- if (PyErr_Occurred()) {
- PyErr_Print();
- PyErr_Clear();
- return 0;
- }
-
- // The callback will return a bool, but we're need to also return ints
- // so we're squirrelling the bool through as an int... And if you return
- // nothing, we'll continue.
- if (strcmp(method_name, "__callback__") == 0) {
- if (result.get() == Py_False)
- return 0;
- else
- return 1;
- }
-
- long long ret_val = unwrapOrSetPythonException(As<long long>(result));
-
- if (PyErr_Occurred()) {
- PyErr_Print();
- PyErr_Clear();
- return 0;
- }
-
- return ret_val;
-}
-
// wrapper that calls an optional instance member of an object taking no
// arguments
static PyObject *LLDBSwigPython_CallOptionalMember(
@@ -554,6 +482,18 @@ void *lldb_private::python::LLDBSWIGPython_CastPyObjectToSBStream(PyObject * dat
return sb_ptr;
}
+void *lldb_private::python::LLDBSWIGPython_CastPyObjectToSBSymbolContext(PyObject * data) {
+ lldb::SBSymbolContext *sb_ptr = nullptr;
+
+ int valid_cast =
+ SWIG_ConvertPtr(data, (void **)&sb_ptr, SWIGTYPE_p_lldb__SBSymbolContext, 0);
+
+ if (valid_cast == -1)
+ return NULL;
+
+ return sb_ptr;
+}
+
void *lldb_private::python::LLDBSWIGPython_CastPyObjectToSBValue(PyObject * data) {
lldb::SBValue *sb_ptr = NULL;
diff --git a/lldb/include/lldb/API/SBSymbolContext.h b/lldb/include/lldb/API/SBSymbolContext.h
index ae9fd8404c5cc..128b0b65b7860 100644
--- a/lldb/include/lldb/API/SBSymbolContext.h
+++ b/lldb/include/lldb/API/SBSymbolContext.h
@@ -80,6 +80,8 @@ class LLDB_API SBSymbolContext {
lldb_private::SymbolContext *get() const;
+ friend class lldb_private::ScriptInterpreter;
+
private:
std::unique_ptr<lldb_private::SymbolContext> m_opaque_up;
};
diff --git a/lldb/include/lldb/Breakpoint/BreakpointResolverScripted.h b/lldb/include/lldb/Breakpoint/BreakpointResolverScripted.h
index 0e4a1d733e107..0322fd9f46ede 100644
--- a/lldb/include/lldb/Breakpoint/BreakpointResolverScripted.h
+++ b/lldb/include/lldb/Breakpoint/BreakpointResolverScripted.h
@@ -12,6 +12,7 @@
#include "lldb/Breakpoint/BreakpointResolver.h"
#include "lldb/Core/ModuleSpec.h"
#include "lldb/Core/StructuredDataImpl.h"
+#include "lldb/Interpreter/Interfaces/ScriptedBreakpointInterface.h"
#include "lldb/lldb-forward.h"
namespace lldb_private {
@@ -64,7 +65,8 @@ class BreakpointResolverScripted : public BreakpointResolver {
std::string m_class_name;
lldb::SearchDepth m_depth;
StructuredDataImpl m_args;
- StructuredData::GenericSP m_implementation_sp;
+ Status m_error;
+ lldb::ScriptedBreakpointInterfaceSP m_interface_sp;
BreakpointResolverScripted(const BreakpointResolverScripted &) = delete;
const BreakpointResolverScripted &
diff --git a/lldb/include/lldb/Interpreter/Interfaces/ScriptedBreakpointInterface.h b/lldb/include/lldb/Interpreter/Interfaces/ScriptedBreakpointInterface.h
new file mode 100644
index 0000000000000..b78d2ef2ab070
--- /dev/null
+++ b/lldb/include/lldb/Interpreter/Interfaces/ScriptedBreakpointInterface.h
@@ -0,0 +1,35 @@
+//===-- ScriptedBreakpointInterface.h -----------------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLDB_INTERPRETER_INTERFACES_SCRIPTEDBREAKPOINTINTERFACE_H
+#define LLDB_INTERPRETER_INTERFACES_SCRIPTEDBREAKPOINTINTERFACE_H
+
+#include "lldb/lldb-private.h"
+#include "lldb/Symbol/SymbolContext.h"
+
+#include "ScriptedInterface.h"
+
+namespace lldb_private {
+class ScriptedBreakpointInterface : public ScriptedInterface {
+public:
+ virtual llvm::Expected<StructuredData::GenericSP>
+ CreatePluginObject(llvm::StringRef class_name, lldb::BreakpointSP target_sp,
+ const StructuredDataImpl &args_sp) = 0;
+
+ /// "ResolverCallback" will get called when a new module is loaded. The
+ /// new module information is passed in sym_ctx. The Resolver will add
+ /// any breakpoint locations it found in that module.
+ virtual bool ResolverCallback(SymbolContext sym_ctx) {
+ return true;
+ }
+ virtual lldb::SearchDepth GetDepth() { return lldb::eSearchDepthModule; }
+ virtual std::optional<std::string> GetShortHelp() { return nullptr; }
+};
+} // namespace lldb_private
+
+#endif // LLDB_INTERPRETER_INTERFACES_SCRIPTEDSTOPHOOKINTERFACE_H
diff --git a/lldb/include/lldb/Interpreter/ScriptInterpreter.h b/lldb/include/lldb/Interpreter/ScriptInterpreter.h
index f1c3eefdcd874..4de5091c938e3 100644
--- a/lldb/include/lldb/Interpreter/ScriptInterpreter.h
+++ b/lldb/include/lldb/Interpreter/ScriptInterpreter.h
@@ -18,6 +18,7 @@
#include "lldb/API/SBLaunchInfo.h"
#include "lldb/API/SBMemoryRegionInfo.h"
#include "lldb/API/SBStream.h"
+#include "lldb/API/SBSymbolContext.h"
#include "lldb/Breakpoint/BreakpointOptions.h"
#include "lldb/Core/PluginInterface.h"
#include "lldb/Core/SearchFilter.h"
@@ -29,6 +30,7 @@
#include "lldb/Interpreter/Interfaces/ScriptedProcessInterface.h"
#include "lldb/Interpreter/Interfaces/ScriptedThreadInterface.h"
#include "lldb/Interpreter/ScriptObject.h"
+#include "lldb/Symbol/SymbolContext.h"
#include "lldb/Utility/Broadcaster.h"
#include "lldb/Utility/Status.h"
#include "lldb/Utility/StructuredData.h"
@@ -257,26 +259,6 @@ class ScriptInterpreter : public PluginInterface {
return false;
}
- virtual StructuredData::GenericSP
- CreateScriptedBreakpointResolver(const char *class_name,
- const StructuredDataImpl &args_data,
- lldb::BreakpointSP &bkpt_sp) {
- return StructuredData::GenericSP();
- }
-
- virtual bool
- ScriptedBreakpointResolverSearchCallback(StructuredData::GenericSP implementor_sp,
- SymbolContext *sym_ctx)
- {
- return false;
- }
-
- virtual lldb::SearchDepth
- ScriptedBreakpointResolverSearchDepth(StructuredData::GenericSP implementor_sp)
- {
- return lldb::eSearchDepthModule;
- }
-
virtual StructuredData::ObjectSP
LoadPluginModule(const FileSpec &file_spec, lldb_private::Status &error) {
return StructuredData::ObjectSP();
@@ -566,6 +548,10 @@ class ScriptInterpreter : public PluginInterface {
return {};
}
+ virtual lldb::ScriptedBreakpointInterfaceSP CreateScriptedBreakpointInterface() {
+ return {};
+ }
+
virtual StructuredData::ObjectSP
CreateStructuredDataFromScriptObject(ScriptObject obj) {
return {};
@@ -580,6 +566,8 @@ class ScriptInterpreter : public PluginInterface {
lldb::StreamSP GetOpaqueTypeFromSBStream(const lldb::SBStream &stream) const;
+ SymbolContext GetOpaqueTypeFromSBSymbolContext(const lldb::SBSymbolContext &sym_ctx) const;
+
lldb::BreakpointSP
GetOpaqueTypeFromSBBreakpoint(const lldb::SBBreakpoint &breakpoint) const;
diff --git a/lldb/include/lldb/lldb-forward.h b/lldb/include/lldb/lldb-forward.h
index 2bc85a2d2afa6..483dce98ea427 100644
--- a/lldb/include/lldb/lldb-forward.h
+++ b/lldb/include/lldb/lldb-forward.h
@@ -188,6 +188,7 @@ class Scalar;
class ScriptInterpreter;
class ScriptInterpreterLocker;
class ScriptedMetadata;
+class ScriptedBreakpointInterface;
class ScriptedPlatformInterface;
class ScriptedProcessInterface;
class ScriptedStopHookInterface;
@@ -418,6 +419,8 @@ typedef std::shared_ptr<lldb_private::ScriptedThreadInterface>
ScriptedThreadInterfaceSP;
typedef std::shared_ptr<lldb_private::ScriptedThreadPlanInterface>
ScriptedThreadPlanInterfaceSP;
+typedef std::shared_ptr<lldb_private::ScriptedBreakpointInterface>
+ ScriptedBreakpointInterfaceSP;
typedef std::shared_ptr<lldb_private::Section> SectionSP;
typedef std::unique_ptr<lldb_private::SectionList> SectionListUP;
typedef std::weak_ptr<lldb_private::Section> SectionWP;
diff --git a/lldb/source/Breakpoint/BreakpointResolverScripted.cpp b/lldb/source/Breakpoint/BreakpointResolverScripted.cpp
index 2457052ddb6a6..09f072c0938d1 100644
--- a/lldb/source/Breakpoint/BreakpointResolverScripted.cpp
+++ b/lldb/source/Breakpoint/BreakpointResolverScripted.cpp
@@ -35,7 +35,7 @@ BreakpointResolverScripted::BreakpointResolverScripted(
void BreakpointResolverScripted::CreateImplementationIfNeeded(
BreakpointSP breakpoint_sp) {
- if (m_implementation_sp)
+ if (m_interface_sp)
return;
if (m_class_name.empty())
@@ -50,8 +50,28 @@ void BreakpointResolverScripted::CreateImplementationIfNeeded(
if (!script_interp)
return;
- m_implementation_sp = script_interp->CreateScriptedBreakpointResolver(
- m_class_name.c_str(), m_args, breakpoint_sp);
+ m_interface_sp = script_interp->CreateScriptedBreakpointInterface();
+ if (!m_interface_sp) {
+ m_error = Status::FromErrorStringWithFormat(
+ "BreakpointResolverScripted::%s () - ERROR: %s", __FUNCTION__,
+ "Script interpreter couldn't create Scripted Breakpoint Interface");
+ return;
+ }
+
+ auto obj_or_err = m_interface_sp->CreatePluginObject(
+ m_class_name, breakpoint_sp, m_args);
+ if (!obj_or_err) {
+ m_error = Status::FromError(obj_or_err.takeError());
+ printf("CreateImplementationIfNeeded got error: %s\n", m_error.AsCString());
+ return;
+ }
+
+ StructuredData::ObjectSP object_sp = *obj_or_err;
+ if (!object_sp || !object_sp->IsValid()) {
+ m_error = Status::FromErrorStringWithFormat(
+ "ScriptedBreakpoint::%s () - ERROR: %s", __FUNCTION__,
+ "Failed to create valid script object");
+ }
}
void BreakpointResolverScripted::NotifyBreakpointSet() {
@@ -104,13 +124,10 @@ ScriptInterpreter *BreakpointResolverScripted::GetScriptInterpreter() {
Searcher::CallbackReturn BreakpointResolverScripted::SearchCallback(
SearchFilter &filter, SymbolContext &context, Address *addr) {
bool should_continue = true;
- if (!m_implementation_sp)
+ if (!m_interface_sp)
return Searcher::eCallbackReturnStop;
- ScriptInterpreter *interp = GetScriptInterpreter();
- should_continue = interp->ScriptedBreakpointResolverSearchCallback(
- m_implementation_sp,
- &context);
+ should_continue = m_interface_sp->ResolverCallback(context);
if (should_continue)
return Searcher::eCallbackReturnContinue;
@@ -120,25 +137,21 @@ Searcher::CallbackReturn BreakpointResolverScripted::SearchCallback(
lldb::SearchDepth
BreakpointResolverScripted::GetDepth() {
lldb::SearchDepth depth = lldb::eSearchDepthModule;
- if (m_implementation_sp) {
- ScriptInterpreter *interp = GetScriptInterpreter();
- depth = interp->ScriptedBreakpointResolverSearchDepth(
- m_implementation_sp);
- }
+ if (m_interface_sp)
+ depth = m_interface_sp->GetDepth();
+
return depth;
}
void BreakpointResolverScripted::GetDescription(Stream *s) {
StructuredData::GenericSP generic_sp;
- std::string short_help;
+ std::optional<std::string> short_help;
- if (m_implementation_sp) {
- ScriptInterpreter *interp = GetScriptInterpreter();
- interp->GetShortHelpForCommandObject(m_implementation_sp,
- short_help);
+ if (m_interface_sp) {
+ short_help = m_interface_sp->GetShortHelp();
}
- if (!short_help.empty())
- s->PutCString(short_help.c_str());
+ if (short_help && !short_help->empty())
+ s->PutCString(short_help->c_str());
else
s->Printf("python class = %s", m_class_name.c_str());
}
diff --git a/lldb/source/Interpreter/ScriptInterpreter.cpp b/lldb/source/Interpreter/ScriptInterpreter.cpp
index ae913e68feb19..93e8405e95acf 100644
--- a/lldb/source/Interpreter/ScriptInterpreter.cpp
+++ b/lldb/source/Interpreter/ScriptInterpreter.cpp
@@ -116,6 +116,14 @@ lldb::StreamSP ScriptInterpreter::GetOpaqueTypeFromSBStream(
return nullptr;
}
+SymbolContext ScriptInterpreter::GetOpaqueTypeFromSBSymbolContext(
+ const lldb::SBSymbolContext &sb_sym_ctx) const {
+ if (sb_sym_ctx.m_opaque_up) {
+ return *sb_sym_ctx.m_opaque_up.get();
+ }
+ return {};
+}
+
std::optional<MemoryRegionInfo>
ScriptInterpreter::GetOpaqueTypeFromSBMemoryRegionInfo(
const lldb::SBMemoryRegionInfo &mem_region) const {
diff --git a/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/CMakeLists.txt b/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/CMakeLists.txt
index db9e11b73895b..04370940423ae 100644
--- a/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/CMakeLists.txt
+++ b/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/CMakeLists.txt
@@ -26,6 +26,7 @@ add_lldb_library(lldbPluginScriptInterpreterPythonInterfaces PLUGIN
ScriptedProcessPythonInterface.cpp
ScriptedPythonInterface.cpp
ScriptedStopHookPythonInterface.cpp
+ ScriptedBreakpointPythonInterface.cpp
ScriptedThreadPlanPythonInterface.cpp
ScriptedThreadPythonInterface.cpp
diff --git a/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptInterpreterPythonInterfaces.cpp b/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptInterpreterPythonInterfaces.cpp
index 1fd32993e385e..d43036d6fe544 100644
--- a/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptInterpreterPythonInterfaces.cpp
+++ b/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptInterpreterPythonInterfaces.cpp
@@ -29,6 +29,7 @@ void ScriptInterpreterPythonInterfaces::Initialize() {
ScriptedPlatformPythonInterface::Initialize();
ScriptedProcessPythonInterface::Initialize();
ScriptedStopHookPythonInterface::Initialize();
+ ScriptedBreakpointPythonInterface::Initialize();
ScriptedThreadPlanPythonInterface::Initialize();
}
@@ -37,6 +38,7 @@ void ScriptInterpreterPythonInterfaces::Terminate() {
ScriptedPlatformPythonInterface::Terminate();
ScriptedProcessPythonInterface::Terminate();
ScriptedStopHookPythonInterface::Terminate();
+ ScriptedBreakpointPythonInterface::Terminate();
ScriptedThreadPlanPythonInterface::Terminate();
}
diff --git a/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptInterpreterPythonInterfaces.h b/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptInterpreterPythonInterfaces.h
index 26c80b7568691..70f7eea543466 100644
--- a/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptInterpreterPythonInterfaces.h
+++ b/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptInterpreterPythonInterfaces.h
@@ -19,6 +19,7 @@
#include "ScriptedPlatformPythonInterface.h"
#include "ScriptedProcessPythonInterface.h"
#include "ScriptedStopHookPythonInterface.h"
+#include "ScriptedBreakpointPythonInterface.h"
#include "ScriptedThreadPlanPythonInterface.h"
namespace lldb_private {
diff --git a/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedBreakpointPythonInterface.cpp b/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedBreakpointPythonInterface.cpp
new file mode 100644
index 0000000000000..2f1cee783ebf9
--- /dev/null
+++ b/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedBreakpointPythonInterface.cpp
@@ -0,0 +1,100 @@
+//===-- ScriptedBreakpointPythonInterface.cpp -------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "lldb/Core/PluginManager.h"
+#include "lldb/Host/Config.h"
+#include "lldb/Symbol/SymbolContext.h"
+#include "lldb/Target/ExecutionContext.h"
+#include "lldb/Utility/Log.h"
+#include "lldb/lldb-enumerations.h"
+
+#if LLDB_ENABLE_PYTHON
+
+// clang-format off
+// LLDB Python header must be included first
+#include "../lldb-python.h"
+//clang-format on
+
+#include "../SWIGPythonBridge.h"
+#include "../ScriptInterpreterPythonImpl.h"
+#include "ScriptedBreakpointPythonInterface.h"
+
+using namespace lldb;
+using namespace lldb_private;
+using namespace lldb_private::python;
+
+ScriptedBreakpointPythonInterface::ScriptedBreakpointPythonInterface(
+ ScriptInterpreterPythonImpl &interpreter)
+ : ScriptedBreakpointInterface(), ScriptedPythonInterface(interpreter) {}
+
+llvm::Expected<StructuredData::GenericSP>
+ScriptedBreakpointPythonInterface::CreatePluginObject(llvm::StringRef class_name,
+ lldb::BreakpointSP break_sp,
+ const StructuredDataImpl &args_sp) {
+ lldb::TargetSP target_sp;
+ return ScriptedPythonInterface::CreatePluginObject(class_name, nullptr,
+ break_sp /*target_sp*/, args_sp);
+}
+
+bool
+ScriptedBreakpointPythonInterface::ResolverCallback(SymbolContext sym_ctx) {
+ Status error;
+
+ StructuredData::ObjectSP obj = Dispatch("__callback__", error, sym_ctx);
+
+ if (!ScriptedInterface::CheckStructuredDataObject(LLVM_PRETTY_FUNCTION, obj,
+ error)) {
+ // FIXME: Should log the error here.
+ return true;
+ }
+ return obj->GetBooleanValue();
+}
+
+lldb::SearchDepth ScriptedBreakpointPythonInterface::GetDepth() {
+ Status error;
+ StructuredData::ObjectSP obj = Dispatch("__get_depth__", error);
+
+ if (!ScriptedInterface::CheckStructuredDataObject(LLVM_PRETTY_FUNCTION, obj,
+ error)) {
+ return lldb::eSearchDepthModule;
+ }
+ uint64_t value = obj->GetUnsignedIntegerValue();
+ if (value <= lldb::kLastSearchDepthKind)
+ return (lldb::SearchDepth) value;
+ // This is what we were doing on error before, though I'm not sure that's
+ // better than returning eSearchDepthInvalid.
+ return lldb::eSearchDepthModule;
+}
+
+std::optional<std::string> ScriptedBreakpointPythonInterface::GetShortHelp() {
+ Status error;
+ StructuredData::ObjectSP obj = Dispatch("get_short_help", error);
+
+ if (!ScriptedInterface::CheckStructuredDataObject(LLVM_PRETTY_FUNCTION, obj,
+ error)) {
+ return {};
+ }
+
+ return obj->GetAsString()->GetValue().str();
+}
+
+void ScriptedBreakpointPythonInterface::Initialize() {
+ const std::vector<llvm::StringRef> ci_usages = {
+ ...
[truncated]
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
✅ With the latest revision this PR passed the Python code formatter. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mostly nits from my end. For everything else I defer to @medismailben, the scripted expert.
@@ -0,0 +1,35 @@ | |||
//===-- ScriptedBreakpointInterface.h -----------------------------*- C++ -*-===// |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: new files shouldn't contain the name and filetype anymore.
//===-- ScriptedBreakpointInterface.h -----------------------------*- C++ -*-===// | |
//===----------------------------------------------------------------------===// |
#include "lldb/lldb-private.h" | ||
#include "lldb/Symbol/SymbolContext.h" | ||
|
||
#include "ScriptedInterface.h" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
#include "lldb/lldb-private.h" | |
#include "lldb/Symbol/SymbolContext.h" | |
#include "ScriptedInterface.h" | |
#include "lldb/lldb-private.h" | |
#include "lldb/Symbol/SymbolContext.h" | |
#include "ScriptedInterface.h" |
m_class_name, breakpoint_sp, m_args); | ||
if (!obj_or_err) { | ||
m_error = Status::FromError(obj_or_err.takeError()); | ||
printf("CreateImplementationIfNeeded got error: %s\n", m_error.AsCString()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
printf("CreateImplementationIfNeeded got error: %s\n", m_error.AsCString()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, that one should actually go, that was for debugging. I have to figure out a way to make this error available. That's a general problem with errors when creating the Python object to back these affordances.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's file an issue or radar to update every scripted affordance to take an Status
reference so the caller can percolate up to the user.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just said "we should come up with a good scheme" I didn't opine on the scheme.
if (sb_sym_ctx.m_opaque_up) { | ||
return *sb_sym_ctx.m_opaque_up.get(); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if (sb_sym_ctx.m_opaque_up) { | |
return *sb_sym_ctx.m_opaque_up.get(); | |
} | |
if (sb_sym_ctx.m_opaque_up) | |
return *sb_sym_ctx.m_opaque_up.get(); |
@@ -0,0 +1,100 @@ | |||
//===-- ScriptedBreakpointPythonInterface.cpp -------------------------------===// |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
//===-- ScriptedBreakpointPythonInterface.cpp -------------------------------===// | |
//===----------------------------------------------------------------------===//``` |
// clang-format off | ||
// LLDB Python header must be included first | ||
#include "../lldb-python.h" | ||
//clang-format on |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The clang-format directives shouldn't be necessary when you have newlines before and after. Other places that include this file don't seem to do this either.
// clang-format off | |
// LLDB Python header must be included first | |
#include "../lldb-python.h" | |
//clang-format on | |
// LLDB Python header must be included first | |
#include "../lldb-python.h" |
lldb::TargetSP target_sp; | ||
return ScriptedPythonInterface::CreatePluginObject(class_name, nullptr, | ||
break_sp /*target_sp*/, args_sp); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
target_sp
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, right. That was trying to figure which of the magic ToSWIGWrapper/Transform I was missing. target_sp worked but break_sp didn't till I added the right transform. This should go.
@@ -0,0 +1,52 @@ | |||
//===-- ScriptedBreakpointPythonInterface.h -----------------------*- C++ -*-===// |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
//===-- ScriptedBreakpointPythonInterface.h -----------------------*- C++ -*-===// | |
//===----------------------------------------------------------------------===//``` |
|
||
if (!ScriptedInterface::CheckStructuredDataObject(LLVM_PRETTY_FUNCTION, obj, | ||
error)) { | ||
// FIXME: Should log the error here. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add logging ?
void ScriptedBreakpointPythonInterface::Initialize() { | ||
const std::vector<llvm::StringRef> ci_usages = { | ||
"breakpoint set -P classname [-k key -v value ...]"}; | ||
const std::vector<llvm::StringRef> api_usages = {}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there any way to setup scripted breakpoints using the SB API ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Beautiful! LGTM with comments!
The two "required" build stages both timed out trying to pull the sources... |
…orm (llvm#150720) This is NFC, I'm modernizing the interface before I add to it in a subsequent commit. (cherry picked from commit 4c8e79f)
…orm (llvm#150720) This is NFC, I'm modernizing the interface before I add to it in a subsequent commit. (cherry picked from commit 4c8e79f)
…orm (llvm#150720) This is NFC, I'm modernizing the interface before I add to it in a subsequent commit. (cherry picked from commit 4c8e79f)
…orm (llvm#150720) This is NFC, I'm modernizing the interface before I add to it in a subsequent commit. (cherry picked from commit 4c8e79f)
…orm (llvm#150720) This is NFC, I'm modernizing the interface before I add to it in a subsequent commit. (cherry picked from commit 4c8e79f) (cherry picked from commit f05bc88)
This is NFC, I'm modernizing the interface before I add to it in a subsequent commit.