Re-land "get rid of PythonInteger::GetInteger()"

This was reverted due to a python2-specific bug.  Re-landing with a fix
for python2.

Summary:
One small step in my long running quest to improve python exception handling in
LLDB.  Replace GetInteger() which just returns an int with As<long long> and
friends, which return Expected types that can track python exceptions

Reviewers: labath, jasonmolenda, JDevlieghere, vadimcn, omjavaid

Reviewed By: labath, omjavaid

Subscribers: omjavaid, lldb-commits

Tags: #lldb

Differential Revision: https://reviews.llvm.org/D78462

3 files changed, 82 insertions, 41 deletions

diff --git a/lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp b/lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
index 40ed22a..6f040fd 100644
--- a/lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
+++ b/lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
@@ -44,7 +44,15 @@ template <>
 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 @@ Expected<std::string> python::As<std::string>(Expected<PythonObject> &&obj) {
   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 @@ 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);
-  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;
 }
 
diff --git a/lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h b/lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
index 1689680..ba127ea 100644
--- a/lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
+++ b/lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
@@ -360,15 +360,12 @@ public:
     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())
@@ -400,6 +397,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 @@ public:
   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 c53b3bd..6f26677 100644
--- a/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
+++ b/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
@@ -3150,20 +3150,15 @@ uint32_t ScriptInterpreterPythonImpl::GetFlagsForCommandObject(
   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;