[Lldb-commits] [PATCH] D68918: eliminate virtual methods from PythonDataObjects

Mon Oct 14 02:09:32 PDT 2019

labath accepted this revision.
labath added a comment.
This revision is now accepted and ready to land.

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 `Expected`s, `Optional`s or whatever)..

================
Comment at: lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h:188-192
   // 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;
----------------
BTW, is this needed? My impression was that PythonObject was/is not convertible to a `PyObject*`..

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68918/new/

https://reviews.llvm.org/D68918