get rid of PythonInteger::GetInteger()

[lawrence_danna]

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

[labath]

lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp

55

obj->AsUnsignedLongLong() ?

lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp

3152–3162

This converts the Expected into a python exception, only to clear (and print) it at the next line. Is there a more direct way of doing it?

lldb/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp

125–133

EXPECT_THAT_EXPECTED(As<long long>(major_version_field), llvm::HasValue(PY_MAJOR_VERSION)) (and similarly in other tests too)

[lawrence_danna]

review fixes

[lawrence_danna]

lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp

3152–3162

I don't know, what did you have in mind? It could just call the Python C API directly and not bother converting it into an Expected, but would that be better?

[lawrence_danna]

fix

[labath]

Looks good. I'll leave it up to you to consider whether the PySys_WriteStderr thingy is a good idea.

lldb/bindings/python/python-typemaps.swig

77

it looks like this line needs wrapping

lldb/bindings/python/python-wrapper.swig

591–604

If you don't print this, then you could definitely do something like:

Expected<long long> result = As<long long>(pfunc.Call(PythonString(child_name));
if (result)
  return std::max<long long>(*result, 0);
consumeError(result.takeError());
return UINT32_MAX;

lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp

3152–3162

I was thinking if theres some function to print to stderr directly without doing the PyErr dance. PySys_WriteStderr maybe?

if (Expected<long long> py_return = As<long long>(implementor.CallMethod(callee_name)))
  result = *py_return;
else
  PySys_WriteStderr("%s", toString(py_return.takeError()).c_str());

?

[lawrence_danna]

wrap long line

lldb/bindings/python/python-wrapper.swig

591–604

aparently this function is not even used... I'll put up a new diff to remove it

[omjavaid]

This causes multiple test failures on LLDB AArch64 Linux buildbot.
http://lab.llvm.org:8011/builders/lldb-aarch64-ubuntu/builds/3695

[omjavaid]

I have temporarily reverted this change to turn buildbot green.

[lawrence_danna]

fix python2 problems

[lawrence_danna]

@omjavaid sorry I didn't catch that on pre-submit testing. PyLong_AsLongLong and friends do not automatically convert on python2 like they do on python3. It's fixed now.

[lawrence_danna]

rebased

[labath]

Still looks good.

[lawrence_danna]

rebased

[lawrence_danna]

@omjavaid ok to re-land?

[omjavaid]

Thanks for getting this fixed. Looks good now.

lldb/bindings/python/python-typemaps.swig

%typemap(out) char** {
   len = 0;
   while ($1[len]) len++;
   PythonList list(len);
   for (i = 0; i < len; i++)
     list.SetItemAtIndex(i, PythonString($1[i]));
   $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 (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;
+  }
+}
 
 /* Typemap definitions to allow SWIG to properly handle char buffer. */
 
 // typemap for a char buffer
 %typemap(in) (char *dst, size_t dst_len) {
    if (!PyInt_Check($input)) {
        PyErr_SetString(PyExc_ValueError, "Expecting an integer");
        return NULL;

lldb/bindings/python/python-wrapper.swig

if (sym_ctx != nullptr) {
       PythonObject sym_ctx_arg(PyRefType::Owned, SBTypeToSWIGWrapper(sb_sym_ctx));
       result = pfunc(sym_ctx_arg);
     } else
       result = pfunc();
 
     if (PyErr_Occurred())
     {
         PyErr_Print();
-        PyErr_Clear();
         return 0;
     }
 
     // The callback will return a bool, but we're need to also return ints
     // so we're squirrelling the bool through as an int...  And if you return
     // nothing, we'll continue.
     if (strcmp(method_name, "__callback__") == 0) {
         if (result.get() == Py_False)
           return 0;
         else
           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;
 }
 
 // wrapper that calls an optional instance member of an object taking no arguments
 static PyObject*
 LLDBSwigPython_CallOptionalMember
 (
if (!pfunc.IsAllocated())
         return 0;
 
     auto arg_info = pfunc.GetArgInfo();
     if (!arg_info) {
         llvm::consumeError(arg_info.takeError());
         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)
         ret_val = std::min(ret_val, static_cast<size_t>(max));
 
     return ret_val;
 }
 
)
     PyErr_Cleaner py_err_cleaner(true);
 
     PythonObject self(PyRefType::Borrowed, implementor);
     auto pfunc = self.ResolveName<PythonCallable>("get_child_index");
 
     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;
 
     return UINT32_MAX;
 }
 
 SWIGEXPORT bool
 LLDBSwigPython_UpdateSynthProviderInstance
 (
     PyObject *implementor
 )
 {

lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h

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> 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();
     int r = PyObject_IsInstance(m_py_obj, cls.get());
     if (r < 0)
       return exception();
     return !!r;
   }

 }
 
 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);
 
 
 template <class T> class TypedPythonObject : public PythonObject {
 public:
   // override to perform implicit type conversions on Reset
   // This can be eliminated once we drop python 2 support.
   static void Convert(PyRefType &type, PyObject *&py_obj) {}
public:
 
   PythonInteger() : TypedPythonObject() {} // MSVC requires this for some reason
 
   explicit PythonInteger(int64_t value);
 
   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;
 };
 
 class PythonBoolean : public TypedPythonObject<PythonBoolean> {
 public:
   using TypedPythonObject::TypedPythonObject;

lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp

if (!obj)
     return obj.takeError();
   return obj.get().IsTrue();
 }
 
 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 <>
 Expected<std::string> python::As<std::string>(Expected<PythonObject> &&obj) {
   if (!obj)
     return obj.takeError();
   PyObject *str_obj = PyObject_Str(obj.get().get());
   if (!obj)
     return llvm::make_error<PythonException>();
   auto str = Take<PythonString>(str_obj);
   auto utf8 = str.AsUTF8();
   if (!utf8)
     return utf8.takeError();
   return std::string(utf8.get());
 }
 
if (type == PyRefType::Owned)
       Py_DECREF(py_obj);
     else
       type = PyRefType::Owned;
     py_obj = l;
   }
 #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;
 }
 
 // PythonBoolean
 
 PythonBoolean::PythonBoolean(bool value) {
   SetValue(value);
 }

lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp

uint32_t ScriptInterpreterPythonImpl::GetFlagsForCommandObject(
   if (PyCallable_Check(pmeth.get()) == 0) {
     if (PyErr_Occurred())
       PyErr_Clear();
     return result;
   }
 
   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;
 }
 
 bool ScriptInterpreterPythonImpl::GetLongHelpForCommandObject(
     StructuredData::GenericSP cmd_obj_sp, std::string &dest) {
   bool got_string = false;

lldb/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp

 
 TEST_F(PythonDataObjectsTest, TestInstanceNameResolutionNoDot) {
   PythonObject sys_version_info = m_sys_module.ResolveName("version_info");
   PythonObject major_version_field = sys_version_info.ResolveName("major");
   PythonObject minor_version_field = sys_version_info.ResolveName("minor");
 
   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) {
   PythonObject sys_path = m_main_module.ResolveName("sys.path");
   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) {
   // Make up a custom dictionary with "sys" pointing to the `sys` module.
   PythonDictionary dict(PyInitialValue::Empty);
   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) {
   // Test that integers behave correctly when wrapped by a PythonInteger.
 
 #if PY_MAJOR_VERSION < 3
   // Verify that `PythonInt` works correctly when given a PyInt object.
   // Note that PyInt doesn't exist in Python 3.x, so this is only for 2.x
   PyObject *py_int = PyInt_FromLong(12);
   EXPECT_TRUE(PythonInteger::Check(py_int));
   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.
   PyObject *py_long = PyLong_FromLong(12);
   EXPECT_TRUE(PythonInteger::Check(py_long));
   PythonInteger python_long(PyRefType::Owned, py_long);
   EXPECT_EQ(PyObjectType::Integer, python_long.GetObjectType());
 
   // 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) {
   // Test PythonBoolean constructed from Py_True
   EXPECT_TRUE(PythonBoolean::Check(Py_True));
   PythonBoolean python_true(PyRefType::Owned, Py_True);
   EXPECT_EQ(PyObjectType::Boolean, python_true.GetObjectType());
 
TEST_F(PythonDataObjectsTest, TestPythonListValueEquality) {
   PythonObject chk_value1 = list.GetItemAtIndex(0);
   PythonObject chk_value2 = list.GetItemAtIndex(1);
   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);
-  ASSERT_THAT_EXPECTED(chkint, llvm::HasValue(long_value0));
+  EXPECT_EQ(long_value0, chk_int.GetInteger());
   EXPECT_EQ(string_value1, chk_str.GetString());
 }
 
 TEST_F(PythonDataObjectsTest, TestPythonListManipulation) {
   // Test that manipulation of a PythonList behaves correctly when
   // wrapped by a PythonDictionary.
 
   static const long long_value0 = 5;
TEST_F(PythonDataObjectsTest, TestPythonListManipulation) {
   PythonObject chk_value1 = list.GetItemAtIndex(0);
   PythonObject chk_value2 = list.GetItemAtIndex(1);
   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 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());
 }
 
 TEST_F(PythonDataObjectsTest, TestPythonListToStructuredList) {
   static const long long_value0 = 5;
   static const char *const string_value1 = "String Index 1";
 
   PythonList list(PyInitialValue::Empty);
TEST_F(PythonDataObjectsTest, TestPythonDictionaryValueEquality) {
   EXPECT_EQ(PyObjectType::Dictionary, dict.GetObjectType());
 
   // Verify that the values match
   PythonObject chk_value1 = dict.GetItemForKey(py_keys[0]);
   PythonObject chk_value2 = dict.GetItemForKey(py_keys[1]);
   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());
 }
 
 TEST_F(PythonDataObjectsTest, TestPythonDictionaryManipulation) {
   // Test that manipulation of a dictionary behaves correctly when wrapped
   // by a PythonDictionary.
   static const unsigned dict_entries = 2;
 
TEST_F(PythonDataObjectsTest, TestPythonDictionaryManipulation) {
   EXPECT_EQ(dict_entries, dict.GetSize());
 
   // Verify that the keys and values match
   PythonObject chk_value1 = dict.GetItemForKey(keys[0]);
   PythonObject chk_value2 = dict.GetItemForKey(keys[1]);
   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());
 }
 
 TEST_F(PythonDataObjectsTest, TestPythonDictionaryToStructuredDictionary) {
   static const char *const string_key0 = "String Key 0";
   static const char *const string_key1 = "String Key 1";
 
   static const char *const string_value0 = "String Value 0";
TEST_F(PythonDataObjectsTest, TestPythonFile) {
   EXPECT_TRUE(PythonFile::Check(py_file.get().get()));
 }
 
 TEST_F(PythonDataObjectsTest, TestObjectAttributes) {
   PythonInteger py_int(42);
   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) {
   // Make up a custom dictionary with "sys" pointing to the `sys` module.
   const char *key_name = "addr";
   const uint64_t value = 0xf000000000000000ull;
   PythonDictionary python_dict(PyInitialValue::Empty);
   PythonInteger python_ull_value(PyRefType::Owned,

 g = foobar()
 )";
 
   r = runStringMultiLine(script, globals, globals);
   ASSERT_THAT_EXPECTED(r, llvm::Succeeded());
   auto g = As<std::string>(globals.GetItem("g"));
   ASSERT_THAT_EXPECTED(g, llvm::HasValue("foobarbaz"));
 }
 No newline at end of file