[Lldb-commits] [lldb] 478619c - Revert "get rid of PythonInteger::GetInteger()"
Muhammad Omair Javaid via lldb-commits
lldb-commits at lists.llvm.org
Wed Apr 22 16:38:42 PDT 2020
Author: Muhammad Omair Javaid
Date: 2020-04-23T04:38:32+05:00
New Revision: 478619cf9a24ad8eac806959ca6a289a9beb71ae
URL: https://github.com/llvm/llvm-project/commit/478619cf9a24ad8eac806959ca6a289a9beb71ae
DIFF: https://github.com/llvm/llvm-project/commit/478619cf9a24ad8eac806959ca6a289a9beb71ae.diff
LOG: Revert "get rid of PythonInteger::GetInteger()"
This reverts commit 7375212172951d2fc283c81d03c1a8588c3280c6.
This causes multiple test failures on LLDB AArch64 Linux buildbot.
http://lab.llvm.org:8011/builders/lldb-aarch64-ubuntu/builds/3695
Differential Revision: https://reviews.llvm.org/D78462
Added:
Modified:
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
Removed:
################################################################################
diff --git a/lldb/bindings/python/python-typemaps.swig b/lldb/bindings/python/python-typemaps.swig
index c08aeab71f78..46dcaf611a4f 100644
--- a/lldb/bindings/python/python-typemaps.swig
+++ b/lldb/bindings/python/python-typemaps.swig
@@ -59,25 +59,37 @@
$result = list.release();
}
+
%typemap(in) lldb::tid_t {
- PythonObject obj = Retain<PythonObject>($input);
- lldb::tid_t value = unwrapOrSetPythonException(As<unsigned long long>(obj));
- if (PyErr_Occurred())
+ 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");
return nullptr;
- $1 = value;
+ }
}
%typemap(in) lldb::StateType {
- 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");
+ 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");
return nullptr;
}
- $1 = static_cast<lldb::StateType>(state_type_value);
}
/* Typemap definitions to allow SWIG to properly handle char buffer. */
diff --git a/lldb/bindings/python/python-wrapper.swig b/lldb/bindings/python/python-wrapper.swig
index f9e89373fe25..3a63165cf58d 100644
--- a/lldb/bindings/python/python-wrapper.swig
+++ b/lldb/bindings/python/python-wrapper.swig
@@ -444,7 +444,6 @@ LLDBSwigPythonCallBreakpointResolver
if (PyErr_Occurred())
{
PyErr_Print();
- PyErr_Clear();
return 0;
}
@@ -458,13 +457,11 @@ LLDBSwigPythonCallBreakpointResolver
return 1;
}
- long long ret_val = unwrapOrSetPythonException(As<long long>(result));
-
- if (PyErr_Occurred()) {
- PyErr_Print();
- PyErr_Clear();
+ PythonInteger int_result = result.AsType<PythonInteger>();
+ if (!int_result.IsAllocated())
return 0;
- }
+
+ unsigned int ret_val = int_result.GetInteger();
return ret_val;
}
@@ -518,17 +515,26 @@ LLDBSwigPython_CalculateNumChildren
return 0;
}
- size_t ret_val;
+ PythonObject result;
+
if (arg_info.get().max_positional_args < 1)
- ret_val = unwrapOrSetPythonException(As<long long>(pfunc.Call()));
+ result = pfunc();
else
- ret_val = unwrapOrSetPythonException(As<long long>(pfunc.Call(PythonInteger(max))));
+ result = pfunc(PythonInteger(max));
- if (PyErr_Occurred())
+ if (!result.IsAllocated())
+ return 0;
+
+ PythonInteger int_result = result.AsType<PythonInteger>();
+ if (!int_result.IsAllocated())
+ return 0;
+
+ size_t ret_val = int_result.GetInteger();
+
+ if (PyErr_Occurred()) //FIXME use Expected to catch python exceptions
{
PyErr_Print();
PyErr_Clear();
- return 0;
}
if (arg_info.get().max_positional_args < 1)
@@ -582,15 +588,16 @@ LLDBSwigPython_GetIndexOfChildWithName
if (!pfunc.IsAllocated())
return UINT32_MAX;
- llvm::Expected<PythonObject> result = pfunc.Call(PythonString(child_name));
+ PythonObject result = pfunc(PythonString(child_name));
- long long retval = unwrapOrSetPythonException(As<long long>(std::move(result)));
+ if (!result.IsAllocated())
+ return UINT32_MAX;
- if (PyErr_Occurred()) {
- PyErr_Clear(); // FIXME print this? do something else
+ PythonInteger int_result = result.AsType<PythonInteger>();
+ if (!int_result.IsAllocated())
return UINT32_MAX;
- }
+ int64_t retval = int_result.GetInteger();
if (retval >= 0)
return (uint32_t)retval;
diff --git a/lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp b/lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
index 3f00f671d88d..40ed22aceebf 100644
--- a/lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
+++ b/lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
@@ -44,15 +44,7 @@ template <>
Expected<long long> python::As<long long>(Expected<PythonObject> &&obj) {
if (!obj)
return obj.takeError();
- return obj->AsLongLong();
-}
-
-template <>
-Expected<unsigned long long>
-python::As<unsigned long long>(Expected<PythonObject> &&obj) {
- if (!obj)
- return obj.takeError();
- return obj->AsUnsignedLongLong();
+ return obj.get().AsLongLong();
}
template <>
@@ -471,22 +463,32 @@ void PythonInteger::Convert(PyRefType &type, PyObject *&py_obj) {
#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);
- // 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());
- }
+ result->SetValue(GetInteger());
return result;
}
diff --git a/lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h b/lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
index b09f42e0d540..16896803e136 100644
--- a/lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
+++ b/lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
@@ -370,27 +370,6 @@ class PythonObject {
return r;
}
- llvm::Expected<long long> AsUnsignedLongLong() {
- if (!m_py_obj)
- return nullDeref();
- assert(!PyErr_Occurred());
- long long r = PyLong_AsUnsignedLongLong(m_py_obj);
- if (PyErr_Occurred())
- return exception();
- return r;
- }
-
- llvm::Expected<unsigned long long> AsModuloUnsignedLongLong() const {
- // wraps on overflow, instead of raising an error.
- if (!m_py_obj)
- return nullDeref();
- assert(!PyErr_Occurred());
- unsigned long long r = PyLong_AsUnsignedLongLongMask(m_py_obj);
- if (PyErr_Occurred())
- return exception();
- return r;
- }
-
llvm::Expected<bool> IsInstance(const PythonObject &cls) {
if (!m_py_obj || !cls.IsValid())
return nullDeref();
@@ -420,10 +399,6 @@ template <> llvm::Expected<bool> As<bool>(llvm::Expected<PythonObject> &&obj);
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);
@@ -516,6 +491,8 @@ class PythonInteger : public TypedPythonObject<PythonInteger> {
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;
diff --git a/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp b/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
index 6f266772624a..c53b3bd0fb65 100644
--- a/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
+++ b/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
@@ -3150,15 +3150,20 @@ uint32_t ScriptInterpreterPythonImpl::GetFlagsForCommandObject(
if (PyErr_Occurred())
PyErr_Clear();
- long long py_return = unwrapOrSetPythonException(
- As<long long>(implementor.CallMethod(callee_name)));
+ // right now we know this function exists and is callable..
+ PythonObject py_return(
+ PyRefType::Owned,
+ PyObject_CallMethod(implementor.get(), callee_name, nullptr));
// if it fails, print the error but otherwise go on
if (PyErr_Occurred()) {
PyErr_Print();
PyErr_Clear();
- } else {
- result = py_return;
+ }
+
+ if (py_return.IsAllocated() && PythonInteger::Check(py_return.get())) {
+ PythonInteger int_value(PyRefType::Borrowed, py_return.get());
+ result = int_value.GetInteger();
}
return result;
diff --git a/lldb/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp b/lldb/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp
index 75a1f5e67d32..fe3b423b4842 100644
--- a/lldb/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp
+++ b/lldb/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp
@@ -123,11 +123,13 @@ TEST_F(PythonDataObjectsTest, TestInstanceNameResolutionNoDot) {
EXPECT_TRUE(major_version_field.IsAllocated());
EXPECT_TRUE(minor_version_field.IsAllocated());
- auto major_version_value = As<long long>(major_version_field);
- auto minor_version_value = As<long long>(minor_version_field);
+ PythonInteger major_version_value =
+ major_version_field.AsType<PythonInteger>();
+ PythonInteger minor_version_value =
+ minor_version_field.AsType<PythonInteger>();
- EXPECT_THAT_EXPECTED(major_version_value, llvm::HasValue(PY_MAJOR_VERSION));
- EXPECT_THAT_EXPECTED(minor_version_value, llvm::HasValue(PY_MINOR_VERSION));
+ EXPECT_EQ(PY_MAJOR_VERSION, major_version_value.GetInteger());
+ EXPECT_EQ(PY_MINOR_VERSION, minor_version_value.GetInteger());
}
TEST_F(PythonDataObjectsTest, TestGlobalNameResolutionWithDot) {
@@ -135,14 +137,16 @@ TEST_F(PythonDataObjectsTest, TestGlobalNameResolutionWithDot) {
EXPECT_TRUE(sys_path.IsAllocated());
EXPECT_TRUE(PythonList::Check(sys_path.get()));
- 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));
+ 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());
}
TEST_F(PythonDataObjectsTest, TestDictionaryResolutionWithDot) {
@@ -151,14 +155,14 @@ TEST_F(PythonDataObjectsTest, TestDictionaryResolutionWithDot) {
dict.SetItemForKey(PythonString("sys"), m_sys_module);
// Now use that dictionary to resolve `sys.version_info.major`
- 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));
+ 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());
}
TEST_F(PythonDataObjectsTest, TestPythonInteger) {
@@ -172,8 +176,7 @@ TEST_F(PythonDataObjectsTest, TestPythonInteger) {
PythonInteger python_int(PyRefType::Owned, py_int);
EXPECT_EQ(PyObjectType::Integer, python_int.GetObjectType());
- auto python_int_value = As<long long>(python_int);
- EXPECT_THAT_EXPECTED(python_int_value, llvm::HasValue(12));
+ EXPECT_EQ(12, python_int.GetInteger());
#endif
// Verify that `PythonInteger` works correctly when given a PyLong object.
@@ -184,14 +187,12 @@ TEST_F(PythonDataObjectsTest, TestPythonInteger) {
// Verify that you can reset the value and that it is reflected properly.
python_long.SetInteger(40);
- auto e = As<long long>(python_long);
- EXPECT_THAT_EXPECTED(e, llvm::HasValue(40));
+ EXPECT_EQ(40, python_long.GetInteger());
// Test that creating a `PythonInteger` object works correctly with the
// int constructor.
PythonInteger constructed_int(7);
- auto value = As<long long>(constructed_int);
- EXPECT_THAT_EXPECTED(value, llvm::HasValue(7));
+ EXPECT_EQ(7, constructed_int.GetInteger());
}
TEST_F(PythonDataObjectsTest, TestPythonBoolean) {
@@ -338,8 +339,7 @@ TEST_F(PythonDataObjectsTest, TestPythonListValueEquality) {
PythonInteger chk_int(PyRefType::Borrowed, chk_value1.get());
PythonString chk_str(PyRefType::Borrowed, chk_value2.get());
- auto chkint = As<long long>(chk_value1);
- ASSERT_THAT_EXPECTED(chkint, llvm::HasValue(long_value0));
+ EXPECT_EQ(long_value0, chk_int.GetInteger());
EXPECT_EQ(string_value1, chk_str.GetString());
}
@@ -367,8 +367,7 @@ TEST_F(PythonDataObjectsTest, TestPythonListManipulation) {
PythonInteger chk_int(PyRefType::Borrowed, chk_value1.get());
PythonString chk_str(PyRefType::Borrowed, chk_value2.get());
- auto e = As<long long>(chk_int);
- EXPECT_THAT_EXPECTED(e, llvm::HasValue(long_value0));
+ EXPECT_EQ(long_value0, chk_int.GetInteger());
EXPECT_EQ(string_value1, chk_str.GetString());
}
@@ -488,10 +487,10 @@ TEST_F(PythonDataObjectsTest, TestPythonDictionaryValueEquality) {
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_THAT_EXPECTED(chkint, llvm::HasValue(value_0));
+ EXPECT_EQ(value_0, chk_int.GetInteger());
EXPECT_EQ(value_1, chk_str.GetString());
}
@@ -525,10 +524,10 @@ TEST_F(PythonDataObjectsTest, TestPythonDictionaryManipulation) {
EXPECT_TRUE(PythonInteger::Check(chk_value1.get()));
EXPECT_TRUE(PythonString::Check(chk_value2.get()));
- auto chkint = As<long long>(chk_value1);
+ PythonInteger chk_int(PyRefType::Borrowed, chk_value1.get());
PythonString chk_str(PyRefType::Borrowed, chk_value2.get());
- EXPECT_THAT_EXPECTED(chkint, llvm::HasValue(value_0));
+ EXPECT_EQ(value_0, chk_int.GetInteger());
EXPECT_EQ(value_1, chk_str.GetString());
}
@@ -595,9 +594,10 @@ TEST_F(PythonDataObjectsTest, TestObjectAttributes) {
EXPECT_TRUE(py_int.HasAttribute("numerator"));
EXPECT_FALSE(py_int.HasAttribute("this_should_not_exist"));
- auto numerator_attr = As<long long>(py_int.GetAttributeValue("numerator"));
-
- EXPECT_THAT_EXPECTED(numerator_attr, llvm::HasValue(42));
+ PythonInteger numerator_attr =
+ py_int.GetAttributeValue("numerator").AsType<PythonInteger>();
+ EXPECT_TRUE(numerator_attr.IsAllocated());
+ EXPECT_EQ(42, numerator_attr.GetInteger());
}
TEST_F(PythonDataObjectsTest, TestExtractingUInt64ThroughStructuredData) {
More information about the lldb-commits
mailing list