[Lldb-commits] [lldb] adbf64c - [LLDB][Python] remove ArgInfo::count
Lawrence D'Anna via lldb-commits
lldb-commits at lists.llvm.org
Mon Nov 4 12:50:09 PST 2019
Author: Lawrence D'Anna
Date: 2019-11-04T12:48:49-08:00
New Revision: adbf64ccc9e18278600ebaeadd8f0117eb8e64b1
URL: https://github.com/llvm/llvm-project/commit/adbf64ccc9e18278600ebaeadd8f0117eb8e64b1
DIFF: https://github.com/llvm/llvm-project/commit/adbf64ccc9e18278600ebaeadd8f0117eb8e64b1.diff
LOG: [LLDB][Python] remove ArgInfo::count
Summary:
This patch updates the last user of ArgInfo::count and deletes
it. I also delete `GetNumInitArguments()` and `GetInitArgInfo()`.
Classess are callables and `GetArgInfo()` should work on them.
On python 3 it already works, of course. `inspect` is good.
On python 2 we have to add yet another special case. But hey if
python 2 wasn't crufty we wouln't need python 3.
I also delete `is_bound_method` becuase it is unused.
This path is tested in `TestStepScripted.py`
Reviewers: labath, mgorny, JDevlieghere
Reviewed By: labath, JDevlieghere
Subscribers: lldb-commits
Tags: #lldb
Differential Revision: https://reviews.llvm.org/D69742
Added:
Modified:
lldb/scripts/Python/python-wrapper.swig
lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
lldb/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp
Removed:
################################################################################
diff --git a/lldb/scripts/Python/python-wrapper.swig b/lldb/scripts/Python/python-wrapper.swig
index 71a958acb72c..3a63165cf58d 100644
--- a/lldb/scripts/Python/python-wrapper.swig
+++ b/lldb/scripts/Python/python-wrapper.swig
@@ -291,20 +291,32 @@ LLDBSwigPythonCreateScriptedThreadPlan
if (!tp_arg.IsAllocated())
Py_RETURN_NONE;
+ llvm::Expected<PythonCallable::ArgInfo> arg_info = pfunc.GetArgInfo();
+ if (!arg_info) {
+ llvm::handleAllErrors(
+ arg_info.takeError(),
+ [&](PythonException &E) {
+ error_string.append(E.ReadBacktrace());
+ },
+ [&](const llvm::ErrorInfoBase &E) {
+ error_string.append(E.message());
+ });
+ Py_RETURN_NONE;
+ }
+
PythonObject result = {};
- size_t init_num_args = pfunc.GetNumInitArguments().count;
- if (init_num_args == 3) {
+ if (arg_info.get().max_positional_args == 2) {
if (args_impl != nullptr) {
error_string.assign("args passed, but __init__ does not take an args dictionary");
Py_RETURN_NONE;
}
result = pfunc(tp_arg, dict);
- } else if (init_num_args == 4) {
+ } else if (arg_info.get().max_positional_args >= 3) {
lldb::SBStructuredData *args_value = new lldb::SBStructuredData(args_impl);
PythonObject args_arg(PyRefType::Owned, SBTypeToSWIGWrapper(args_value));
result = pfunc(tp_arg, args_arg, dict);
} else {
- error_string.assign("wrong number of arguments in __init__, should be 1 or 2 (not including self & dict)");
+ error_string.assign("wrong number of arguments in __init__, should be 2 or 3 (not including self)");
Py_RETURN_NONE;
}
diff --git a/lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp b/lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
index ef5eb7a57d9c..9dee25c300ab 100644
--- a/lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
+++ b/lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
@@ -802,29 +802,11 @@ bool PythonCallable::Check(PyObject *py_obj) {
return PyCallable_Check(py_obj);
}
-PythonCallable::ArgInfo PythonCallable::GetNumInitArguments() const {
- auto arginfo = GetInitArgInfo();
- if (!arginfo) {
- llvm::consumeError(arginfo.takeError());
- return ArgInfo{};
- }
- return arginfo.get();
-}
-
-Expected<PythonCallable::ArgInfo> PythonCallable::GetInitArgInfo() const {
- if (!IsValid())
- 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'])
+ArgInfo = namedtuple('ArgInfo', ['count', 'has_varargs'])
def main(f):
count = 0
varargs = False
@@ -840,7 +822,7 @@ def main(f):
pass
else:
raise Exception(f'unknown parameter kind: {kind}')
- return ArgInfo(count, varargs, ismethod(f))
+ return ArgInfo(count, varargs)
)";
#endif
@@ -856,21 +838,27 @@ Expected<PythonCallable::ArgInfo> PythonCallable::GetArgInfo() const {
Expected<PythonObject> pyarginfo = get_arg_info(*this);
if (!pyarginfo)
return pyarginfo.takeError();
- result.count = cantFail(As<long long>(pyarginfo.get().GetAttribute("count")));
- result.has_varargs =
+ long long count =
+ cantFail(As<long long>(pyarginfo.get().GetAttribute("count")));
+ bool has_varargs =
cantFail(As<bool>(pyarginfo.get().GetAttribute("has_varargs")));
- bool is_method =
- cantFail(As<bool>(pyarginfo.get().GetAttribute("is_bound_method")));
- result.max_positional_args =
- result.has_varargs ? ArgInfo::UNBOUNDED : result.count;
-
- // FIXME emulate old broken behavior
- if (is_method)
- result.count++;
+ result.max_positional_args = has_varargs ? ArgInfo::UNBOUNDED : count;
#else
+ PyObject *py_func_obj;
bool is_bound_method = false;
- PyObject *py_func_obj = m_py_obj;
+ bool is_class = false;
+
+ if (PyType_Check(m_py_obj) || PyClass_Check(m_py_obj)) {
+ auto init = GetAttribute("__init__");
+ if (!init)
+ return init.takeError();
+ py_func_obj = init.get().get();
+ is_class = true;
+ } else {
+ py_func_obj = m_py_obj;
+ }
+
if (PyMethod_Check(py_func_obj)) {
py_func_obj = PyMethod_GET_FUNCTION(py_func_obj);
PythonObject im_self = GetAttributeValue("im_self");
@@ -899,11 +887,11 @@ Expected<PythonCallable::ArgInfo> PythonCallable::GetArgInfo() const {
if (!code)
return result;
- result.count = code->co_argcount;
- result.has_varargs = !!(code->co_flags & CO_VARARGS);
- result.max_positional_args = result.has_varargs
- ? ArgInfo::UNBOUNDED
- : (result.count - (int)is_bound_method);
+ auto count = code->co_argcount;
+ bool has_varargs = !!(code->co_flags & CO_VARARGS);
+ result.max_positional_args =
+ has_varargs ? ArgInfo::UNBOUNDED
+ : (count - (int)is_bound_method) - (int)is_class;
#endif
@@ -913,15 +901,6 @@ Expected<PythonCallable::ArgInfo> PythonCallable::GetArgInfo() const {
constexpr unsigned
PythonCallable::ArgInfo::UNBOUNDED; // FIXME delete after c++17
-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));
}
diff --git a/lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h b/lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
index 302901664ec0..05f280ae6988 100644
--- a/lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
+++ b/lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
@@ -619,30 +619,12 @@ class PythonCallable : public TypedPythonObject<PythonCallable> {
* function and can accept an arbitrary number */
unsigned max_positional_args;
static constexpr unsigned UNBOUNDED = UINT_MAX; // FIXME c++17 inline
- /* 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
};
static bool Check(PyObject *py_obj);
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; // DEPRECATED
-
PythonObject operator()();
PythonObject operator()(std::initializer_list<PyObject *> args);
diff --git a/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp b/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
index 15843feae57c..e7d9443c5fdb 100644
--- a/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
+++ b/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
@@ -807,8 +807,10 @@ ScriptInterpreterPythonImpl::GetMaxPositionalArgumentsForCallable(
llvm::inconvertibleErrorCode(),
"can't find callable: %s", callable_name.str().c_str());
}
- PythonCallable::ArgInfo arg_info = pfunc.GetNumArguments();
- return arg_info.max_positional_args;
+ llvm::Expected<PythonCallable::ArgInfo> arg_info = pfunc.GetArgInfo();
+ if (!arg_info)
+ return arg_info.takeError();
+ return arg_info.get().max_positional_args;
}
static std::string GenerateUniqueName(const char *base_name_wanted,
diff --git a/lldb/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp b/lldb/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp
index 7481482fd8fc..ec5d02ee04d0 100644
--- a/lldb/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp
+++ b/lldb/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp
@@ -640,9 +640,7 @@ TEST_F(PythonDataObjectsTest, TestCallable) {
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().max_positional_args, 1u);
- EXPECT_EQ(arginfo.get().has_varargs, false);
}
{
@@ -652,9 +650,7 @@ TEST_F(PythonDataObjectsTest, TestCallable) {
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().max_positional_args, 2u);
- EXPECT_EQ(arginfo.get().has_varargs, false);
}
{
@@ -664,9 +660,7 @@ TEST_F(PythonDataObjectsTest, TestCallable) {
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().max_positional_args, 2u);
- EXPECT_EQ(arginfo.get().has_varargs, false);
}
{
@@ -676,10 +670,8 @@ TEST_F(PythonDataObjectsTest, TestCallable) {
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().max_positional_args,
PythonCallable::ArgInfo::UNBOUNDED);
- EXPECT_EQ(arginfo.get().has_varargs, true);
}
{
@@ -689,10 +681,8 @@ TEST_F(PythonDataObjectsTest, TestCallable) {
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().max_positional_args,
PythonCallable::ArgInfo::UNBOUNDED);
- EXPECT_EQ(arginfo.get().has_varargs, true);
}
{
@@ -713,6 +703,16 @@ bar_bound = Foo().bar
bar_class = Foo().classbar
bar_static = Foo().staticbar
bar_unbound = Foo.bar
+
+
+class OldStyle:
+ def __init__(self, one, two, three):
+ pass
+
+class NewStyle(object):
+ def __init__(self, one, two, three):
+ pass
+
)";
PyObject *o =
PyRun_String(script, Py_file_input, globals.get(), globals.get());
@@ -723,38 +723,43 @@ bar_unbound = Foo.bar
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().max_positional_args, 1u);
- EXPECT_EQ(arginfo.get().has_varargs, false);
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().max_positional_args, 2u);
- EXPECT_EQ(arginfo.get().has_varargs, false);
auto bar_class = As<PythonCallable>(globals.GetItem("bar_class"));
ASSERT_THAT_EXPECTED(bar_class, llvm::Succeeded());
arginfo = bar_class.get().GetArgInfo();
ASSERT_THAT_EXPECTED(arginfo, llvm::Succeeded());
EXPECT_EQ(arginfo.get().max_positional_args, 1u);
- EXPECT_EQ(arginfo.get().has_varargs, false);
auto bar_static = As<PythonCallable>(globals.GetItem("bar_static"));
ASSERT_THAT_EXPECTED(bar_static, llvm::Succeeded());
arginfo = bar_static.get().GetArgInfo();
ASSERT_THAT_EXPECTED(arginfo, llvm::Succeeded());
EXPECT_EQ(arginfo.get().max_positional_args, 1u);
- EXPECT_EQ(arginfo.get().has_varargs, false);
auto obj = As<PythonCallable>(globals.GetItem("obj"));
ASSERT_THAT_EXPECTED(obj, llvm::Succeeded());
arginfo = obj.get().GetArgInfo();
ASSERT_THAT_EXPECTED(arginfo, llvm::Succeeded());
EXPECT_EQ(arginfo.get().max_positional_args, 1u);
- EXPECT_EQ(arginfo.get().has_varargs, false);
+
+ auto oldstyle = As<PythonCallable>(globals.GetItem("OldStyle"));
+ ASSERT_THAT_EXPECTED(oldstyle, llvm::Succeeded());
+ arginfo = oldstyle.get().GetArgInfo();
+ ASSERT_THAT_EXPECTED(arginfo, llvm::Succeeded());
+ EXPECT_EQ(arginfo.get().max_positional_args, 3u);
+
+ auto newstyle = As<PythonCallable>(globals.GetItem("NewStyle"));
+ ASSERT_THAT_EXPECTED(newstyle, llvm::Succeeded());
+ arginfo = newstyle.get().GetArgInfo();
+ ASSERT_THAT_EXPECTED(arginfo, llvm::Succeeded());
+ EXPECT_EQ(arginfo.get().max_positional_args, 3u);
}
#if PY_MAJOR_VERSION >= 3 && PY_MINOR_VERSION >= 3
@@ -767,9 +772,7 @@ bar_unbound = Foo.bar
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().max_positional_args, 1u);
- EXPECT_EQ(arginfo.get().has_varargs, false);
}
#endif
More information about the lldb-commits
mailing list