lawrence_danna updated this revision to Diff 225302.
lawrence_danna marked 8 inline comments as done.
lawrence_danna added a comment.
review fixes
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D68995/new/
https://reviews.llvm.org/D68995
Files:
lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
lldb/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp
Index: lldb/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp
===================================================================
--- lldb/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp
+++ lldb/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp
@@ -20,6 +20,7 @@
#include "PythonTestSuite.h"
using namespace lldb_private;
+using namespace lldb_private::python;
class PythonDataObjectsTest : public PythonTestSuite {
public:
@@ -626,3 +627,92 @@
}
}
}
+
+TEST_F(PythonDataObjectsTest, TestCallable) {
+
+ PythonDictionary globals(PyInitialValue::Empty);
+ auto builtins = PythonModule::BuiltinsModule();
+ llvm::Error error = globals.SetItem("__builtins__", builtins);
+ ASSERT_FALSE(error);
+
+ {
+ PyObject *o = PyRun_String("lambda x : x", Py_eval_input, globals.get(),
+ globals.get());
+ ASSERT_FALSE(o == NULL);
+ auto lambda = Take<PythonCallable>(o);
+ auto arginfo = lambda.GetArgInfo();
+ ASSERT_THAT_EXPECTED(arginfo, llvm::Succeeded());
+ EXPECT_EQ(arginfo.get().count, 1);
+ EXPECT_EQ(arginfo.get().has_varargs, false);
+ EXPECT_EQ(arginfo.get().is_bound_method, false);
+ }
+
+ {
+ PyObject *o = PyRun_String("lambda x,y=0: x", Py_eval_input, globals.get(),
+ globals.get());
+ ASSERT_FALSE(o == NULL);
+ auto lambda = Take<PythonCallable>(o);
+ auto arginfo = lambda.GetArgInfo();
+ ASSERT_THAT_EXPECTED(arginfo, llvm::Succeeded());
+ EXPECT_EQ(arginfo.get().count, 2);
+ EXPECT_EQ(arginfo.get().has_varargs, false);
+ EXPECT_EQ(arginfo.get().is_bound_method, false);
+ }
+
+ {
+ PyObject *o = PyRun_String("lambda x,y,*a: x", Py_eval_input, globals.get(),
+ globals.get());
+ ASSERT_FALSE(o == NULL);
+ auto lambda = Take<PythonCallable>(o);
+ auto arginfo = lambda.GetArgInfo();
+ ASSERT_THAT_EXPECTED(arginfo, llvm::Succeeded());
+ EXPECT_EQ(arginfo.get().count, 2);
+ EXPECT_EQ(arginfo.get().has_varargs, true);
+ EXPECT_EQ(arginfo.get().is_bound_method, false);
+ }
+
+ {
+ const char *script = "class Foo: \n"
+ " def bar(self, x):\n"
+ " return x \n"
+ "bar_bound = Foo().bar \n"
+ "bar_unbound = Foo.bar \n";
+ PyObject *o =
+ PyRun_String(script, Py_file_input, globals.get(), globals.get());
+ ASSERT_FALSE(o == NULL);
+ Take<PythonObject>(o);
+
+ auto bar_bound = As<PythonCallable>(globals.GetItem("bar_bound"));
+ ASSERT_THAT_EXPECTED(bar_bound, llvm::Succeeded());
+ auto arginfo = bar_bound.get().GetArgInfo();
+ ASSERT_THAT_EXPECTED(arginfo, llvm::Succeeded());
+ EXPECT_EQ(arginfo.get().count, 2); // FIXME, wrong
+ EXPECT_EQ(arginfo.get().has_varargs, false);
+ EXPECT_EQ(arginfo.get().is_bound_method, true);
+
+ auto bar_unbound = As<PythonCallable>(globals.GetItem("bar_unbound"));
+ ASSERT_THAT_EXPECTED(bar_unbound, llvm::Succeeded());
+ arginfo = bar_unbound.get().GetArgInfo();
+ ASSERT_THAT_EXPECTED(arginfo, llvm::Succeeded());
+ EXPECT_EQ(arginfo.get().count, 2);
+ EXPECT_EQ(arginfo.get().has_varargs, false);
+ EXPECT_EQ(arginfo.get().is_bound_method, false);
+ }
+
+#if PY_MAJOR_VERSION >= 3 && PY_MINOR_VERSION >= 3
+
+ // the old implementation of GetArgInfo just doesn't work on builtins.
+
+ {
+ auto builtins = PythonModule::BuiltinsModule();
+ auto hex = As<PythonCallable>(builtins.GetAttribute("hex"));
+ ASSERT_THAT_EXPECTED(hex, llvm::Succeeded());
+ auto arginfo = hex.get().GetArgInfo();
+ ASSERT_THAT_EXPECTED(arginfo, llvm::Succeeded());
+ EXPECT_EQ(arginfo.get().count, 1);
+ EXPECT_EQ(arginfo.get().has_varargs, false);
+ EXPECT_EQ(arginfo.get().is_bound_method, false);
+ }
+
+#endif
+}
\ No newline at end of file
Index: lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
===================================================================
--- lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
+++ lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
@@ -309,20 +309,35 @@
static llvm::Error exception(const char *s = nullptr) {
return llvm::make_error<PythonException>(s);
}
+ static llvm::Error keyError() {
+ return llvm::createStringError(llvm::inconvertibleErrorCode(),
+ "key not in dict");
+ }
+
+#if PY_MAJOR_VERSION < 3
+ static char *py2_const_cast(const char *s) { return const_cast<char *>(s); }
+#else
+ static const char *py2_const_cast(const char *s) { return s; }
+#endif
public:
template <typename... T>
llvm::Expected<PythonObject> CallMethod(const char *name,
const T &... t) const {
const char format[] = {'(', PythonFormat<T>::format..., ')', 0};
-#if PY_MAJOR_VERSION < 3
- PyObject *obj = PyObject_CallMethod(m_py_obj, const_cast<char *>(name),
- const_cast<char *>(format),
- PythonFormat<T>::get(t)...);
-#else
PyObject *obj =
- PyObject_CallMethod(m_py_obj, name, format, PythonFormat<T>::get(t)...);
-#endif
+ PyObject_CallMethod(m_py_obj, py2_const_cast(name),
+ py2_const_cast(format), PythonFormat<T>::get(t)...);
+ if (!obj)
+ return exception();
+ return python::Take<PythonObject>(obj);
+ }
+
+ template <typename... T>
+ llvm::Expected<PythonObject> Call(const T &... t) const {
+ const char format[] = {'(', PythonFormat<T>::format..., ')', 0};
+ PyObject *obj = PyObject_CallFunction(m_py_obj, py2_const_cast(format),
+ PythonFormat<T>::get(t)...);
if (!obj)
return exception();
return python::Take<PythonObject>(obj);
@@ -386,6 +401,9 @@
template <>
llvm::Expected<long long> As<long long>(llvm::Expected<PythonObject> &&obj);
+template <>
+llvm::Expected<std::string> As<std::string>(llvm::Expected<PythonObject> &&obj);
+
} // namespace python
template <class T> class TypedPythonObject : public PythonObject {
@@ -559,8 +577,14 @@
PythonList GetKeys() const;
- PythonObject GetItemForKey(const PythonObject &key) const;
- void SetItemForKey(const PythonObject &key, const PythonObject &value);
+ PythonObject GetItemForKey(const PythonObject &key) const; // DEPRECATED
+ void SetItemForKey(const PythonObject &key,
+ const PythonObject &value); // DEPRECATED
+
+ llvm::Expected<PythonObject> GetItem(const PythonObject &key) const;
+ llvm::Expected<PythonObject> GetItem(const char *key) const;
+ llvm::Error SetItem(const PythonObject &key, const PythonObject &value) const;
+ llvm::Error SetItem(const char *key, const PythonObject &value) const;
StructuredData::DictionarySP CreateStructuredDictionary() const;
};
@@ -600,19 +624,31 @@
using TypedPythonObject::TypedPythonObject;
struct ArgInfo {
- size_t count;
- bool is_bound_method : 1;
- bool has_varargs : 1;
- bool has_kwargs : 1;
+ /* the number of positional arguments, including optional ones,
+ * and excluding varargs. If this is a bound method, then the
+ * count will still include a +1 for self.
+ *
+ * FIXME. That's crazy. This should be replaced with
+ * an accurate min and max for positional args.
+ */
+ int count;
+ /* does the callable have positional varargs? */
+ bool has_varargs : 1; // FIXME delete this
+ /* is the callable a bound method written in python? */
+ bool is_bound_method : 1; // FIXME delete this
};
static bool Check(PyObject *py_obj);
- ArgInfo GetNumArguments() const;
+ llvm::Expected<ArgInfo> GetArgInfo() const;
+
+ llvm::Expected<ArgInfo> GetInitArgInfo() const;
+
+ ArgInfo GetNumArguments() const; // DEPRECATED
// If the callable is a Py_Class, then find the number of arguments
// of the __init__ method.
- ArgInfo GetNumInitArguments() const;
+ ArgInfo GetNumInitArguments() const; // DEPRECATED
PythonObject operator()();
Index: lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
===================================================================
--- lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
@@ -31,6 +31,7 @@
using namespace lldb_private;
using namespace lldb;
using namespace lldb_private::python;
+using llvm::cantFail;
using llvm::Error;
using llvm::Expected;
@@ -47,6 +48,20 @@
return obj.get().AsLongLong();
}
+template <>
+Expected<std::string> python::As<std::string>(Expected<PythonObject> &&obj) {
+ if (!obj)
+ return obj.takeError();
+ PyObject *str_obj = PyObject_Str(obj.get().get());
+ if (!obj)
+ return llvm::make_error<PythonException>();
+ auto str = Take<PythonString>(str_obj);
+ auto utf8 = str.AsUTF8();
+ if (!utf8)
+ return utf8.takeError();
+ return utf8.get();
+}
+
void StructuredPythonObject::Serialize(llvm::json::OStream &s) const {
s.value(llvm::formatv("Python Obj: {0:X}", GetValue()).str());
}
@@ -657,16 +672,66 @@
}
PythonObject PythonDictionary::GetItemForKey(const PythonObject &key) const {
- if (IsAllocated() && key.IsValid())
- return PythonObject(PyRefType::Borrowed,
- PyDict_GetItem(m_py_obj, key.get()));
- return PythonObject();
+ auto item = GetItem(key);
+ if (!item) {
+ llvm::consumeError(item.takeError());
+ return PythonObject();
+ }
+ return std::move(item.get());
+}
+
+Expected<PythonObject>
+PythonDictionary::GetItem(const PythonObject &key) const {
+ if (!IsValid())
+ return nullDeref();
+#if PY_MAJOR_VERSION >= 3
+ PyObject *o = PyDict_GetItemWithError(m_py_obj, key.get());
+ if (PyErr_Occurred())
+ return exception();
+#else
+ PyObject *o = PyDict_GetItem(m_py_obj, key.get());
+#endif
+ if (!o)
+ return keyError();
+ return Retain<PythonObject>(o);
+}
+
+Expected<PythonObject> PythonDictionary::GetItem(const char *key) const {
+ if (!IsValid())
+ return nullDeref();
+ PyObject *o = PyDict_GetItemString(m_py_obj, key);
+ if (PyErr_Occurred())
+ return exception();
+ if (!o)
+ return keyError();
+ return Retain<PythonObject>(o);
+}
+
+Error PythonDictionary::SetItem(const PythonObject &key,
+ const PythonObject &value) const {
+ if (!IsValid() || !value.IsValid())
+ return nullDeref();
+ int r = PyDict_SetItem(m_py_obj, key.get(), value.get());
+ if (r < 0)
+ return exception();
+ return Error::success();
+}
+
+Error PythonDictionary::SetItem(const char *key,
+ const PythonObject &value) const {
+ if (!IsValid() || !value.IsValid())
+ return nullDeref();
+ int r = PyDict_SetItemString(m_py_obj, key, value.get());
+ if (r < 0)
+ return exception();
+ return Error::success();
}
void PythonDictionary::SetItemForKey(const PythonObject &key,
const PythonObject &value) {
- if (IsAllocated() && key.IsValid() && value.IsValid())
- PyDict_SetItem(m_py_obj, key.get(), value.get());
+ Error error = SetItem(key, value);
+ if (error)
+ llvm::consumeError(std::move(error));
}
StructuredData::DictionarySP
@@ -736,23 +801,89 @@
}
PythonCallable::ArgInfo PythonCallable::GetNumInitArguments() const {
- ArgInfo result = {0, false, false, false};
- if (!IsValid())
- return result;
-
- PythonObject __init__ = GetAttributeValue("__init__");
- if (__init__.IsValid() ) {
- auto __init_callable__ = __init__.AsType<PythonCallable>();
- if (__init_callable__.IsValid())
- return __init_callable__.GetNumArguments();
+ auto arginfo = GetInitArgInfo();
+ if (!arginfo) {
+ llvm::consumeError(arginfo.takeError());
+ return ArgInfo{};
}
- return result;
+ return arginfo.get();
}
-PythonCallable::ArgInfo PythonCallable::GetNumArguments() const {
- ArgInfo result = {0, false, false, false};
+Expected<PythonCallable::ArgInfo> PythonCallable::GetInitArgInfo() const {
if (!IsValid())
- return result;
+ return nullDeref();
+ auto init = As<PythonCallable>(GetAttribute("__init__"));
+ if (!init)
+ return init.takeError();
+ return init.get().GetArgInfo();
+}
+
+#if PY_MAJOR_VERSION >= 3 && PY_MINOR_VERSION >= 3
+static const char *get_arg_info_script = R"(
+from inspect import signature, Parameter, ismethod
+from collections import namedtuple
+ArgInfo = namedtuple('ArgInfo', ['count', 'has_varargs', 'is_bound_method'])
+def get_arg_info(f):
+ count = 0
+ varargs = False
+ for parameter in signature(f).parameters.values():
+ kind = parameter.kind
+ if kind in (Parameter.POSITIONAL_ONLY,
+ Parameter.POSITIONAL_OR_KEYWORD):
+ count += 1
+ elif kind == Parameter.VAR_POSITIONAL:
+ varargs = True
+ elif kind in (Parameter.KEYWORD_ONLY,
+ Parameter.KEYWORD_ONLY):
+ pass
+ else:
+ raise Exception(f'unknown parameter kind: {kind}')
+ return ArgInfo(count, varargs, ismethod(f))
+)";
+#endif
+
+Expected<PythonCallable::ArgInfo> PythonCallable::GetArgInfo() const {
+ ArgInfo result = {};
+ if (!IsValid())
+ return nullDeref();
+
+#if PY_MAJOR_VERSION >= 3 && PY_MINOR_VERSION >= 3
+
+ // this global is protected by the GIL
+ static PythonCallable get_arg_info;
+
+ if (!get_arg_info.IsValid()) {
+ PythonDictionary globals(PyInitialValue::Empty);
+
+ auto builtins = PythonModule::BuiltinsModule();
+ Error error = globals.SetItem("__builtins__", builtins);
+ if (error)
+ return std::move(error);
+ PyObject *o = PyRun_String(get_arg_info_script, Py_file_input,
+ globals.get(), globals.get());
+ if (!o)
+ return exception();
+ Take<PythonObject>(o);
+ auto function = As<PythonCallable>(globals.GetItem("get_arg_info"));
+ if (!function)
+ return function.takeError();
+ get_arg_info = std::move(function.get());
+ }
+
+ Expected<PythonObject> pyarginfo = get_arg_info.Call(*this);
+ if (!pyarginfo)
+ return pyarginfo.takeError();
+ result.count = cantFail(As<long long>(pyarginfo.get().GetAttribute("count")));
+ result.has_varargs =
+ cantFail(As<bool>(pyarginfo.get().GetAttribute("has_varargs")));
+ result.is_bound_method =
+ cantFail(As<bool>(pyarginfo.get().GetAttribute("is_bound_method")));
+
+ // FIXME emulate old broken behavior
+ if (result.is_bound_method)
+ result.count++;
+
+#else
PyObject *py_func_obj = m_py_obj;
if (PyMethod_Check(py_func_obj)) {
@@ -785,10 +916,21 @@
result.count = code->co_argcount;
result.has_varargs = !!(code->co_flags & CO_VARARGS);
- result.has_kwargs = !!(code->co_flags & CO_VARKEYWORDS);
+
+#endif
+
return result;
}
+PythonCallable::ArgInfo PythonCallable::GetNumArguments() const {
+ auto arginfo = GetArgInfo();
+ if (!arginfo) {
+ llvm::consumeError(arginfo.takeError());
+ return ArgInfo{};
+ }
+ return arginfo.get();
+}
+
PythonObject PythonCallable::operator()() {
return PythonObject(PyRefType::Owned, PyObject_CallObject(m_py_obj, nullptr));
}
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
