eliminate virtual methods from PythonDataObjects

[lawrence_danna]

Summary

This patch eliminates a bunch of boilerplate from
PythonDataObjects, as well as the use of virtual methods.
In my opinion it also makes the Reset logic a lot more
clear and easy to follow. The price is yet another
template. I think it's worth it.

[labath]

Yes, this definitely looks better. Ideally, I'd like to get rid of the Reset functions altogether, and just ensure we already create/return fully valid objects (probably via factory functions returning Expecteds, Optionals or whatever)..

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

207–211

BTW, is this needed? My impression was that PythonObject was/is not convertible to a PyObject*..

[lawrence_danna]

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

207–211

Yea it's not actually convertible, i'll remove this.

[lawrence_danna]

rebased, and added in explicit default constructors for MSVC's sake.

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

 // Nearly all the code in this header does not handle python exceptions
 // correctly.  It should all be converted to return Expected<> or
 // Error types to capture the exception.
 //
 // Everything in this file except functions that return Error or
 // Expected<> is considered deprecated and should not be
 // used in new code.  If you need to use it, fix it first.
 //
+//
+// TODOs for this file
+//
+// * Make all methods safe for exceptions.
+//
+// * Eliminate method signatures that must translate exceptions into
+//   empty objects or NULLs.   Almost everything here should return
+//   Expected<>.   It should be acceptable for certain operations that
+//   can never fail to assert instead, such as the creation of
+//   PythonString from a string literal.
+//
+// * Elimintate Reset(), and make all non-default constructors private.
+//   Python objects should be created with Retain<> or Take<>, and they
+//   should be assigned with operator=
+//
+// * Eliminate default constructors, make python objects always
+//   nonnull, and use optionals where necessary.
+//
+
 
 #ifndef LLDB_PLUGINS_SCRIPTINTERPRETER_PYTHON_PYTHONDATAOBJECTS_H
 #define LLDB_PLUGINS_SCRIPTINTERPRETER_PYTHON_PYTHONDATAOBJECTS_H
 
 #ifndef LLDB_DISABLE_PYTHON
 
 // LLDB Python header must be included first
 #include "lldb-python.h"
public:
 
   PythonObject(const PythonObject &rhs) : m_py_obj(nullptr) { Reset(rhs); }
 
   PythonObject(PythonObject &&rhs) {
     m_py_obj = rhs.m_py_obj;
     rhs.m_py_obj = nullptr;
   }
 
-  virtual ~PythonObject() { Reset(); }
+  ~PythonObject() { Reset(); }
 
   void Reset() {
-    // Avoid calling the virtual method since it's not necessary
-    // to actually validate the type of the PyObject if we're
-    // just setting to null.
     if (m_py_obj && Py_IsInitialized())
       Py_DECREF(m_py_obj);
     m_py_obj = nullptr;
   }
 
   void Reset(const PythonObject &rhs) {
-    // Avoid calling the virtual method if it's not necessary
-    // to actually validate the type of the PyObject.
     if (!rhs.IsValid())
       Reset();
     else
       Reset(PyRefType::Borrowed, rhs.m_py_obj);
   }
 
   // PythonObject is implicitly convertible to PyObject *, which will call the
   // wrong overload.  We want to explicitly disallow this, since a PyObject
   // *always* owns its reference.  Therefore the overload which takes a
   // PyRefType doesn't make sense, and the copy constructor should be used.
   void Reset(PyRefType type, const PythonObject &ref) = delete;
 
-  // FIXME We shouldn't have virtual anything.  PythonObject should be a
-  // strictly pass-by-value type.
-  virtual void Reset(PyRefType type, PyObject *py_obj) {
+  void Reset(PyRefType type, PyObject *py_obj) {
     if (py_obj == m_py_obj)
       return;
 
     if (Py_IsInitialized())
       Py_XDECREF(m_py_obj);
 
     m_py_obj = py_obj;
 

 
 template <> llvm::Expected<bool> As<bool>(llvm::Expected<PythonObject> &&obj);
 
 template <>
 llvm::Expected<long long> As<long long>(llvm::Expected<PythonObject> &&obj);
 
 } // namespace python
 
-class PythonBytes : public PythonObject {
+template <class T> class TypedPythonObject : public PythonObject {
 public:
-  PythonBytes();
-  explicit PythonBytes(llvm::ArrayRef<uint8_t> bytes);
-  PythonBytes(const uint8_t *bytes, size_t length);
-  PythonBytes(PyRefType type, PyObject *o);
+  // 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) {}
+
+  using PythonObject::Reset;
 
-  ~PythonBytes() override;
+  void Reset(PyRefType type, PyObject *py_obj) {
+    Reset();
+    if (!py_obj)
+      return;
+    T::Convert(type, py_obj);
+    if (T::Check(py_obj))
+      PythonObject::Reset(type, py_obj);
+    else if (type == PyRefType::Owned)
+      Py_DECREF(py_obj);
+  }
 
-  static bool Check(PyObject *py_obj);
+  TypedPythonObject(PyRefType type, PyObject *py_obj) { Reset(type, py_obj); }
 
-  // Bring in the no-argument base class version
-  using PythonObject::Reset;
+  TypedPythonObject() {}
+};
+
+class PythonBytes : public TypedPythonObject<PythonBytes> {
+public:
+  using TypedPythonObject::TypedPythonObject;
+  explicit PythonBytes(llvm::ArrayRef<uint8_t> bytes);
+  PythonBytes(const uint8_t *bytes, size_t length);
 
-  void Reset(PyRefType type, PyObject *py_obj) override;
+  static bool Check(PyObject *py_obj);
 
   llvm::ArrayRef<uint8_t> GetBytes() const;
 
   size_t GetSize() const;
 
   void SetBytes(llvm::ArrayRef<uint8_t> stringbytes);
 
   StructuredData::StringSP CreateStructuredString() const;
 };
 
-class PythonByteArray : public PythonObject {
+class PythonByteArray : public TypedPythonObject<PythonByteArray> {
 public:
-  PythonByteArray();
+  using TypedPythonObject::TypedPythonObject;
   explicit PythonByteArray(llvm::ArrayRef<uint8_t> bytes);
   PythonByteArray(const uint8_t *bytes, size_t length);
-  PythonByteArray(PyRefType type, PyObject *o);
   PythonByteArray(const PythonBytes &object);
 
-  ~PythonByteArray() override;
-
   static bool Check(PyObject *py_obj);
 
-  // Bring in the no-argument base class version
-  using PythonObject::Reset;
-
-  void Reset(PyRefType type, PyObject *py_obj) override;
-
   llvm::ArrayRef<uint8_t> GetBytes() const;
 
   size_t GetSize() const;
 
   void SetBytes(llvm::ArrayRef<uint8_t> stringbytes);
 
   StructuredData::StringSP CreateStructuredString() const;
 };
 
-class PythonString : public PythonObject {
+class PythonString : public TypedPythonObject<PythonString> {
 public:
+  using TypedPythonObject::TypedPythonObject;
   static llvm::Expected<PythonString> FromUTF8(llvm::StringRef string);
 
-  PythonString();
-  explicit PythonString(llvm::StringRef string); // safe, null on error
-  PythonString(PyRefType type, PyObject *o);
+  PythonString() : TypedPythonObject() {} // MSVC requires this for some reason
 
-  ~PythonString() override;
+  explicit PythonString(llvm::StringRef string); // safe, null on error
 
   static bool Check(PyObject *py_obj);
-
-  // Bring in the no-argument base class version
-  using PythonObject::Reset;
-
-  void Reset(PyRefType type, PyObject *py_obj) override;
+  static void Convert(PyRefType &type, PyObject *&py_obj);
 
   llvm::StringRef GetString() const; // safe, empty string on error
 
   llvm::Expected<llvm::StringRef> AsUTF8() const;
 
   size_t GetSize() const;
 
   void SetString(llvm::StringRef string); // safe, null on error
 
   StructuredData::StringSP CreateStructuredString() const;
 };
 
-class PythonInteger : public PythonObject {
+class PythonInteger : public TypedPythonObject<PythonInteger> {
 public:
-  PythonInteger();
-  explicit PythonInteger(int64_t value);
-  PythonInteger(PyRefType type, PyObject *o);
+  using TypedPythonObject::TypedPythonObject;
 
-  ~PythonInteger() override;
+  PythonInteger() : TypedPythonObject() {} // MSVC requires this for some reason
 
-  static bool Check(PyObject *py_obj);
-
-  // Bring in the no-argument base class version
-  using PythonObject::Reset;
+  explicit PythonInteger(int64_t value);
 
-  void Reset(PyRefType type, PyObject *py_obj) override;
+  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 PythonObject {
+class PythonBoolean : public TypedPythonObject<PythonBoolean> {
 public:
-  PythonBoolean() = default;
-  explicit PythonBoolean(bool value);
-  PythonBoolean(PyRefType type, PyObject *o);
+  using TypedPythonObject::TypedPythonObject;
 
-  ~PythonBoolean() override = default;
+  explicit PythonBoolean(bool value);
 
   static bool Check(PyObject *py_obj);
 
-  // Bring in the no-argument base class version
-  using PythonObject::Reset;
-
-  void Reset(PyRefType type, PyObject *py_obj) override;
-
   bool GetValue() const;
 
   void SetValue(bool value);
 
   StructuredData::BooleanSP CreateStructuredBoolean() const;
 };
 
-class PythonList : public PythonObject {
+class PythonList : public TypedPythonObject<PythonList> {
 public:
-  PythonList() {}
+  using TypedPythonObject::TypedPythonObject;
+
+  PythonList() : TypedPythonObject() {} // MSVC requires this for some reason
+
   explicit PythonList(PyInitialValue value);
   explicit PythonList(int list_size);
-  PythonList(PyRefType type, PyObject *o);
-
-  ~PythonList() override;
 
   static bool Check(PyObject *py_obj);
 
-  // Bring in the no-argument base class version
-  using PythonObject::Reset;
-
-  void Reset(PyRefType type, PyObject *py_obj) override;
-
   uint32_t GetSize() const;
 
   PythonObject GetItemAtIndex(uint32_t index) const;
 
   void SetItemAtIndex(uint32_t index, const PythonObject &object);
 
   void AppendItem(const PythonObject &object);
 
   StructuredData::ArraySP CreateStructuredArray() const;
 };
 
-class PythonTuple : public PythonObject {
+class PythonTuple : public TypedPythonObject<PythonTuple> {
 public:
-  PythonTuple() {}
+  using TypedPythonObject::TypedPythonObject;
+
   explicit PythonTuple(PyInitialValue value);
   explicit PythonTuple(int tuple_size);
-  PythonTuple(PyRefType type, PyObject *o);
   PythonTuple(std::initializer_list<PythonObject> objects);
   PythonTuple(std::initializer_list<PyObject *> objects);
 
-  ~PythonTuple() override;
-
   static bool Check(PyObject *py_obj);
 
-  // Bring in the no-argument base class version
-  using PythonObject::Reset;
-
-  void Reset(PyRefType type, PyObject *py_obj) override;
-
   uint32_t GetSize() const;
 
   PythonObject GetItemAtIndex(uint32_t index) const;
 
   void SetItemAtIndex(uint32_t index, const PythonObject &object);
 
   StructuredData::ArraySP CreateStructuredArray() const;
 };
 
-class PythonDictionary : public PythonObject {
+class PythonDictionary : public TypedPythonObject<PythonDictionary> {
 public:
-  PythonDictionary() {}
-  explicit PythonDictionary(PyInitialValue value);
-  PythonDictionary(PyRefType type, PyObject *o);
+  using TypedPythonObject::TypedPythonObject;
 
-  ~PythonDictionary() override;
+  PythonDictionary() : TypedPythonObject() {} // MSVC requires this for some reason
 
-  static bool Check(PyObject *py_obj);
-
-  // Bring in the no-argument base class version
-  using PythonObject::Reset;
+  explicit PythonDictionary(PyInitialValue value);
 
-  void Reset(PyRefType type, PyObject *py_obj) override;
+  static bool Check(PyObject *py_obj);
 
   uint32_t GetSize() const;
 
   PythonList GetKeys() const;
 
   PythonObject GetItemForKey(const PythonObject &key) const;
   void SetItemForKey(const PythonObject &key, const PythonObject &value);
 
   StructuredData::DictionarySP CreateStructuredDictionary() const;
 };
 
-class PythonModule : public PythonObject {
+class PythonModule : public TypedPythonObject<PythonModule> {
 public:
-  PythonModule();
-  PythonModule(PyRefType type, PyObject *o);
-
-  ~PythonModule() override;
+  using TypedPythonObject::TypedPythonObject;
 
   static bool Check(PyObject *py_obj);
 
   static PythonModule BuiltinsModule();
 
   static PythonModule MainModule();
 
   static PythonModule AddModule(llvm::StringRef module);
 
   // safe, returns invalid on error;
   static PythonModule ImportModule(llvm::StringRef name) {
     std::string s = name;
     auto mod = Import(s.c_str());
     if (!mod) {
       llvm::consumeError(mod.takeError());
       return PythonModule();
     }
     return std::move(mod.get());
   }
 
   static llvm::Expected<PythonModule> Import(const char *name);
 
   llvm::Expected<PythonObject> Get(const char *name);
 
-  // Bring in the no-argument base class version
-  using PythonObject::Reset;
-
-  void Reset(PyRefType type, PyObject *py_obj) override;
-
   PythonDictionary GetDictionary() const;
 };
 
-class PythonCallable : public PythonObject {
+class PythonCallable : public TypedPythonObject<PythonCallable> {
 public:
+  using TypedPythonObject::TypedPythonObject;
+
   struct ArgInfo {
     size_t count;
     bool is_bound_method : 1;
     bool has_varargs : 1;
     bool has_kwargs : 1;
   };
 
-  PythonCallable();
-  PythonCallable(PyRefType type, PyObject *o);
-
-  ~PythonCallable() override;
-
   static bool Check(PyObject *py_obj);
 
-  // Bring in the no-argument base class version
-  using PythonObject::Reset;
-
-  void Reset(PyRefType type, PyObject *py_obj) override;
-
   ArgInfo GetNumArguments() const;
 
   // If the callable is a Py_Class, then find the number of arguments
   // of the __init__ method.
   ArgInfo GetNumInitArguments() const;
 
   PythonObject operator()();
 
   PythonObject operator()(std::initializer_list<PyObject *> args);
 
   PythonObject operator()(std::initializer_list<PythonObject> args);
 
   template <typename Arg, typename... Args>
   PythonObject operator()(const Arg &arg, Args... args) {
     return operator()({arg, args...});
   }
 };
 
-class PythonFile : public PythonObject {
+class PythonFile : public TypedPythonObject<PythonFile> {
 public:
-  PythonFile();
-  PythonFile(PyRefType type, PyObject *o);
+  using TypedPythonObject::TypedPythonObject;
 
-  ~PythonFile() override;
+  PythonFile() : TypedPythonObject() {} // MSVC requires this for some reason
 
   static bool Check(PyObject *py_obj);
 
-  using PythonObject::Reset;
-
-  void Reset(PyRefType type, PyObject *py_obj) override;
-
   static llvm::Expected<PythonFile> FromFile(File &file,
                                              const char *mode = nullptr);
 
   // FIXME delete this after FILE* typemaps are deleted
   // and ScriptInterpreterPython is fixed
   PythonFile(File &file, const char *mode = nullptr) {
     auto f = FromFile(file, mode);
     if (f)

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

StructuredData::ObjectSP PythonObject::CreateStructuredObject() const {
   case PyObjectType::None:
     return StructuredData::ObjectSP();
   default:
     return StructuredData::ObjectSP(new StructuredPythonObject(m_py_obj));
   }
 }
 
 // PythonString
-PythonBytes::PythonBytes() : PythonObject() {}
 
-PythonBytes::PythonBytes(llvm::ArrayRef<uint8_t> bytes) : PythonObject() {
-  SetBytes(bytes);
-}
+PythonBytes::PythonBytes(llvm::ArrayRef<uint8_t> bytes) { SetBytes(bytes); }
 
-PythonBytes::PythonBytes(const uint8_t *bytes, size_t length) : PythonObject() {
+PythonBytes::PythonBytes(const uint8_t *bytes, size_t length) {
   SetBytes(llvm::ArrayRef<uint8_t>(bytes, length));
 }
 
-PythonBytes::PythonBytes(PyRefType type, PyObject *py_obj) : PythonObject() {
-  Reset(type, py_obj); // Use "Reset()" to ensure that py_obj is a string
-}
-
-PythonBytes::~PythonBytes() {}
-
 bool PythonBytes::Check(PyObject *py_obj) {
   if (!py_obj)
     return false;
   return PyBytes_Check(py_obj);
 }
 
-void PythonBytes::Reset(PyRefType type, PyObject *py_obj) {
-  // Grab the desired reference type so that if we end up rejecting `py_obj` it
-  // still gets decremented if necessary.
-  PythonObject result(type, py_obj);
-
-  if (!PythonBytes::Check(py_obj)) {
-    PythonObject::Reset();
-    return;
-  }
-
-  // Calling PythonObject::Reset(const PythonObject&) will lead to stack
-  // overflow since it calls back into the virtual implementation.
-  PythonObject::Reset(PyRefType::Borrowed, result.get());
-}
-
 llvm::ArrayRef<uint8_t> PythonBytes::GetBytes() const {
   if (!IsValid())
     return llvm::ArrayRef<uint8_t>();
 
   Py_ssize_t size;
   char *c;
 
   PyBytes_AsStringAndSize(m_py_obj, &c, &size);

 PythonByteArray::PythonByteArray(llvm::ArrayRef<uint8_t> bytes)
     : PythonByteArray(bytes.data(), bytes.size()) {}
 
 PythonByteArray::PythonByteArray(const uint8_t *bytes, size_t length) {
   const char *str = reinterpret_cast<const char *>(bytes);
   Reset(PyRefType::Owned, PyByteArray_FromStringAndSize(str, length));
 }
 
-PythonByteArray::PythonByteArray(PyRefType type, PyObject *o) {
-  Reset(type, o);
-}
-
-PythonByteArray::PythonByteArray(const PythonBytes &object)
-    : PythonObject(object) {}
-
-PythonByteArray::~PythonByteArray() {}
-
 bool PythonByteArray::Check(PyObject *py_obj) {
   if (!py_obj)
     return false;
   return PyByteArray_Check(py_obj);
 }
 
-void PythonByteArray::Reset(PyRefType type, PyObject *py_obj) {
-  // Grab the desired reference type so that if we end up rejecting `py_obj` it
-  // still gets decremented if necessary.
-  PythonObject result(type, py_obj);
-
-  if (!PythonByteArray::Check(py_obj)) {
-    PythonObject::Reset();
-    return;
-  }
-
-  // Calling PythonObject::Reset(const PythonObject&) will lead to stack
-  // overflow since it calls back into the virtual implementation.
-  PythonObject::Reset(PyRefType::Borrowed, result.get());
-}
-
 llvm::ArrayRef<uint8_t> PythonByteArray::GetBytes() const {
   if (!IsValid())
     return llvm::ArrayRef<uint8_t>();
 
   char *c = PyByteArray_AsString(m_py_obj);
   size_t size = GetSize();
   return llvm::ArrayRef<uint8_t>(reinterpret_cast<uint8_t *>(c), size);
 }

 #else
   PyObject *str = PyString_FromStringAndSize(string.data(), string.size());
 #endif
   if (!str)
     return llvm::make_error<PythonException>();
   return Take<PythonString>(str);
 }
 
-PythonString::PythonString(PyRefType type, PyObject *py_obj) : PythonObject() {
-  Reset(type, py_obj); // Use "Reset()" to ensure that py_obj is a string
-}
-
-PythonString::PythonString(llvm::StringRef string) : PythonObject() {
-  SetString(string);
-}
-
-PythonString::PythonString() : PythonObject() {}
-
-PythonString::~PythonString() {}
+PythonString::PythonString(llvm::StringRef string) { SetString(string); }
 
 bool PythonString::Check(PyObject *py_obj) {
   if (!py_obj)
     return false;
 
   if (PyUnicode_Check(py_obj))
     return true;
 #if PY_MAJOR_VERSION < 3
   if (PyString_Check(py_obj))
     return true;
 #endif
   return false;
 }
 
-void PythonString::Reset(PyRefType type, PyObject *py_obj) {
-  // Grab the desired reference type so that if we end up rejecting `py_obj` it
-  // still gets decremented if necessary.
-  PythonObject result(type, py_obj);
-
-  if (!PythonString::Check(py_obj)) {
-    PythonObject::Reset();
-    return;
-  }
+void PythonString::Convert(PyRefType &type, PyObject *&py_obj) {
 #if PY_MAJOR_VERSION < 3
   // In Python 2, Don't store PyUnicode objects directly, because we need
   // access to their underlying character buffers which Python 2 doesn't
   // provide.
   if (PyUnicode_Check(py_obj)) {
-    PyObject *s = PyUnicode_AsUTF8String(result.get());
-    if (s == NULL)
+    PyObject *s = PyUnicode_AsUTF8String(py_obj);
+    if (s == nullptr) {
       PyErr_Clear();
-    result.Reset(PyRefType::Owned, s);
+      if (type == PyRefType::Owned)
+        Py_DECREF(py_obj);
+      return;
+    }
+    if (type == PyRefType::Owned)
+      Py_DECREF(py_obj);
+    else
+      type = PyRefType::Owned;
+    py_obj = s;
   }
 #endif
-  // Calling PythonObject::Reset(const PythonObject&) will lead to stack
-  // overflow since it calls back into the virtual implementation.
-  PythonObject::Reset(PyRefType::Borrowed, result.get());
 }
 
 llvm::StringRef PythonString::GetString() const {
   auto s = AsUTF8();
   if (!s) {
     llvm::consumeError(s.takeError());
     return llvm::StringRef("");
   }

 StructuredData::StringSP PythonString::CreateStructuredString() const {
   StructuredData::StringSP result(new StructuredData::String);
   result->SetValue(GetString());
   return result;
 }
 
 // PythonInteger
 
-PythonInteger::PythonInteger() : PythonObject() {}
-
-PythonInteger::PythonInteger(PyRefType type, PyObject *py_obj)
-    : PythonObject() {
-  Reset(type, py_obj); // Use "Reset()" to ensure that py_obj is a integer type
-}
-
-PythonInteger::PythonInteger(int64_t value) : PythonObject() {
-  SetInteger(value);
-}
-
-PythonInteger::~PythonInteger() {}
+PythonInteger::PythonInteger(int64_t value) { SetInteger(value); }
 
 bool PythonInteger::Check(PyObject *py_obj) {
   if (!py_obj)
     return false;
 
 #if PY_MAJOR_VERSION >= 3
   // Python 3 does not have PyInt_Check.  There is only one type of integral
   // value, long.
   return PyLong_Check(py_obj);
 #else
   return PyLong_Check(py_obj) || PyInt_Check(py_obj);
 #endif
 }
 
-void PythonInteger::Reset(PyRefType type, PyObject *py_obj) {
-  // Grab the desired reference type so that if we end up rejecting `py_obj` it
-  // still gets decremented if necessary.
-  PythonObject result(type, py_obj);
-
-  if (!PythonInteger::Check(py_obj)) {
-    PythonObject::Reset();
-    return;
-  }
-
+void PythonInteger::Convert(PyRefType &type, PyObject *&py_obj) {
 #if PY_MAJOR_VERSION < 3
   // Always store this as a PyLong, which makes interoperability between Python
   // 2.x and Python 3.x easier.  This is only necessary in 2.x, since 3.x
   // doesn't even have a PyInt.
   if (PyInt_Check(py_obj)) {
     // Since we converted the original object to a different type, the new
     // object is an owned object regardless of the ownership semantics
     // requested by the user.
-    result.Reset(PyRefType::Owned, PyLong_FromLongLong(PyInt_AsLong(py_obj)));
+    long long value = PyInt_AsLong(py_obj);
+    PyObject *l = nullptr;
+    if (!PyErr_Occurred())
+      l = PyLong_FromLongLong(value);
+    if (l == nullptr) {
+      PyErr_Clear();
+      if (type == PyRefType::Owned)
+        Py_DECREF(py_obj);
+      return;
+    }
+    if (type == PyRefType::Owned)
+      Py_DECREF(py_obj);
+    else
+      type = PyRefType::Owned;
+    py_obj = l;
   }
 #endif
-
-  assert(PyLong_Check(result.get()) &&
-         "Couldn't get a PyLong from this PyObject");
-
-  // Calling PythonObject::Reset(const PythonObject&) will lead to stack
-  // overflow since it calls back into the virtual implementation.
-  PythonObject::Reset(PyRefType::Borrowed, result.get());
 }
 
 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;

 StructuredData::IntegerSP PythonInteger::CreateStructuredInteger() const {
   StructuredData::IntegerSP result(new StructuredData::Integer);
   result->SetValue(GetInteger());
   return result;
 }
 
 // PythonBoolean
 
-PythonBoolean::PythonBoolean(PyRefType type, PyObject *py_obj)
-    : PythonObject() {
-  Reset(type, py_obj); // Use "Reset()" to ensure that py_obj is a boolean type
-}
-
 PythonBoolean::PythonBoolean(bool value) {
   SetValue(value);
 }
 
 bool PythonBoolean::Check(PyObject *py_obj) {
   return py_obj ? PyBool_Check(py_obj) : false;
 }
 
-void PythonBoolean::Reset(PyRefType type, PyObject *py_obj) {
-  // Grab the desired reference type so that if we end up rejecting `py_obj` it
-  // still gets decremented if necessary.
-  PythonObject result(type, py_obj);
-
-  if (!PythonBoolean::Check(py_obj)) {
-    PythonObject::Reset();
-    return;
-  }
-
-  // Calling PythonObject::Reset(const PythonObject&) will lead to stack
-  // overflow since it calls back into the virtual implementation.
-  PythonObject::Reset(PyRefType::Borrowed, result.get());
-}
-
 bool PythonBoolean::GetValue() const {
   return m_py_obj ? PyObject_IsTrue(m_py_obj) : false;
 }
 
 void PythonBoolean::SetValue(bool value) {
   PythonObject::Reset(PyRefType::Owned, PyBool_FromLong(value));
 }
 
 StructuredData::BooleanSP PythonBoolean::CreateStructuredBoolean() const {
   StructuredData::BooleanSP result(new StructuredData::Boolean);
   result->SetValue(GetValue());
   return result;
 }
 
 // PythonList
 
-PythonList::PythonList(PyInitialValue value) : PythonObject() {
+PythonList::PythonList(PyInitialValue value) {
   if (value == PyInitialValue::Empty)
     Reset(PyRefType::Owned, PyList_New(0));
 }
 
-PythonList::PythonList(int list_size) : PythonObject() {
+PythonList::PythonList(int list_size) {
   Reset(PyRefType::Owned, PyList_New(list_size));
 }
 
-PythonList::PythonList(PyRefType type, PyObject *py_obj) : PythonObject() {
-  Reset(type, py_obj); // Use "Reset()" to ensure that py_obj is a list
-}
-
-PythonList::~PythonList() {}
-
 bool PythonList::Check(PyObject *py_obj) {
   if (!py_obj)
     return false;
   return PyList_Check(py_obj);
 }
 
-void PythonList::Reset(PyRefType type, PyObject *py_obj) {
-  // Grab the desired reference type so that if we end up rejecting `py_obj` it
-  // still gets decremented if necessary.
-  PythonObject result(type, py_obj);
-
-  if (!PythonList::Check(py_obj)) {
-    PythonObject::Reset();
-    return;
-  }
-
-  // Calling PythonObject::Reset(const PythonObject&) will lead to stack
-  // overflow since it calls back into the virtual implementation.
-  PythonObject::Reset(PyRefType::Borrowed, result.get());
-}
-
 uint32_t PythonList::GetSize() const {
   if (IsValid())
     return PyList_GET_SIZE(m_py_obj);
   return 0;
 }
 
 PythonObject PythonList::GetItemAtIndex(uint32_t index) const {
   if (IsValid())
for (uint32_t i = 0; i < count; ++i) {
     PythonObject obj = GetItemAtIndex(i);
     result->AddItem(obj.CreateStructuredObject());
   }
   return result;
 }
 
 // PythonTuple
 
-PythonTuple::PythonTuple(PyInitialValue value) : PythonObject() {
+PythonTuple::PythonTuple(PyInitialValue value) {
   if (value == PyInitialValue::Empty)
     Reset(PyRefType::Owned, PyTuple_New(0));
 }
 
-PythonTuple::PythonTuple(int tuple_size) : PythonObject() {
+PythonTuple::PythonTuple(int tuple_size) {
   Reset(PyRefType::Owned, PyTuple_New(tuple_size));
 }
 
-PythonTuple::PythonTuple(PyRefType type, PyObject *py_obj) : PythonObject() {
-  Reset(type, py_obj); // Use "Reset()" to ensure that py_obj is a tuple
-}
-
 PythonTuple::PythonTuple(std::initializer_list<PythonObject> objects) {
   m_py_obj = PyTuple_New(objects.size());
 
   uint32_t idx = 0;
   for (auto object : objects) {
     if (object.IsValid())
       SetItemAtIndex(idx, object);
     idx++;
   }
 }
 
 PythonTuple::PythonTuple(std::initializer_list<PyObject *> objects) {
   m_py_obj = PyTuple_New(objects.size());
 
   uint32_t idx = 0;
   for (auto py_object : objects) {
     PythonObject object(PyRefType::Borrowed, py_object);
     if (object.IsValid())
       SetItemAtIndex(idx, object);
     idx++;
   }
 }
 
-PythonTuple::~PythonTuple() {}
-
 bool PythonTuple::Check(PyObject *py_obj) {
   if (!py_obj)
     return false;
   return PyTuple_Check(py_obj);
 }
 
-void PythonTuple::Reset(PyRefType type, PyObject *py_obj) {
-  // Grab the desired reference type so that if we end up rejecting `py_obj` it
-  // still gets decremented if necessary.
-  PythonObject result(type, py_obj);
-
-  if (!PythonTuple::Check(py_obj)) {
-    PythonObject::Reset();
-    return;
-  }
-
-  // Calling PythonObject::Reset(const PythonObject&) will lead to stack
-  // overflow since it calls back into the virtual implementation.
-  PythonObject::Reset(PyRefType::Borrowed, result.get());
-}
-
 uint32_t PythonTuple::GetSize() const {
   if (IsValid())
     return PyTuple_GET_SIZE(m_py_obj);
   return 0;
 }
 
 PythonObject PythonTuple::GetItemAtIndex(uint32_t index) const {
   if (IsValid())
for (uint32_t i = 0; i < count; ++i) {
     PythonObject obj = GetItemAtIndex(i);
     result->AddItem(obj.CreateStructuredObject());
   }
   return result;
 }
 
 // PythonDictionary
 
-PythonDictionary::PythonDictionary(PyInitialValue value) : PythonObject() {
+PythonDictionary::PythonDictionary(PyInitialValue value) {
   if (value == PyInitialValue::Empty)
     Reset(PyRefType::Owned, PyDict_New());
 }
 
-PythonDictionary::PythonDictionary(PyRefType type, PyObject *py_obj)
-    : PythonObject() {
-  Reset(type, py_obj); // Use "Reset()" to ensure that py_obj is a dictionary
-}
-
-PythonDictionary::~PythonDictionary() {}
-
 bool PythonDictionary::Check(PyObject *py_obj) {
   if (!py_obj)
     return false;
 
   return PyDict_Check(py_obj);
 }
 
-void PythonDictionary::Reset(PyRefType type, PyObject *py_obj) {
-  // Grab the desired reference type so that if we end up rejecting `py_obj` it
-  // still gets decremented if necessary.
-  PythonObject result(type, py_obj);
-
-  if (!PythonDictionary::Check(py_obj)) {
-    PythonObject::Reset();
-    return;
-  }
-
-  // Calling PythonObject::Reset(const PythonObject&) will lead to stack
-  // overflow since it calls back into the virtual implementation.
-  PythonObject::Reset(PyRefType::Borrowed, result.get());
-}
-
 uint32_t PythonDictionary::GetSize() const {
   if (IsValid())
     return PyDict_Size(m_py_obj);
   return 0;
 }
 
 PythonList PythonDictionary::GetKeys() const {
   if (IsValid())
for (uint32_t i = 0; i < num_keys; ++i) {
     PythonObject key = keys.GetItemAtIndex(i);
     PythonObject value = GetItemForKey(key);
     StructuredData::ObjectSP structured_value = value.CreateStructuredObject();
     result->AddItem(key.Str().GetString(), structured_value);
   }
   return result;
 }
 
-PythonModule::PythonModule() : PythonObject() {}
-
-PythonModule::PythonModule(PyRefType type, PyObject *py_obj) {
-  Reset(type, py_obj); // Use "Reset()" to ensure that py_obj is a module
-}
-
-PythonModule::~PythonModule() {}
-
 PythonModule PythonModule::BuiltinsModule() {
 #if PY_MAJOR_VERSION >= 3
   return AddModule("builtins");
 #else
   return AddModule("__builtin__");
 #endif
 }
 

 
 bool PythonModule::Check(PyObject *py_obj) {
   if (!py_obj)
     return false;
 
   return PyModule_Check(py_obj);
 }
 
-void PythonModule::Reset(PyRefType type, PyObject *py_obj) {
-  // Grab the desired reference type so that if we end up rejecting `py_obj` it
-  // still gets decremented if necessary.
-  PythonObject result(type, py_obj);
-
-  if (!PythonModule::Check(py_obj)) {
-    PythonObject::Reset();
-    return;
-  }
-
-  // Calling PythonObject::Reset(const PythonObject&) will lead to stack
-  // overflow since it calls back into the virtual implementation.
-  PythonObject::Reset(PyRefType::Borrowed, result.get());
-}
-
 PythonDictionary PythonModule::GetDictionary() const {
   return PythonDictionary(PyRefType::Borrowed, PyModule_GetDict(m_py_obj));
 }
 
-PythonCallable::PythonCallable() : PythonObject() {}
-
-PythonCallable::PythonCallable(PyRefType type, PyObject *py_obj) {
-  Reset(type, py_obj); // Use "Reset()" to ensure that py_obj is a callable
-}
-
-PythonCallable::~PythonCallable() {}
-
 bool PythonCallable::Check(PyObject *py_obj) {
   if (!py_obj)
     return false;
 
   return PyCallable_Check(py_obj);
 }
 
-void PythonCallable::Reset(PyRefType type, PyObject *py_obj) {
-  // Grab the desired reference type so that if we end up rejecting `py_obj` it
-  // still gets decremented if necessary.
-  PythonObject result(type, py_obj);
-
-  if (!PythonCallable::Check(py_obj)) {
-    PythonObject::Reset();
-    return;
-  }
-
-  // Calling PythonObject::Reset(const PythonObject&) will lead to stack
-  // overflow since it calls back into the virtual implementation.
-  PythonObject::Reset(PyRefType::Borrowed, result.get());
-}
-
 PythonCallable::ArgInfo PythonCallable::GetNumInitArguments() const {
   ArgInfo result = {0, false, false, false};
   if (!IsValid())
     return result;
 
   PythonObject __init__ = GetAttributeValue("__init__");
   if (__init__.IsValid() ) {
     auto __init_callable__ = __init__.AsType<PythonCallable>();

 
 PythonObject PythonCallable::
 operator()(std::initializer_list<PythonObject> args) {
   PythonTuple arg_tuple(args);
   return PythonObject(PyRefType::Owned,
                       PyObject_CallObject(m_py_obj, arg_tuple.get()));
 }
 
-PythonFile::PythonFile() : PythonObject() {}
-
-PythonFile::PythonFile(PyRefType type, PyObject *o) { Reset(type, o); }
-
-PythonFile::~PythonFile() {}
-
 bool PythonFile::Check(PyObject *py_obj) {
   if (!py_obj)
     return false;
 #if PY_MAJOR_VERSION < 3
   return PyFile_Check(py_obj);
 #else
   // In Python 3, there is no `PyFile_Check`, and in fact PyFile is not even a
   // first-class object type anymore.  `PyFile_FromFd` is just a thin wrapper
#else
   if (r < 0) {
     llvm::consumeError(exception()); // clear the exception and log it.
     return false;
   }
   return !!r;
 #endif
 }
 
-void PythonFile::Reset(PyRefType type, PyObject *py_obj) {
-  // Grab the desired reference type so that if we end up rejecting `py_obj` it
-  // still gets decremented if necessary.
-  PythonObject result(type, py_obj);
-
-  if (!PythonFile::Check(py_obj)) {
-    PythonObject::Reset();
-    return;
-  }
-
-  // Calling PythonObject::Reset(const PythonObject&) will lead to stack
-  // overflow since it calls back into the virtual implementation.
-  PythonObject::Reset(PyRefType::Borrowed, result.get());
-}
-
 FileUP PythonFile::GetUnderlyingFile() const {
   if (!IsValid())
     return nullptr;
 
   // We don't own the file descriptor returned by this function, make sure the
   // File object knows about that.
   PythonString py_mode = GetAttributeValue("mode").AsType<PythonString>();
   auto options = File::GetOptionsFromMode(py_mode.GetString());