lawrence_danna updated this revision to Diff 259659. lawrence_danna added a comment.
fix python2 projblems Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78462/new/ https://reviews.llvm.org/D78462 Files: lldb/bindings/python/python-typemaps.swig lldb/bindings/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
Index: lldb/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp =================================================================== --- lldb/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp +++ lldb/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp @@ -123,13 +123,11 @@ EXPECT_TRUE(major_version_field.IsAllocated()); EXPECT_TRUE(minor_version_field.IsAllocated()); - PythonInteger major_version_value = - major_version_field.AsType<PythonInteger>(); - PythonInteger minor_version_value = - minor_version_field.AsType<PythonInteger>(); + auto major_version_value = As<long long>(major_version_field); + auto minor_version_value = As<long long>(minor_version_field); - EXPECT_EQ(PY_MAJOR_VERSION, major_version_value.GetInteger()); - EXPECT_EQ(PY_MINOR_VERSION, minor_version_value.GetInteger()); + EXPECT_THAT_EXPECTED(major_version_value, llvm::HasValue(PY_MAJOR_VERSION)); + EXPECT_THAT_EXPECTED(minor_version_value, llvm::HasValue(PY_MINOR_VERSION)); } TEST_F(PythonDataObjectsTest, TestGlobalNameResolutionWithDot) { @@ -137,16 +135,14 @@ EXPECT_TRUE(sys_path.IsAllocated()); EXPECT_TRUE(PythonList::Check(sys_path.get())); - PythonInteger version_major = - m_main_module.ResolveName("sys.version_info.major") - .AsType<PythonInteger>(); - PythonInteger version_minor = - m_main_module.ResolveName("sys.version_info.minor") - .AsType<PythonInteger>(); - EXPECT_TRUE(version_major.IsAllocated()); - EXPECT_TRUE(version_minor.IsAllocated()); - EXPECT_EQ(PY_MAJOR_VERSION, version_major.GetInteger()); - EXPECT_EQ(PY_MINOR_VERSION, version_minor.GetInteger()); + auto version_major = + As<long long>(m_main_module.ResolveName("sys.version_info.major")); + + auto version_minor = + As<long long>(m_main_module.ResolveName("sys.version_info.minor")); + + EXPECT_THAT_EXPECTED(version_major, llvm::HasValue(PY_MAJOR_VERSION)); + EXPECT_THAT_EXPECTED(version_minor, llvm::HasValue(PY_MINOR_VERSION)); } TEST_F(PythonDataObjectsTest, TestDictionaryResolutionWithDot) { @@ -155,14 +151,14 @@ dict.SetItemForKey(PythonString("sys"), m_sys_module); // Now use that dictionary to resolve `sys.version_info.major` - PythonInteger version_major = - PythonObject::ResolveNameWithDictionary("sys.version_info.major", dict) - .AsType<PythonInteger>(); - PythonInteger version_minor = - PythonObject::ResolveNameWithDictionary("sys.version_info.minor", dict) - .AsType<PythonInteger>(); - EXPECT_EQ(PY_MAJOR_VERSION, version_major.GetInteger()); - EXPECT_EQ(PY_MINOR_VERSION, version_minor.GetInteger()); + auto version_major = As<long long>( + PythonObject::ResolveNameWithDictionary("sys.version_info.major", dict)); + + auto version_minor = As<long long>( + PythonObject::ResolveNameWithDictionary("sys.version_info.minor", dict)); + + EXPECT_THAT_EXPECTED(version_major, llvm::HasValue(PY_MAJOR_VERSION)); + EXPECT_THAT_EXPECTED(version_minor, llvm::HasValue(PY_MINOR_VERSION)); } TEST_F(PythonDataObjectsTest, TestPythonInteger) { @@ -176,7 +172,8 @@ PythonInteger python_int(PyRefType::Owned, py_int); EXPECT_EQ(PyObjectType::Integer, python_int.GetObjectType()); - EXPECT_EQ(12, python_int.GetInteger()); + auto python_int_value = As<long long>(python_int); + EXPECT_THAT_EXPECTED(python_int_value, llvm::HasValue(12)); #endif // Verify that `PythonInteger` works correctly when given a PyLong object. @@ -187,12 +184,14 @@ // Verify that you can reset the value and that it is reflected properly. python_long.SetInteger(40); - EXPECT_EQ(40, python_long.GetInteger()); + auto e = As<long long>(python_long); + EXPECT_THAT_EXPECTED(e, llvm::HasValue(40)); // Test that creating a `PythonInteger` object works correctly with the // int constructor. PythonInteger constructed_int(7); - EXPECT_EQ(7, constructed_int.GetInteger()); + auto value = As<long long>(constructed_int); + EXPECT_THAT_EXPECTED(value, llvm::HasValue(7)); } TEST_F(PythonDataObjectsTest, TestPythonBoolean) { @@ -339,7 +338,8 @@ PythonInteger chk_int(PyRefType::Borrowed, chk_value1.get()); PythonString chk_str(PyRefType::Borrowed, chk_value2.get()); - EXPECT_EQ(long_value0, chk_int.GetInteger()); + auto chkint = As<long long>(chk_value1); + ASSERT_THAT_EXPECTED(chkint, llvm::HasValue(long_value0)); EXPECT_EQ(string_value1, chk_str.GetString()); } @@ -367,7 +367,8 @@ PythonInteger chk_int(PyRefType::Borrowed, chk_value1.get()); PythonString chk_str(PyRefType::Borrowed, chk_value2.get()); - EXPECT_EQ(long_value0, chk_int.GetInteger()); + auto e = As<long long>(chk_int); + EXPECT_THAT_EXPECTED(e, llvm::HasValue(long_value0)); EXPECT_EQ(string_value1, chk_str.GetString()); } @@ -487,10 +488,10 @@ EXPECT_TRUE(PythonInteger::Check(chk_value1.get())); EXPECT_TRUE(PythonString::Check(chk_value2.get())); - PythonInteger chk_int(PyRefType::Borrowed, chk_value1.get()); PythonString chk_str(PyRefType::Borrowed, chk_value2.get()); + auto chkint = As<long long>(chk_value1); - EXPECT_EQ(value_0, chk_int.GetInteger()); + EXPECT_THAT_EXPECTED(chkint, llvm::HasValue(value_0)); EXPECT_EQ(value_1, chk_str.GetString()); } @@ -524,10 +525,10 @@ EXPECT_TRUE(PythonInteger::Check(chk_value1.get())); EXPECT_TRUE(PythonString::Check(chk_value2.get())); - PythonInteger chk_int(PyRefType::Borrowed, chk_value1.get()); + auto chkint = As<long long>(chk_value1); PythonString chk_str(PyRefType::Borrowed, chk_value2.get()); - EXPECT_EQ(value_0, chk_int.GetInteger()); + EXPECT_THAT_EXPECTED(chkint, llvm::HasValue(value_0)); EXPECT_EQ(value_1, chk_str.GetString()); } @@ -594,10 +595,9 @@ EXPECT_TRUE(py_int.HasAttribute("numerator")); EXPECT_FALSE(py_int.HasAttribute("this_should_not_exist")); - PythonInteger numerator_attr = - py_int.GetAttributeValue("numerator").AsType<PythonInteger>(); - EXPECT_TRUE(numerator_attr.IsAllocated()); - EXPECT_EQ(42, numerator_attr.GetInteger()); + auto numerator_attr = As<long long>(py_int.GetAttributeValue("numerator")); + + EXPECT_THAT_EXPECTED(numerator_attr, llvm::HasValue(42)); } TEST_F(PythonDataObjectsTest, TestExtractingUInt64ThroughStructuredData) { Index: lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp =================================================================== --- lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp +++ lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp @@ -3150,20 +3150,15 @@ if (PyErr_Occurred()) PyErr_Clear(); - // right now we know this function exists and is callable.. - PythonObject py_return( - PyRefType::Owned, - PyObject_CallMethod(implementor.get(), callee_name, nullptr)); + long long py_return = unwrapOrSetPythonException( + As<long long>(implementor.CallMethod(callee_name))); // if it fails, print the error but otherwise go on if (PyErr_Occurred()) { PyErr_Print(); PyErr_Clear(); - } - - if (py_return.IsAllocated() && PythonInteger::Check(py_return.get())) { - PythonInteger int_value(PyRefType::Borrowed, py_return.get()); - result = int_value.GetInteger(); + } else { + result = py_return; } return result; Index: lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h =================================================================== --- lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h +++ lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h @@ -360,15 +360,12 @@ return !!r; } - llvm::Expected<long long> AsLongLong() { - if (!m_py_obj) - return nullDeref(); - assert(!PyErr_Occurred()); - long long r = PyLong_AsLongLong(m_py_obj); - if (PyErr_Occurred()) - return exception(); - return r; - } + llvm::Expected<long long> AsLongLong() const; + + llvm::Expected<long long> AsUnsignedLongLong() const; + + // wraps on overflow, instead of raising an error. + llvm::Expected<unsigned long long> AsModuloUnsignedLongLong() const; llvm::Expected<bool> IsInstance(const PythonObject &cls) { if (!m_py_obj || !cls.IsValid()) @@ -399,6 +396,10 @@ template <> llvm::Expected<long long> As<long long>(llvm::Expected<PythonObject> &&obj); +template <> +llvm::Expected<unsigned long long> +As<unsigned long long>(llvm::Expected<PythonObject> &&obj); + template <> llvm::Expected<std::string> As<std::string>(llvm::Expected<PythonObject> &&obj); @@ -491,8 +492,6 @@ static bool Check(PyObject *py_obj); static void Convert(PyRefType &type, PyObject *&py_obj); - int64_t GetInteger() const; - void SetInteger(int64_t value); StructuredData::IntegerSP CreateStructuredInteger() const; Index: lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp =================================================================== --- lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp +++ lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp @@ -44,7 +44,15 @@ Expected<long long> python::As<long long>(Expected<PythonObject> &&obj) { if (!obj) return obj.takeError(); - return obj.get().AsLongLong(); + return obj->AsLongLong(); +} + +template <> +Expected<unsigned long long> +python::As<unsigned long long>(Expected<PythonObject> &&obj) { + if (!obj) + return obj.takeError(); + return obj->AsUnsignedLongLong(); } template <> @@ -61,6 +69,55 @@ return std::string(utf8.get()); } +Expected<long long> PythonObject::AsLongLong() const { + if (!m_py_obj) + return nullDeref(); +#if PY_MAJOR_VERSION < 3 + if (!PyLong_Check(m_py_obj)) { + PythonInteger i(PyRefType::Borrowed, m_py_obj); + return i.AsLongLong(); + } +#endif + assert(!PyErr_Occurred()); + long long r = PyLong_AsLongLong(m_py_obj); + if (PyErr_Occurred()) + return exception(); + return r; +} + +Expected<long long> PythonObject::AsUnsignedLongLong() const { + if (!m_py_obj) + return nullDeref(); +#if PY_MAJOR_VERSION < 3 + if (!PyLong_Check(m_py_obj)) { + PythonInteger i(PyRefType::Borrowed, m_py_obj); + return i.AsUnsignedLongLong(); + } +#endif + assert(!PyErr_Occurred()); + long long r = PyLong_AsUnsignedLongLong(m_py_obj); + if (PyErr_Occurred()) + return exception(); + return r; +} + +// wraps on overflow, instead of raising an error. +Expected<unsigned long long> PythonObject::AsModuloUnsignedLongLong() const { + if (!m_py_obj) + return nullDeref(); +#if PY_MAJOR_VERSION < 3 + if (!PyLong_Check(m_py_obj)) { + PythonInteger i(PyRefType::Borrowed, m_py_obj); + return i.AsModuloUnsignedLongLong(); + } +#endif + assert(!PyErr_Occurred()); + unsigned long long r = PyLong_AsUnsignedLongLongMask(m_py_obj); + if (PyErr_Occurred()) + return exception(); + return r; +} + void StructuredPythonObject::Serialize(llvm::json::OStream &s) const { s.value(llvm::formatv("Python Obj: {0:X}", GetValue()).str()); } @@ -463,32 +520,22 @@ #endif } -int64_t PythonInteger::GetInteger() const { - if (m_py_obj) { - assert(PyLong_Check(m_py_obj) && - "PythonInteger::GetInteger has a PyObject that isn't a PyLong"); - - int overflow = 0; - int64_t result = PyLong_AsLongLongAndOverflow(m_py_obj, &overflow); - if (overflow != 0) { - // We got an integer that overflows, like 18446744072853913392L we can't - // use PyLong_AsLongLong() as it will return 0xffffffffffffffff. If we - // use the unsigned long long it will work as expected. - const uint64_t uval = PyLong_AsUnsignedLongLong(m_py_obj); - result = static_cast<int64_t>(uval); - } - return result; - } - return UINT64_MAX; -} - void PythonInteger::SetInteger(int64_t value) { *this = Take<PythonInteger>(PyLong_FromLongLong(value)); } StructuredData::IntegerSP PythonInteger::CreateStructuredInteger() const { StructuredData::IntegerSP result(new StructuredData::Integer); - result->SetValue(GetInteger()); + // FIXME this is really not ideal. Errors are silently converted to 0 + // and overflows are silently wrapped. But we'd need larger changes + // to StructuredData to fix it, so that's how it is for now. + llvm::Expected<unsigned long long> value = AsModuloUnsignedLongLong(); + if (!value) { + llvm::consumeError(value.takeError()); + result->SetValue(0); + } else { + result->SetValue(value.get()); + } return result; } Index: lldb/bindings/python/python-wrapper.swig =================================================================== --- lldb/bindings/python/python-wrapper.swig +++ lldb/bindings/python/python-wrapper.swig @@ -444,6 +444,7 @@ if (PyErr_Occurred()) { PyErr_Print(); + PyErr_Clear(); return 0; } @@ -457,11 +458,13 @@ return 1; } - PythonInteger int_result = result.AsType<PythonInteger>(); - if (!int_result.IsAllocated()) - return 0; + long long ret_val = unwrapOrSetPythonException(As<long long>(result)); - unsigned int ret_val = int_result.GetInteger(); + if (PyErr_Occurred()) { + PyErr_Print(); + PyErr_Clear(); + return 0; + } return ret_val; } @@ -515,26 +518,17 @@ return 0; } - PythonObject result; - + size_t ret_val; if (arg_info.get().max_positional_args < 1) - result = pfunc(); + ret_val = unwrapOrSetPythonException(As<long long>(pfunc.Call())); else - result = pfunc(PythonInteger(max)); - - if (!result.IsAllocated()) - return 0; - - PythonInteger int_result = result.AsType<PythonInteger>(); - if (!int_result.IsAllocated()) - return 0; - - size_t ret_val = int_result.GetInteger(); + ret_val = unwrapOrSetPythonException(As<long long>(pfunc.Call(PythonInteger(max)))); - if (PyErr_Occurred()) //FIXME use Expected to catch python exceptions + if (PyErr_Occurred()) { PyErr_Print(); PyErr_Clear(); + return 0; } if (arg_info.get().max_positional_args < 1) @@ -588,16 +582,15 @@ if (!pfunc.IsAllocated()) return UINT32_MAX; - PythonObject result = pfunc(PythonString(child_name)); + llvm::Expected<PythonObject> result = pfunc.Call(PythonString(child_name)); - if (!result.IsAllocated()) - return UINT32_MAX; + long long retval = unwrapOrSetPythonException(As<long long>(std::move(result))); - PythonInteger int_result = result.AsType<PythonInteger>(); - if (!int_result.IsAllocated()) + if (PyErr_Occurred()) { + PyErr_Clear(); // FIXME print this? do something else return UINT32_MAX; + } - int64_t retval = int_result.GetInteger(); if (retval >= 0) return (uint32_t)retval; Index: lldb/bindings/python/python-typemaps.swig =================================================================== --- lldb/bindings/python/python-typemaps.swig +++ lldb/bindings/python/python-typemaps.swig @@ -59,37 +59,25 @@ $result = list.release(); } - %typemap(in) lldb::tid_t { - if (PythonInteger::Check($input)) - { - PythonInteger py_int(PyRefType::Borrowed, $input); - $1 = static_cast<lldb::tid_t>(py_int.GetInteger()); - } - else - { - PyErr_SetString(PyExc_ValueError, "Expecting an integer"); + PythonObject obj = Retain<PythonObject>($input); + lldb::tid_t value = unwrapOrSetPythonException(As<unsigned long long>(obj)); + if (PyErr_Occurred()) return nullptr; - } + $1 = value; } %typemap(in) lldb::StateType { - if (PythonInteger::Check($input)) - { - PythonInteger py_int(PyRefType::Borrowed, $input); - int64_t state_type_value = py_int.GetInteger() ; - - if (state_type_value > lldb::StateType::kLastStateType) { - PyErr_SetString(PyExc_ValueError, "Not a valid StateType value"); - return nullptr; - } - $1 = static_cast<lldb::StateType>(state_type_value); - } - else - { - PyErr_SetString(PyExc_ValueError, "Expecting an integer"); + PythonObject obj = Retain<PythonObject>($input); + unsigned long long state_type_value = + unwrapOrSetPythonException(As<unsigned long long>(obj)); + if (PyErr_Occurred()) + return nullptr; + if (state_type_value > lldb::StateType::kLastStateType) { + PyErr_SetString(PyExc_ValueError, "Not a valid StateType value"); return nullptr; } + $1 = static_cast<lldb::StateType>(state_type_value); } /* Typemap definitions to allow SWIG to properly handle char buffer. */
_______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits