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 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::GetInitArgInfo() const { - if (!IsValid()) - return nullDeref(); - auto init = As(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::GetArgInfo() const { Expected pyarginfo = get_arg_info(*this); if (!pyarginfo) return pyarginfo.takeError(); - result.count = cantFail(As(pyarginfo.get().GetAttribute("count"))); - result.has_varargs = + long long count = + cantFail(As(pyarginfo.get().GetAttribute("count"))); + bool has_varargs = cantFail(As(pyarginfo.get().GetAttribute("has_varargs"))); - bool is_method = - cantFail(As(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::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::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 @@ public: * 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 GetArgInfo() const; - llvm::Expected 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 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 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(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(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(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(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(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(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(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(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(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(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(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